Open Bug 1629803 Opened 4 years ago Updated 19 days ago

Lazily resolved properties for functions break expected property order

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

People

(Reporter: anba, Unassigned)

References

(Blocks 1 open bug)

Details

Test262 failures because we're lazily creating the "name" and "length" properties of functions, which breaks the expected property iteration order:

test262/built-ins/Object/keys/order-after-define-property.js
test262/built-ins/Object/entries/order-after-define-property.js
test262/language/computed-property-names/class/static/method-number.js
test262/language/computed-property-names/class/static/method-string.js
test262/language/computed-property-names/class/static/method-symbol.js

Yes... we used to have a lot of resolve hooks and they all had this problem.

The general fix (apart from doing away with resolve hooks) would be to keep these properties lazy, but lazily create them in the right order, so that the list of properties physically present is always a prefix of what the spec says and what users must observe to be there.

Priority: -- → P3
Severity: -- → S3

Do we just change the order in which they are called here in the FunctionEmitter constructor?

https://searchfox.org/mozilla-central/source/js/src/frontend/FunctionEmitter.cpp#40

Noob question: In which file would the work be done for this bug?

Flags: needinfo?(jorendorff)

I think the change needs to be made in fun_resolve, in vm/JSFunction.cpp.

Flags: needinfo?(jorendorff)
See Also: → 1670042

The same behaviour is observable for other types of objects which are using lazily resolved properties, for example arguments objects:

js> Object.getOwnPropertyNames(function(){ var args = arguments; return args; }())               
["length", "callee"]
js> Object.getOwnPropertyNames(function(){ var args = arguments; args.callee; return args; }())
["callee", "length"]
js> Object.getOwnPropertyNames(function(){ var args = arguments; args.a = 0; return args; }())
["a", "length", "callee"]
  • RecordObject and TupleObject also define resolve hooks, but both are already special-cased in EnumerateNativeProperties.
  • StringObject defines a resolve hook, but only lazily resolves indexed properties, which are sorted in Enumerate anyway, so StringObject should work correctly.

We could add additional flags to JSFunction and ArgumentsObject to record if the "name", "length", etc. haven't been deleted and then special-case both objects in EnumerateNativeProperties to append the names at the front unless they've been already deleted. ArgumentsObject has plenty of free space for more flags, but JSFunction already uses all free bits in JSFunction::FlagsAndArgCountSlot. Well, we could add a new PrivateUint48Value to gain 16 more bits for flags. But I'm not sure whether it's worth it to spend that much time for this issue.

Duplicate of this bug: 1799287

We're working around this issue for WebIDL interface objects (which are builtin functions) here: https://searchfox.org/mozilla-central/rev/0e9ea50a999420d93df0e4e27094952af48dd3b8/dom/bindings/BindingUtils.cpp#941-943,955-963. The WebIDL spec calls CreateBuiltinFunction, which sets length and name properties, and then the WebIDL spec itself defines a prototype property. Depending on how this is fixed in SpiderMonkey we might be able to remove that workaround.

You need to log in before you can comment on or make changes to this bug.