826587 - Add a parameter to ObjectOps::defineProperty and ObjectOps::setProperty to return success/failure of the definition/setting (distinct from JSAPI's understanding of success via return value)

Status: RESOLVED DUPLICATE of bug 1113369

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

[[DefineOwnProperty]] in the spec takes a property name P, a descriptor Desc, and a boolean Throw that indicates whether a define failure should throw or not.

We don't have anything like the Throw parameter in our property-definition API.  We need to add it to evolve more in the direction of the spec APIs.  This also would apparently help out the WebIDL people, since they're generating proxy hooks that are working around this lack in bizarre ways now (like, by using the |bool set| passed to the getOwnPropertyDescriptor hook, which is just crazy).

Note that Throw is used to implement both strict mode behavior and making Object.defineProperty(...) throw when it should.  So the new parameter shouldn't be named "strict" or something like that.  And of course it should be an enum so that you see Throw/DontThrow at call sites, or something.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: WIP, doesn't compile at all

Right now I'm making the extra parameter be of type ErrorBehavior, an enum with Throw/DontThrow as its values.  In the spec the parameter is a boolean named Throw, because it's used by more than just strict mode (all the [[DefineOwnProperty]] calls in the array methods, for example, specify Throw=true), so it makes sense to do the same here.

It turns out, if you want to add this to defineProperty, you really want to add it to setProperty as well at the same time, because one more or less flows into the other.

Ms2ger pointed me at a similar bug for adding this to the deleteProperty hook as well, but that's clearly a separable concern.  It also might be less important than this bit of work, as far as the property/element splitting is concerned.

Comment 2

[Jeff Walden [:Waldo]]

So after the previous patchwork, I had the bright idea to look at ES6, and it changes [[DefineOwnProperty]] and [[SetP]] (roughly equivalent to setProperty) to not take a Throw/strict mode parameter, and to return their success or failure.  Callers then handle success/failure as desired.  So I'm morphing this to do that -- should cut down on complexity slightly, too.  New patchwork for that coming soonish, hopefully.

Comment 4

[André Bargull [:anba]]

Will resolving this bug make sure `Object.defineProperty(new Proxy(Object.defineProperty({}, "x", {value:0}), {}), "x", {value:1})` throws a TypeError? Asking here to avoid creating just another duplicate bug report...

Comment 5

[Florian Bender]

(In reply to André Bargull from comment #4)
> Will resolving this bug make sure `Object.defineProperty(new
> Proxy(Object.defineProperty({}, "x", {value:0}), {}), "x", {value:1})`
> throws a TypeError? Asking here to avoid creating just another duplicate bug
> report...

When in doubt, create a new bug (that depends on this one or adds it as "see also"), otherwise (if it's not a dupe), it may get lost.

Comment 6

[Jason Orendorff [:jorendorff]]

I'm patching bug 1113369 this week. I think this basically is a dupe of that; closing.

In short, ES6 re-implemented the Throw argument as a boolean return value (wince).