-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Runtime Semantics for MemberExpression do not conform to web reality(?) #1224
Comments
If you're correct here, then I wonder if it's something we could get browsers to change - i wouldn't expect the RHS to ever evaluate when evaluating the LHS throws. |
See also #467. |
@ljharb: I agree, and had filed a bug against V8 about it. But since (in the process of preparing that bug report, I discovered that) other major browsers do the same, I thought I should inquire about it here too. |
v8 already has a bug for it. (Edit: or rather, to be precise, they have a bug for evaluating the property name before doing the RequireObjectCoercible, though I don't know if anyone's noticed they also evaluate the RHS before doing the RequireObjectCoercible. The test in question tests both, but they fail at the property name part, since it comes first.) |
The spec says that if the LHS (base) of a MemberExpression fails CheckObjectCoercible / RequireObjectCoercible that the resulting TypeError should be propagated, which should preven the RHS of an enclosing AssignmentExpression from being evaluated. So do that, even though apparently no one else does (see https://bugs.chromium.org/p/v8/issues/detail?id=7847 and tc39/ecma262#1224).
Huh. Any idea why the failure you point to at https://github.com/v8/v8/blob/80ec11d20d8d895f347a91eeff0d21e62441c6fb/test/test262/test262.status#L50 is in a section titled "MISSING ES6 FEATURES"? This seems like a clear non-conformance rather than a missing feature. |
The spec says that if the LHS (base) of a MemberExpression fails CheckObjectCoercible / RequireObjectCoercible that the resulting TypeError should be propagated, which should preven the RHS of an enclosing AssignmentExpression from being evaluated. So do that, even though apparently no one else does (see https://bugs.chromium.org/p/v8/issues/detail?id=7847 and tc39/ecma262#1224).
The last time we discussed reference evaluation issues, and all implementations mismatched the spec, @efaust just went and fixed SpiderMonkey, and we left the spec as is. This time, it may be more complex and require reexamination--double evaluation (as we discussed previously) is somewhat of a tougher pill to swallow than these ordering issues (where the spec isn't "as much better" than implementation reality, and a full implementation of the Reference spec type may be difficult from implementation perspective). So, I wouldn't mind reconsidering the evaluation order here. @erights You can blame my sloppiness for not fixing up that comment when doing some of V8's test262 rolls. The comment doesn't mean anything in particular. |
function f() { throw "BAD"; }
null.x = f(); Per spec, this should throw a TypeError, but it throws "BAD" in JSC, V8, SM, and Moddable. engine262 follows the spec. This is worth revisiting now, because we're adding private fields, which have the same spec behavior. function f() { throw "BAD"; }
class C { #f = 1; constructor() { null.#f = f(); } }
new C; Per the proposal, this should throw a TypeError, but it this throws "BAD" in JSC, V8, and Moddable—all the engines that implement it. SM's implementation (not shipping) currently follows the spec. I'm a little worried about shipping it in that state. |
I note that §12.3.2.1 Property Accessors: Runtime Semantics: Evaluation says:
MemberExpression
:
MemberExpression.
IdentifierNameAnd § 12.15.4 Assignment Operators: Runtime Semantics: Evaluation says:
AssignmentExpression
:
LeftHandSideExpression=
AssignmentExpressionAnd yet the following code logs
whoops
in at least V8 (6.7.288.46), Firefox (60.0.2) and Safari (11.1.1), even though RequireObjectCoercible is defined to throwTypeError
when passedundefined
as its argument:The most plausible explanation for this apparent discrepancy is that I have misunderstood the spec in some way. Failing that, however: are these (several, prominent) implementations wrong, or should the spec be changed to reflect reality?
The text was updated successfully, but these errors were encountered: