-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[GR-71769] Remove array coercion rules #12708
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
graalvmbot
wants to merge
16
commits into
master
Choose a base branch
from
pz/GR-71769-array-coerce
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+1,144
−917
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The proxy handler always takes the hub instance as its only argument
setPrototypeOf is not recommended because it can mess up runtime optimizations.
It used the 'in' operator to check if the "$as" method was already defined, however because the _methods object has the prototype set to the _methods object of the superclass proxy handler, the "$as" property already exists (but not as an own property), but it points to a method that captures the supertype proxy handler causing the wrong class metadata to be loaded when used This only caused issues with WasmGC because the JS backend implemented _getSingleAbstractMethod (which is the method called on the proxy handler) without using 'this'.
The proxy now simplifies this because it implements the iterable protocol
This is now a uniform way to invoke toString on a JS value. This introduces a slight breaking change in that the JSValue.toString now has slightly different output (mostly no trailing .0 for integer values) because it now shows the value how the JS engine would show it, not how it would look like converted to Java
The Conversion class already contains a lot of similar utilities
It's a relatively new feature, only available on Node 24 or higher Ref: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/isError
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The existing array coercion rules require JS typed arrays to be coercible to Java arrays while observing changes in both directions and the other way around Java primitive arrays to be coercible to JS typed arrays.
This is only possible in WasmGC if we can have a Java array backed by a JS typed array, this introduces an overhead on all arrays and we don't do here.
Instead, all array coercion rules are removed in both directions. You cannot get a typed JS array backed by a Java array anymore and you can't get a Java primitive array backed by a JS typed array anymore.
This PR thus introduces some breaking changes in the JS interop API (which is experimental so this is fine):
JSObjectthrough conversion. You can't coerce to a Java primitive array anymore.JSValue#toStringnow contains thetoStringresult of callingtoStringon the underlying JS value instead of first converting it to Java and then callingtoStringon the Java boxed equivalent.JSValue.as*Arraymethods have been removed since casting to primitive arrays is no longer allowed:asBooleanArray,asByteArray,asShortArray,asCharArray,asIntArray,asFloatArray,asLongArray,asDoubleArrayThe removal of the JS to Java coercion is probably the biggest breaking change because there is no good way to make accessing JS arrays from Java seamless anymore: You now have to use
JSObject.getto access array elements (or alternatively create your own subtype that implements agetwith the right return value (and out of bounds semantics)).The remove of Java to JS coercion is only problematic if you relied on it being a typed array and/or accessed the underlying buffer, these things will not work anymore. Now you just get a proxy around the Java array. However, that proxy behaves almost like an array: the
lengthproperty exists on it, indexed accesses (e.g.arrayProxy[0] = 12) work, and it implements the iterable protocol meaning things like the spread operator (...arrayProxy), orforloops just work from JavaScript.Accessing elements of primitive arrays is subject to the JS->Java (for stores) and Java->JS (for loads) coercion rules:
boolean[]instances return JSbooleanfor loads and only accept JSbooleanfor stores (otherwiseTypeError)byte[],short[],char[],int[],float[], anddouble[]instances return JSnumberfor loads and only accept JSnumberandbigintfor stores (otherwiseTypeError). Additionally, the numbers have to be in range for the target Java type (e.g. from 0 to 65536 forchar[]) and must be integer values for the integer valued arrays (otherwiseRangeError)long[]behaves the same as the other number arrays but returns a JSbigintfor loads.See
ArrayProxyTestfor the full set of expected behavior of the new Java array proxies.This also cleans up how we create proxies a bit, unifying some functionality between JS and WasmGC.
Due to the more powerful array proxies, this didn't break too many tests. Only the ones that relied on coercion to a Java array had to be removed.