Closed
Bug 571452
Opened 14 years ago
Closed 12 years ago
proxy construct trap coerces to object but should faithfully return primitives instead (per spec)
Categories
(Core :: JavaScript Engine, defect, P2)
Tracking
()
RESOLVED
INVALID
People
(Reporter: gal, Unassigned)
References
Details
jorendorff: "My reading of the straw-man spec is that JSProxy::construct and JSProxyHandler::construct should not take a 'receiver' argument at all.
The JSClass::construct hook is not the same thing as the constructTrap for
function proxies, so we're probably not implementing the straw-man
correctly. JSClass::construct only virtualizes the function-calling step of
constructing a new object (specifically, step 8 of the [[Construct]] internal
method described in ES5 13.2.2). I think the constructTrap is supposed to
virtualize the entire [[Construct]] method.
This further raises the question of whether the straw-man intends that |new x|
can be a primitive value if x happens to be a function proxy. I imagine not; it
should be brought up in the committee."
Reporter | ||
Updated•14 years ago
|
Summary: TM: → TM:construct hook might not be sufficient to faithfully implement a proxy's construct trap
Reporter | ||
Updated•14 years ago
|
Blocks: harmony:proxies
Summary: TM:construct hook might not be sufficient to faithfully implement a proxy's construct trap → TM: construct hook might not be sufficient to faithfully implement a proxy's construct trap
Updated•14 years ago
|
Summary: TM: construct hook might not be sufficient to faithfully implement a proxy's construct trap → JSClass construct hook might not be sufficient to faithfully implement a proxy's construct trap
Reporter | ||
Comment 1•14 years ago
|
||
Alright, so looking at the spec and the implementation, I am not sure this is really a bug.
js> var p = Proxy.createFunction(({ getPropertyDescriptor: function() { return ({}); } }), function() { return 5; }, function() { return 5; });
js> new p()
5
js> typeof (new p())
"number"
jorendorff, can you explain to me again what exactly you are concerned about?
The new expression should always produce an object or throw
Reporter | ||
Comment 3•14 years ago
|
||
Yeah, thats why I posted the code above. I have to censor primitive return values, but that has nothing to do with the Class construct hook. The issue above is all I could think of wrt non-compliance.
Comment 4•14 years ago
|
||
Andreas, I think you're right that the summary is too dire. JSClass::construct is sufficient; the actual issues are:
1. We're passing a this-argument to the construct trap. The straw-man says "The this-binding of the constructTrap is undefined." I think that means it should behave like this:
var a, p;
p = Proxy.createFunction({getPropertyDescriptor: function(){return {};}},
function () {},
function () { a = this; return {};});
new p();
assertEq(a === this); // the global object
p = Proxy.createFunction({getPropertyDescriptor: function(){return {};}},
function () {},
function () { "use strict";
a = this; return {};});
new p();
assertEq(a === undefined);
Currently we fail both assertions. This is what I meant by "JSProxy::construct and JSProxyHandler::construct shouldn't take a 'receiver' argument."
2. I think the spec says that what we do in comment 1 is correct. The committee probably hasn't discussed that; they might decide on the behavior shaver wants. Or not. I don't know.
Reporter | ||
Comment 5•14 years ago
|
||
Ok, we have consensus then. Some stuff needs to be hashed out here. I hope Tom and Mark can chime in. But this is minor surgery, and the general use of the construct JSClass hook seems viable.
Comment 6•14 years ago
|
||
* What strawman spec is comment 1 referring to?
* Step 6 for [[Construct]] at <http://wiki.ecmascript.org/doku.php?id=harmony:proxies_semantics#detailed_semantics_of_additional_behavior_for_function_proxies> is
6. Return ToObject(result).
which would give the invariant Mike Shaver asks for in comment 2. However, on reflection, I'm not sure this is good. We've already removed from ES5/strict (and therefore from ES6) other cases where primitive values are implicitly wrapped. It would be a shame to lose ground on that. I agree it should be brought up in committee.
* Regarding the this-binding of the constructTrap, the two assertions in comment 4 must pass. Anything else violates step 5 of that same spec and is clearly a bug.
Comment 7•14 years ago
|
||
Hi Mark, thanks for commenting.
(In reply to comment #6)
> * What strawman spec is comment 1 referring to?
I'm sure the non-strawman harmony:proxies proposal was meant ;-).
> * Step 6 for [[Construct]] at
> <http://wiki.ecmascript.org/doku.php?id=harmony:proxies_semantics#detailed_semantics_of_additional_behavior_for_function_proxies>
> is
>
> 6. Return ToObject(result).
>
> which would give the invariant Mike Shaver asks for in comment 2. However, on
> reflection, I'm not sure this is good. We've already removed from ES5/strict
> (and therefore from ES6) other cases where primitive values are implicitly
> wrapped. It would be a shame to lose ground on that.
I agree. If we do add value types, one plausible basis is proxies. These would not want to produce a non-primitive from new.
> I agree it should be brought up in committee.
Could you please mail John Neumann about adding it to the agenda? Thanks.
> * Regarding the this-binding of the constructTrap, the two assertions in
> comment 4 must pass. Anything else violates step 5 of that same spec and is
> clearly a bug.
Absolutely.
/be
Sorry, I wasn't objecting because I don't personally like producing primitives from proxies -- though I think that doing so via the |new| operator is pretty confusing -- but rather because ES5 seems to require it:
ES5 11.2.2 says that [[Construct]] produces the value, and Table 9 in 8.6.2 says that [[Construct]]'s domain is "SpecOp(a List of any) → Object" with the description stating that it "[c]reates an object."
I wasn't proposing that it automatically wrap primitives, but rather that a proxy hook returning a primitive for construct would trigger an exception. If we can make proxies obey different rules, that's more than fine with me, I just didn't know that was in the scope of the design.
Furthermore, as regards value-types-atop-proxies, this would only matter if one tried to [[Construct]] on a proxy-based-value-type-instance which then produces another primitive which was therefore *not* a proxy-based-value-type-instance (whew!), which would certainly seem an unusual departure from what we permit on instances of the current primitive types.
Comment 10•14 years ago
|
||
(In reply to comment #9)
> Furthermore, as regards value-types-atop-proxies, this would only matter if one
> tried to [[Construct]] on a proxy-based-value-type-instance which then produces
> another primitive which was therefore *not* a proxy-based-value-type-instance
> (whew!), which would certainly seem an unusual departure from what we permit on
> instances of the current primitive types.
Not if we let proxies customize typeof-type and otherwise implement value types. But I agree this is in the future. Throwing for now is prudent.
/be
Comment 11•14 years ago
|
||
(In reply to comment #6)
> * What strawman spec is comment 1 referring to?
http://wiki.ecmascript.org/doku.php?id=harmony:proxies#semantics
I wasn't aware of the new page.
Comment 12•14 years ago
|
||
At the last TC39 meeting, I believe we had consensus that "new FunctionProxy()" should return whatever the function proxy's "construct" trap returns (so no coercion to Object, no wrapping of primitives).
I updated the [[Construct]] trap semantics at <http://wiki.ecmascript.org/doku.php?id=harmony:proxies_semantics#detailed_semantics_of_additional_behavior_for_function_proxies> accordingly.
Reporter | ||
Updated•14 years ago
|
Priority: -- → P2
Reporter | ||
Comment 13•14 years ago
|
||
JS_REQUIRES_STACK bool
InvokeConstructor(JSContext *cx, const CallArgs &argsRef)
{
...
/* Check the return value and if it's primitive, force it to be obj. */
if (args.rval().isPrimitive()) {
if (callee->getClass() != &js_FunctionClass) {
/* native [[Construct]] returning primitive is error */
JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
JSMSG_BAD_NEW_RESULT,
js_ValueToPrintableString(cx, args.rval()));
return false;
}
args.rval().setObject(*obj);
}
we coerce pretty deep down in the layering. We don't know here its a proxy. That needs to be fixed.
Reporter | ||
Updated•14 years ago
|
Summary: JSClass construct hook might not be sufficient to faithfully implement a proxy's construct trap → proxy construct trap coerces to object but should faithfully return primitives instead (per spec)
Updated•13 years ago
|
Blocks: harmony:proxy
Updated•13 years ago
|
No longer blocks: harmony:proxy
Comment 14•12 years ago
|
||
Marking this bug as invalid since Proxy.createFunction is not relevant in the direct proxies design.
Feel free to change the status if I'm mistaken.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•