Annex E incompatibilities should include "early Reference Error"

[saurik]

In earlier ECMAScript versions, the syntax ++++a would lead to a ReferenceError when executed, but was considered valid code: if it existed in some large script and was never used, it would not lead to a failure. In ECMAScript 2015, this behavior has been changed to require an "early Reference Error" per 12.4.1, 12.5.1, and 12.14.1. As far as I can tell, this has not been marked as being an incompatibility.

Worse, while we would pray that people were not relying on this strange syntax, Chrome has judged that in at least one case this behavior is not compatible with the legacy web :/. In researching why V8 was not throwing errors in some circumstances (such as how 12.3.1.5 requires that function calls return false for IsValidSimpleAssignmentTarget), I determined that Chrome backed out this change a while back.

https://codereview.chromium.org/217823003

Make invalid LHSs that are calls late errors Necessary for web legacy compatibility.

V8 version 4.9.0 (candidate) [sample shell]
> if (false) a() = 0; true
true

Mozilla actually has a similar compatibility case in SpiderMonkey, though I can't find any discussion to find out exactly why. (They modify IsValidSimpleAssignmentTarget to gate on a feature flag they call "PermitAssignmentToFunctionCalls", which is in turn controlled by whether a "keyed destructuring" is in place, though I haven't yet taken the time to figure out what is going on there... but the syntax works ;P.)

https://dxr.mozilla.org/mozilla-central/rev/97e537f85183ef31481602ab9e5587a6e7d16b4d/js/src/frontend/Parser.cpp#7272

js> if (false) a() = 0; true
true

There is a mention in Annex E that "6.2.3: In ECMAScript 2015, Function calls are not allowed to return a Reference value.", but I do not believe that is a sufficient notice of the incompatibility (and would bet it is not intended to address this case, but I could be wrong?). I also did not see anything in Annex B that "for web compatibility reasons some of these early errors are relaxed", but maybe I'm just being dense.

(The versions of JavaScriptCore I have easy access to currently, which may or may not be terribly up-to-date, simply do not implement any of these early error semantics, so they would not have run into any compatibility concerns that I can try to dig up and report.)

[anba]

In earlier ECMAScript versions, the syntax ++++a would lead to a ReferenceError when executed, but was considered valid code: if it existed in some large script and was never used, it would not lead to a failure. In ECMAScript 2015,

This is specified as an early error since ECMAScript 5 (ES5, ch. 16). ECMAScript 3 made it an implementation dependent choice whether or not to report early errors for invalid assignments. Specifically this sentence from ES3:

An implementation may treat any instance of the following kinds of runtime errors as a syntax error and therefore report it early:

was changed in ES5 to:

An implementation must treat any instance of the following kinds of errors as an early error:

The assignment to a function call is a special case: ES5 allowed function calls to return a Reference value (ES5, ch. 8.6.2, Table 9), this feature was removed in ES2015 as documented in Annex E.

[littledan]

It's still a dilemma in Chrome whether to prohibit these sorts of things. Does anyone have data about whether they are used on the web?

[allenwb]

So this has all been extensively discussed in TC39 when these changes were introduced.

When the change from optional early error to mandatory early error was made in ES5, I believe the state of the web was that some of the "top 5" browsers (that was the criteria we used back then) reported early errors and others did not. That meant that code that that depended upon late error treatment was not interoperable across browsers on the web. In other words, it was a non-breaking change.

Reference returning host functions (and the syntax that allowed their invocation) was in the spec. to support VBScript compatibility in early versions of IE. I'm pretty sure you can find meeting notes that document this decision.

[littledan]

Thanks for the references, Allen.

Unfortunately, not all browsers can ship something just because some other browser in the past has shipped it. In Chrome, we have to worry about compatibility with subsets of the web which currently work on Chrome, such as websites which are written for the 'mobile web', which in practice means WebKit-based browsers. In other parts of the web platform, WebKit prefixes are unfortunately being standardized for similar reasons. "Websites which work on non-Presto browsers" would probably be another similar subset (not sure how much that affected ES2015 decisions). Sometimes these subsets of web usage have things in common with each other which certain browsers don't follow. We would really like to both adhere to standards and not break the web.

Is there a strong reason why this has to be an early error? What sort of bad consequences would come if we made these runtime errors in the spec?

[ajklein]

I just tested current SpiderMonkey, JSC, V8, and Chakra, and all fail to throw an error for the following code:

function f() {
  g() = 5;
}

They all of course throw an error if f is called.

Given the above, it seems like the most reasonable thing would be to specify this behavior. If it must be in an Annex, so be it, but I don't think we have any evidence that early errors in such cases are web-compatible. Until someone presents such evidence, it seems safest to specify the current browser behavior.

[ajklein]

@bterlson any thoughts here? Would you merge a PR that made this change, or should I put this on the agenda for a future meeting?

[evilpie]

I just wanted to note that SpiderMonkey doesn't parse this syntax in strict mode.

[bterlson]

@ajklein We should make a proposal PR. There will be debate about where to put it though (annex B vs. main spec) so it's needs-consensus for sure. Sure would be good to get data about how common this is.  We were able to make some changes like this in the past - eg. SM/JSC was able to fix the re-evaluation of LHS references issue.

[littledan]

@bakkot implemented data collection in V8 for the f() = x case, separating out strict and sloppy mode. If all goes well, we should have some good data in four months or so. Does anyone have more information about the ++++x case?

[bakkot]

@littledan, in ES3 the grammar included UnaryExpression : ++ UnaryExpression, and no Early Errors or anything of the kind to restrict it syntactically. But as far as I'm aware there was no environment in which this was ever anything other than an error at runtime (unlike f() = 0), so browsers either never supported it or stopped supporting it sometime long past. That one, and related things like ++~x etc., are almost certainly safe to leave as-is.

Edit: although as this issue was originally about, those cases should be marked in Annex E.

[ljharb]

@littledan @bakkot what were the results of the v8 data collection?

[bakkot]

@ljharb Use is very low in strict mode and less low in sloppy mode. Note that Spidermonkey and (as of recently, thanks @rkirsling) JSC only allow it in sloppy mode, so forbidding it in strict mode is very likely to be web compatible.

I can't make the call for what browsers would be comfortable shipping, but eyeballing the above graphs (and given that every browser has found they need to allow this syntax in sloppy mode), I think it makes to spec "f() = 0 is a runtime {syntax/reference} error in sloppy mode, early syntax error in strict" (details TBD) at this point.

[bakkot]

For posterity, I believe the historical use of this syntax was for vbarrays: https://matrixlogs.bakkot.com/TC39_Delegates/2021-10-26#L443-L450