Runtime Semantics for MemberExpression do not conform to web reality(?)

[cpcallen]

I note that §12.3.2.1 Property Accessors: Runtime Semantics: Evaluation says:

MemberExpression : MemberExpression . IdentifierName

1. Let baseReference be the result of evaluating MemberExpression.
2. Let baseValue be ? GetValue(baseReference).
3. Let bv be ? RequireObjectCoercible(baseValue).
4. …

And § 12.15.4 Assignment Operators: Runtime Semantics: Evaluation says:

AssignmentExpression : LeftHandSideExpression = AssignmentExpression

1. If LeftHandSideExpression is neither an ObjectLiteral nor an ArrayLiteral, then
2. Let lref be the result of evaluating LeftHandSideExpression.
3. ReturnIfAbrupt(lref).
4. Let rref be the result of evaluating AssignmentExpression.
5. …

And 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 throw TypeError when passed undefined as its argument:

'use strict';
undefined.foo = console.log('whoops');

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?

[ljharb]

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.

[bakkot]

See also #467.

[cpcallen]

@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.

[bakkot]

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.)

Here's the test262 test for this behvior, which v8 xfails.

[erights]

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.

[littledan]

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.

[jorendorff]

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.