whatwg
Normative: Match ECMA‑262 function property enumeration order
This makes the order of the "length", "name" and "prototype" properties on WebIDL function objects match web reality (at least Firefox’s behaviour) and ECMA‑262 as of https://github.com/tc39/ecma262/pull/1490 and https://github.com/tc39/ecma262/pull/2116, which also changed CreateBuiltinFunction so that it takes the additional length and name required parameters.
- [x] At least two implementers are interested (and none opposed):
- Firefox (already implements this)
- Safari (implemented as of changeset 280289)
- [x] Tests are written and can be reviewed and commented upon at:
- [x] Implementation bugs are filed:
- Chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=1257969
- Firefox: N/A
- Safari: https://bugs.webkit.org/show_bug.cgi?id=230584 (FIXED)
(See WHATWG Working Mode: Changes for more details.)
review?(@annevk, @domenic, @littledan, @ljharb)
Fixes https://github.com/whatwg/webidl/issues/1106
https://github.com/tc39/ecma262/pull/2116 has been merged, so this PR has been updated to pass the length and name parameters to CreateBuiltinFunction directly.
Great work @ExE-Boss! I'll leave this to others to review, but this looks like a really nice cleanup. Do we have some tests for this as well?
Based on https://github.com/web-platform-tests/wpt/pull/30333 it looks like this matches Firefox but mismatches Chrome and Safari (for constructors). So we should make sure at least one of those browsers is interested in changing. @yuki3 @shvaikalesh what do you think?
Reflect.ownKeys(Blob):
- Chrome: ["length", "name", "arguments", "caller", "prototype"]
- Firefox: [ "length", "name", "prototype", Symbol("Symbol.hasInstance") ]
- Safari: ["prototype", "name", "length"]
So, Chrome has additional arguments and caller properties, which surprises me. (They appear to be non-functional, at least - (function f(){ new Blob([], { get type(){ console.log(Blob.caller, Blob.arguments) } }) })() prints null twice.)
But Chrome does match the enumeration order for the length and name properties, which I think is the only change actually implied by this PR. In particular, ecma262 specifically refrains from specifying the full enumeration order, including of prototype; it only specifies that length appears before name. (The test262 tests for the corresponding ecma262 PR are much more permissive than the WPT linked above.)
So, possibly the WPT is just over-zealous for the actual change implied by this PR, and could be changed to match the test262 tests. Then both Chrome and Firefox would pass.
@bakkot to check my understanding, it seems like the correct test would be that name appears right after length, right? So the test262 test is not as fully strict as it could be.
I think Web IDL, being more imperative than the ES spec, actually gives a full ordering between length/name/prototype that matches the tests written. But in terms of ensuring implementer interest for this particular change, I agree we're probably OK.
it seems like the correct test would be that name appears right after length, right? So the test262 test is not as fully strict as it could be.
Actually, the test262 test checks exactly that using: nameIndex === lengthIndex + 1
assert(lengthIndex >= 0 && nameIndex === lengthIndex + 1,
"The `length` property comes before the `name` property on built-in functions");
@bakkot
So, possibly the WPT is just over-zealous for the actual change implied by this PR, and could be changed to match the test262 tests. Then both Chrome and Firefox would pass.
With this PR, the prototype property is the first property that’s added to constructors after the CreateBuiltinFunction call in create an interface object.
it seems like the correct test would be that name appears right after length, right?
Right.
I think Web IDL, being more imperative than the ES spec, actually gives a full ordering between length/name/prototype that matches the tests written.
Ah, so it does.
I like this change, both editorial-wise and the way it improves enumeration order consistency. Please use this bug to follow WebKit implementation.
As for caller and arguments, there is a proposal to move them to Function.prototype, which was recently implemented in WebKit to match Firefox.
Based on web-platform-tests/wpt#30333 it looks like this matches Firefox but mismatches Chrome and Safari (for constructors). So we should make sure at least one of those browsers is interested in changing. @yuki3 @shvaikalesh what do you think?
I'm very sorry that I somehow overlooked the mention. I'm fine with the change to make the order of property enumeration consistent. However, I'm afraid that I cannot help much on this. IIUC, the order of these standard(ish?) properties on function objects is controlled by V8. We'd better ask V8 team to make the change. @verwaest, any thoughts?
The proposed changes sound alright. Making this more uniform makes sense.
It seems like we're invalidly adding special accessors for caller/arguments but will always return null for native functions. We can change that too.
As for moving caller/arguments to the prototype chain: Currently v8 returns the caller of the "holder" on which the property is installed. With this I mean that if you put a function with a .caller on the prototype chain of e.g., a sloppy-mode arrow function, we won't return the caller of the arrow function, but the caller of the function on the prototype chain. What are the expected semantics there? (The change itself sounds like an improvement though!)
@verwaest The proposal sets up Function.prototype.{arguments,caller} as getters (spec), so they can return null or real caller or even throw a TypeError depending on a holder (receiver).
One important bit of the proposal I kinda hope V8 would implement is returning null for (async) generators to match the JSC: we can't return real caller for them and it seems web-compatible.
The “needs tests” label is wrong, as https://github.com/web-platform-tests/wpt/pull/30333 has been merged.
Argh, apologies for merging the tests ahead of time. Would be good to get this landed anyway, though.
That's blocked on one of two things:
- @ExE-Boss updates https://github.com/whatwg/participant-data/blob/main/individuals.json with their real name. (See https://github.com/whatwg/sg/issues/93#issuecomment-1120799691 for context.)
- Someone else makes this contribution in a new PR.