ecma262 icon indicating copy to clipboard operation
ecma262 copied to clipboard

Normative: new Annex B.3 clause to specify "AssignmentTargetType of LeftHandSideExpression : CallExpression" web reality.

Open rwaldron opened this issue 5 years ago • 11 comments

The intention of this change is to allow JS engines which run in browsers to be spec conformant in their handling of assignment to CallExpression Arguments. Both Chrome and Firefox have attempted to make the following a Syntax Error:

f() = 1

...However they were unable to do so without breaking the web! As it turns out, every engine that runs in a web browser has implemented the same behavior:

$ eshost -x 'let f = () => {}; f() = 1'
#### ChakraCore

ReferenceError: Invalid left-hand side in assignment

#### JavaScriptCore

ReferenceError: Left side of assignment is not a reference.

#### SpiderMonkey

ReferenceError: cannot assign to function call

#### V8

ReferenceError: Invalid left-hand side in assignment

Note that non-browser embedded engines, which do not bear the same burden as the above engines, implement this according to the current specification:

$ eshost -x 'let f = () => {}; f() = 1' --no-color
#### engine262

SyntaxError: Invalid assignment target

#### graaljs

SyntaxError: Invalid left hand side for assignment

#### hermes

SyntaxError: invalid assignment left-hand side

#### Moddable XS

SyntaxError: no reference

#### qjs

SyntaxError: invalid assignment left-hand side

I'd like to propose that TC39 codify the web reality. If there is a better way to define this, please advise!

cc @syg @jorendorff

rwaldron avatar Sep 25 '20 16:09 rwaldron

See older discussion in https://github.com/tc39/ecma262/issues/257.

Note that both SpiderMonkey and JavaScriptCore make this an early error in strict code, so it should be safe to make this legal only in non-strict code. V8 has use counters for this pattern in both strict and non-strict code, which show almost no usage in strict code and a nontrivial amount of usage in non-strict code.

bakkot avatar Sep 25 '20 17:09 bakkot

I agree with @bakkot in https://github.com/tc39/ecma262/pull/2193#issuecomment-699047908. Seems like a good idea to clean this up for strict, but too risky for non-strict.

syg avatar Sep 25 '20 17:09 syg

Do we instead want to put non-strict in the main body with an "icky" note?

devsnek avatar Sep 25 '20 17:09 devsnek

I've added the limitation to non-strict code, however I'm not sure the language is exactly right.

rwaldron avatar Oct 05 '20 15:10 rwaldron

The Early Errors in https://tc39.es/ecma262/#sec-assignment-operators-static-semantics-early-errors also need to be modified by Annex B to allow CallExpression in sloppy mode.

ExE-Boss avatar Oct 08 '20 03:10 ExE-Boss

sending this again: Do we instead want to put this in the main body with an "icky" note?

devsnek avatar Oct 08 '20 03:10 devsnek

Thanks for following up with this PR, @rwaldron . It's too bad we all dropped it on the floor earlier. LGTM in its current form.

Note that this version does not include logical assignment operators, and as such, V8 is banning this construct. (Apparently there are already test262 tests for this.) https://chromium-review.googlesource.com/c/v8/v8/+/2423721 . This prohibition seems appropriate to me, since it's not needed for compatibility.

littledan avatar Oct 26 '20 13:10 littledan

Hmm, I always feel like it's better not to make an already-unfortunate rule more complicated than it has to be. We can certainly get away with allowing f() *= and banning f() &&= at parse time, but this requires awareness of &&= being a recent feature, in spite of the fact that it could have easily been added to the spec many years ago.

rkirsling avatar Oct 26 '20 19:10 rkirsling

this requires awareness

Awareness by whom?

bakkot avatar Oct 26 '20 19:10 bakkot

Spec readers, I guess? I understand that users won't write this, and if they do they'll get an error sooner or later, but it seems odd to create two special rules when one will do—no one seems to benefit from the added complexity.

rkirsling avatar Oct 26 '20 19:10 rkirsling

Whew, four years have gone by. Am I correct in understanding that this is still what we'd want, it was just never presented in plenary?

(I guess I was the most recent commenter here but I honestly don't feel that strongly about what I said at that time.)

rkirsling avatar Sep 24 '24 01:09 rkirsling

Okay, lemme state that I'm happy to champion this, though I imagine I'll need to do so in a new PR. (May as well keep using this one in lieu of an issue until then though.)

rkirsling avatar Apr 14 '25 07:04 rkirsling

@rkirsling You should follow the conventions being established in https://github.com/tc39/ecma262/pull/2952.

michaelficarra avatar Apr 14 '25 14:04 michaelficarra

Hmm, want to make note of an interesting thing here. Although browsers are in alignment about throwing a ReferenceError at runtime, SM/V8 perform the function call first, while JSC does not.

function f() { print("hi"); }

try { f() *= 1; } catch {}
try { f()++; } catch {}
try { for (f() of [1,2,3]) {} } catch {}
#### JavaScriptCore


#### SpiderMonkey, V8
hi
hi
hi

I think SM/V8 should be considered correct though, since presumably we want the ReferenceError to come from step 1 of PutValue.

rkirsling avatar Apr 18 '25 07:04 rkirsling

Closing this PR now that https://github.com/tc39/ecma262/pull/3568 is active.

rkirsling avatar Apr 21 '25 06:04 rkirsling