Re: Intent to Implement and Ship: Let all early errors be SyntaxErrors

[Ross Kirsling]

+v8-users@

[Ross Kirsling]

Contact email

rkir...@gmail.com

Explainer

https://github.com/tc39/ecma262/pull/1527#issuecomment-489228449

Spec

https://github.com/tc39/ecma262/pull/1527

Summary

Replace all early ReferenceErrors with early SyntaxErrors.

Motivation

The spec has been inconsistent with respect to the "early ReferenceError" notion: only update/assignment expressions (e.g. `0++`, `0 = 0`) have had early ReferenceErrors, while other invalid assignment target cases like `[0] = []` and `for (0 of []) {}` have been early SyntaxError.

Furthermore, this becomes an observable concern with eval: if eval throws a ReferenceError, the user has had no way to know whether the code was actually executed or simply parsed (short of matching on error message strings).

Risks

Interoperability

None. This reached consensus at TC39, it received explicit positive feedback from all major engines, and it is already shipping in Safari TP 83.

Compatibility

None expected. This simply changes a parse-time error type. As mentioned above, it would have been nearly impossible for userland code to depend on this typing.

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.

Is this feature fully tested by Test262?

Yes: https://github.com/tc39/test262/pull/2147

Link to entry on the feature dashboard

https://www.chromestatus.com/feature/5756460363415552

Requesting approval to ship?

Yes.

[Sathya Gunasekaran]

On Thu, Jun 6, 2019 at 11:29 AM Yoav Weiss <yo...@yoav.ws> wrote:

>
>
>
> On Tue, Jun 4, 2019 at 5:30 PM Ross Kirsling <rkir...@gmail.com> wrote:
>>
>> Contact email
>>
>> rkir...@gmail.com
>>
>>
>> Explainer
>>
>> https://github.com/tc39/ecma262/pull/1527#issuecomment-489228449
>>
>>
>> Spec
>>
>> https://github.com/tc39/ecma262/pull/1527
>>
>>
>> Summary
>> Replace all early ReferenceErrors with early SyntaxErrors.
>>
>> Motivation
>> The spec has been inconsistent with respect to the "early ReferenceError" notion: only update/assignment expressions (e.g. `0++`, `0 = 0`) have had early ReferenceErrors, while other invalid assignment target cases like `[0] = []` and `for (0 of []) {}` have been early SyntaxError.
>>
>> Furthermore, this becomes an observable concern with eval: if eval throws a ReferenceError, the user has had no way to know whether the code was actually executed or simply parsed (short of matching on error message strings).
>>
>> Risks
>> Interoperability
>> None. This reached consensus at TC39, it received explicit positive feedback from all major engines, and it is already shipping in Safari TP 83.
>>
>> Compatibility
>> None expected. This simply changes a parse-time error type. As mentioned above, it would have been nearly impossible for userland code to depend on this typing.
>
>

> I'm confused. Above you say "this becomes an observable concern with eval" which seems to contradict your "Compatibility" section.

I think Ross is trying to say that it becomes *less* of a concern with
this change. Previously, with a ReferenceError you wouldn't know if it
was an early error or not. But now, with this change, you will know
for sure that a ReferenceError is not an early error. This change
makes everything more consistent.

> Can you elaborate on the observability of this change and how would code that depends on it may look like? (if it can be observed)

The observability of this change is that all early errors that were
ReferenceErrors are now SyntaxErrors. If previously you had code that
eval'd something that resulted in a ReferenceError, it will now be a
SyntaxError.

In the following example,
```
try {
eval('0 = 0');
} catch (e) {
if (e instanceof ReferenceError) {
doFoo();
} else {
doBar();
}
}
```

this change will run doBar, instead of doFoo.

I don't expect to see code that special cases against ReferenceError
in the wild especially for just early errors. I expect to see code
that handles both ReferenceErrors and SyntaxErrors. Since we're not
adding a new type of error, I wouldn't expect to see any breakage.

Thanks,
--Sathya

>>
>> Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?
>> Yes.
>>
>> Is this feature fully tested by Test262?
>> Yes: https://github.com/tc39/test262/pull/2147
>>
>> Link to entry on the feature dashboard
>> https://www.chromestatus.com/feature/5756460363415552
>>
>> Requesting approval to ship?
>> Yes.
>>

>> --
>> You received this message because you are subscribed to the Google Groups "blink-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
>> To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHnZyqBi3XLxmrsCUVt1tcOirf4rQzz_RwzHh89%2B0zCk0B-e6w%40mail.gmail.com.
>
> --
> You received this message because you are subscribed to the Google Groups "blink-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
> To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CACj%3DBEiHuLpw%3D7wU8Hya4%2B-0X6MNcUm5Pvbn4Xwy-u9PSp%3DbCw%40mail.gmail.com.

[Ross Kirsling]

On Mon, Jun 17, 2019, 11:42 Chris Harrelson <chri...@google.com> wrote:

Hi again,

Sorry for the slow response.

On Thu, Jun 6, 2019 at 7:17 AM Sathya Gunasekaran <gsa...@chromium.org> wrote:

I don't expect to see code that special cases against ReferenceError
in the wild especially for just early errors. I expect to see code
that handles both ReferenceErrors and SyntaxErrors. Since we're not
adding a new type of error, I wouldn't expect to see any breakage.

Do you have any data to support this? Are there any use counters for how often this situation might occur? e.g. what fraction of pages trigger ReferenceErrors?

Also, have any other browsers shipped this change yet?

The concern here is purely around *early* ReferenceError. Since ReferenceError has been ambiguous between parse time and runtime, it would have been extremely difficult/fragile to use this information—as mentioned, one would need to string-match on error messages. (I think folks are hung up on the phrase "observable concern", but I meant this to indicate that there's something non-cosmetic to be *gained*, not that there's worry about breakage.)

The new behavior shipped in Safari TP 83 three weeks ago, and it's important to note that the major engines were not previously in alignment anyway. V8 was conforming to spec, but JSC actually had *late* ReferenceError for all four cases and SM was split (`0++` and `0--` were early SyntaxError while `0 = 0` and `0 += 0` were early ReferenceError).

[Adam Klein]

Hi Yoav,

We don't have existing metrics that would help here, and I honestly don't even know what we would track for this specific issue (detecting that a certain exception came from eval, for example, isn't something we currently track).

Bug reports are how we tend to track issues like this. And fwiw my intuition matches Sathya & Ross's (that this is very unlikely to cause problems in practice).

On Mon, Jun 17, 2019 at 9:42 PM Yoav Weiss <yo...@yoav.ws> wrote:

LGTM2

Do we have metrics in place in case we got it wrong and sites do rely on eval's error types in weird and unexpected ways? Or we're relying on bug reports to alert us of that?

On Tue, Jun 18, 2019 at 6:34 AM Chris Harrelson <chri...@chromium.org> wrote:

I see. Given that lack of interop, I do agree that probably sites are not relying on the current behavior, and if they are then they are likely not interoperable.

LGTM1 to try.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CACj%3DBEhQtWJoxxi3PwfutmSbZFJGyNkKpgD1OeMFDM6%3D2sA4bw%40mail.gmail.com.

[Frank Tang]

Is this feature controlled by a flag. 

I try to roll test262 and got a lot of errors related to this now. 

--
--
v8-users mailing list
v8-u...@googlegroups.com
http://groups.google.com/group/v8-users
---
You received this message because you are subscribed to the Google Groups "v8-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to v8-users+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/v8-users/CAEvLGcKV93MirncPshaX9z29%3D5-0aBcecEOJ-Z%3D0zZE1%2BQb%3D7w%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[Ross Kirsling]

On Thu, Jun 20, 2019 at 6:11 PM Frank Tang <ft...@chromium.org> wrote:

Is this feature controlled by a flag. 

I try to roll test262 and got a lot of errors related to this now.

It hasn't been landed yet, but it does depend on a test262 update.

(https://chromium-review.googlesource.com/c/v8/v8/+/1643171)

[Adam Klein]

On Fri, Jun 21, 2019 at 12:09 AM Yoav Weiss <yo...@yoav.ws> wrote:

On Wed, Jun 19, 2019 at 9:07 PM Adam Klein <ad...@chromium.org> wrote:

Hi Yoav,

We don't have existing metrics that would help here, and I honestly don't even know what we would track for this specific issue (detecting that a certain exception came from eval, for example, isn't something we currently track).

Bug reports are how we tend to track issues like this. And fwiw my intuition matches Sathya & Ross's (that this is very unlikely to cause problems in practice).

OK. Do you typically get those bug reports on Canary, Beta, or once the feature in question have hit stable?

(This is more of a generic question: I don't think this particular feature should block on putting counters in place if this is how V8 typically rolls. I just want to better understand your workflow regarding deprecations and changes)

I don't think this has been tracked systematically (the number of bug reports due to V8 web-exposed-API changes in the last couple years is pretty small, probably ~5-10), but I'd say Stable is most common, unfortunately.

Sathya has been working more actively in this area recently than I have, and might have other thoughts.

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAEvLGcKV93MirncPshaX9z29%3D5-0aBcecEOJ-Z%3D0zZE1%2BQb%3D7w%40mail.gmail.com.

[Sathya Gunasekaran]

On Mon, Jun 24, 2019 at 11:49 PM Adam Klein <ad...@chromium.org> wrote:
>
> On Fri, Jun 21, 2019 at 12:09 AM Yoav Weiss <yo...@yoav.ws> wrote:
>>
>> OK. Do you typically get those bug reports on Canary, Beta, or once the feature in question have hit stable?
>> (This is more of a generic question: I don't think this particular feature should block on putting counters in place if this is how V8 typically rolls. I just want to better understand your workflow regarding deprecations and changes)
>
>
> I don't think this has been tracked systematically (the number of bug reports due to V8 web-exposed-API changes in the last couple years is pretty small, probably ~5-10), but I'd say Stable is most common, unfortunately.
>
> Sathya has been working more actively in this area recently than I have, and might have other thoughts.
>

This matches my experience shipping recent breaking changes -- Stable
is the best bet, there's not much coverage from canary or beta.

Thanks,
--Sathya