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

Wording for early reference errors #691

Open
bergus opened this Issue Sep 14, 2016 · 13 comments

Comments

Projects
None yet
5 participants
@bergus

bergus commented Sep 14, 2016

The ES6 and ES7 specs use the phrasing

It is an early Reference Error if …

which is confusing, especially the link target. Does it mean that an error (any error) about references should be thrown, or does this specifically mean that "a ReferenceError" should be thrown?
The phrase appears four times in the static semantics of prefix, postfix, normal and composite assignment operators.


Not sure whether this is the right channel for reporting, but https://bugs.ecmascript.org/ seems to be abandoned (and the certificate has expired).

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Sep 14, 2016

Member

@bergus you're in the right place. It means a ReferenceError. The link is bogus. Could fix this in a couple ways, not sure which is best as yet:

  1. Stop auto-linking Reference
  2. Add a dfn for Reference Error and Syntax Error
  3. Always refer to errors using their "class" name (ReferenceError, SyntaxError, etc.)
Member

bterlson commented Sep 14, 2016

@bergus you're in the right place. It means a ReferenceError. The link is bogus. Could fix this in a couple ways, not sure which is best as yet:

  1. Stop auto-linking Reference
  2. Add a dfn for Reference Error and Syntax Error
  3. Always refer to errors using their "class" name (ReferenceError, SyntaxError, etc.)
@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Sep 14, 2016

Member

I like (3) personally.

Member

domenic commented Sep 14, 2016

I like (3) personally.

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Sep 14, 2016

Member

The internal link is obviously bogus.

Whether a SyntaxError or a ReferenceError exception is thrown should only be observable via eval. So which should it be? Here is the history:

The concept of early reporting of of some Reference errors was introduced in the ES3 spec where chapter 16 said:

An implementation may treat any instance of the following kinds of runtime errors as a syntax error and
therefore report it early: ... Attempts to call PutValue on a value that is not a reference (for example, executing the assignment statement 3=4 ).

Note that a runtime, any such calls to PutValue would produce ReferenceError exceptions.

This language was retained in chapter 16 of ES5/ES5.1.

In ES6, explicit early error static semantic rules were added to associate such errors with the syntactic constructs that could generate them. For example12.14. Also the "early Reference Error" phrasing was introduced for describe such errors.

In ES3 and ES5, the specification of eval said: "If the parse fails, throw a SyntaxError exception (but see also clause 16)." The clause 16 reference is presumably to the language quoted above concerning PutValue.

In ES6, the eval spec was elaborated (step 3 of 18.2.1.1 to make it clear that early errors, in addition to parse failures are covered by that statement: "If the parse fails or any early errors are detected, throw a SyntaxError exception (but see also clause 16)." This could be interpreted to mean that an early Reference errors should be reported by eval as SyntaxError exceptions. But I recall the main intent of that language was to make sure that early errors, in general, should be propagated by eval as exceptions.

In ES 2016 step 3 of 18.2.1.1 that statement was changed to say "If any early errors are detected, throw a SyntaxError or a ReferenceError exception, depending on the type of the error".

I'm not sure why that change was made, there is probably a related issue that could be tracked down. It seems like a breaking change, if you read of ES3-ES6 to say that eval throws SyntaxError for all early errors. That's how I read it. However, its possible the change was made to reflect an implementation reality.

Personally, I think it would be better if eval reported all early errors as SyntaxError exceptions.

Member

allenwb commented Sep 14, 2016

The internal link is obviously bogus.

Whether a SyntaxError or a ReferenceError exception is thrown should only be observable via eval. So which should it be? Here is the history:

The concept of early reporting of of some Reference errors was introduced in the ES3 spec where chapter 16 said:

An implementation may treat any instance of the following kinds of runtime errors as a syntax error and
therefore report it early: ... Attempts to call PutValue on a value that is not a reference (for example, executing the assignment statement 3=4 ).

Note that a runtime, any such calls to PutValue would produce ReferenceError exceptions.

This language was retained in chapter 16 of ES5/ES5.1.

In ES6, explicit early error static semantic rules were added to associate such errors with the syntactic constructs that could generate them. For example12.14. Also the "early Reference Error" phrasing was introduced for describe such errors.

In ES3 and ES5, the specification of eval said: "If the parse fails, throw a SyntaxError exception (but see also clause 16)." The clause 16 reference is presumably to the language quoted above concerning PutValue.

In ES6, the eval spec was elaborated (step 3 of 18.2.1.1 to make it clear that early errors, in addition to parse failures are covered by that statement: "If the parse fails or any early errors are detected, throw a SyntaxError exception (but see also clause 16)." This could be interpreted to mean that an early Reference errors should be reported by eval as SyntaxError exceptions. But I recall the main intent of that language was to make sure that early errors, in general, should be propagated by eval as exceptions.

In ES 2016 step 3 of 18.2.1.1 that statement was changed to say "If any early errors are detected, throw a SyntaxError or a ReferenceError exception, depending on the type of the error".

I'm not sure why that change was made, there is probably a related issue that could be tracked down. It seems like a breaking change, if you read of ES3-ES6 to say that eval throws SyntaxError for all early errors. That's how I read it. However, its possible the change was made to reflect an implementation reality.

Personally, I think it would be better if eval reported all early errors as SyntaxError exceptions.

@jmdyck

This comment has been minimized.

Show comment
Hide comment
@jmdyck

jmdyck Sep 14, 2016

Collaborator

I like (3) personally.

In fact, it's a bit odd that we're not doing (3) already. The current style suggests that an error raised by an early error condition isn't necessarily an instance of the built-in SyntaxError or ReferenceError. (But e.g., ParseScript() and ParseModule() make it clear that it is.)

Collaborator

jmdyck commented Sep 14, 2016

I like (3) personally.

In fact, it's a bit odd that we're not doing (3) already. The current style suggests that an error raised by an early error condition isn't necessarily an instance of the built-in SyntaxError or ReferenceError. (But e.g., ParseScript() and ParseModule() make it clear that it is.)

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Sep 14, 2016

Member

@domenic The reason the phrasing "Syntax Error and Reference Error" was used is because these aren't runtime errors and hence may be detected in a context there it is not meaningful to talk about SyntaxError/ReferenceError exception object or the ES runtime mechanisms for throwing exceptions.

Member

allenwb commented Sep 14, 2016

@domenic The reason the phrasing "Syntax Error and Reference Error" was used is because these aren't runtime errors and hence may be detected in a context there it is not meaningful to talk about SyntaxError/ReferenceError exception object or the ES runtime mechanisms for throwing exceptions.

@jmdyck

This comment has been minimized.

Show comment
Hide comment
@jmdyck

jmdyck Sep 14, 2016

Collaborator

(Presumably you mean they aren't runtime errors.)

So do you think that it's not meaningful to say "one or more SyntaxError or ReferenceError objects" in ParseScript and ParseModule?

Collaborator

jmdyck commented Sep 14, 2016

(Presumably you mean they aren't runtime errors.)

So do you think that it's not meaningful to say "one or more SyntaxError or ReferenceError objects" in ParseScript and ParseModule?

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Sep 14, 2016

Member

@jmdyck In ES6 ParseModule was careful to not talk about ES exception objects in specifying how parse time errors are represented: "Otherwise, let body be an indication of one or more parsing errors and/or early errors. If more than one parse or early error is present, the number and ordering of reported errors is implementation dependent."

In ES2016, ParseScript was added and shares common language with ParseModule that constrains an implementation to use ES exception objects to internally communicate parse time errors: "Otherwise, let body be a List of one or more SyntaxError or ReferenceError objects representing the parsing errors and/or early errors. Parsing and early error detection may be interweaved in an implementation dependent manner. If more than one parsing error or early error is present, the number and ordering of error objects in the list is implementation dependent, but at least one must be present."

To me this is a regression. There is no need to imply that parsing operates in an environment where it is meaningful to talk about ES exception objects. Nor is there any need to require that such exception object are the vehicle used to communicate error conditions to the host environment via HostReportErrors. Such design decisions are not observable and better left to the implementations.

Member

allenwb commented Sep 14, 2016

@jmdyck In ES6 ParseModule was careful to not talk about ES exception objects in specifying how parse time errors are represented: "Otherwise, let body be an indication of one or more parsing errors and/or early errors. If more than one parse or early error is present, the number and ordering of reported errors is implementation dependent."

In ES2016, ParseScript was added and shares common language with ParseModule that constrains an implementation to use ES exception objects to internally communicate parse time errors: "Otherwise, let body be a List of one or more SyntaxError or ReferenceError objects representing the parsing errors and/or early errors. Parsing and early error detection may be interweaved in an implementation dependent manner. If more than one parsing error or early error is present, the number and ordering of error objects in the list is implementation dependent, but at least one must be present."

To me this is a regression. There is no need to imply that parsing operates in an environment where it is meaningful to talk about ES exception objects. Nor is there any need to require that such exception object are the vehicle used to communicate error conditions to the host environment via HostReportErrors. Such design decisions are not observable and better left to the implementations.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Sep 14, 2016

Member

I feel like eval is a good justification for always talking about these errors as runtime values. I don't think we want to change all eval errors to syntax errors (reference errors thrown out of eval make more sense and also broadly implemented). Also, while ParseModule might have tried to not talk about errors as if they were objects, the note in 16.1 already says something to this effect.

What about we do (3) above but make clear (somewhere, but not sure where yet) that for the purposes of early errors, actual runtime values need not be produced. I think this is obvious as the general rule of thumb of "if it's not observable, you don't have to do it" applies but it can't hurt to be explicit.

Member

bterlson commented Sep 14, 2016

I feel like eval is a good justification for always talking about these errors as runtime values. I don't think we want to change all eval errors to syntax errors (reference errors thrown out of eval make more sense and also broadly implemented). Also, while ParseModule might have tried to not talk about errors as if they were objects, the note in 16.1 already says something to this effect.

What about we do (3) above but make clear (somewhere, but not sure where yet) that for the purposes of early errors, actual runtime values need not be produced. I think this is obvious as the general rule of thumb of "if it's not observable, you don't have to do it" applies but it can't hurt to be explicit.

@bergus

This comment has been minimized.

Show comment
Hide comment
@bergus

bergus Sep 14, 2016

I'm in favour of (3) as well.

@allenwb thanks for the history writeup, that's what I figured as well. My investigation actually was triggered by a StackOverflow question about the implementation reality which suggests that there is no consistency anyway. Actually I would prefer if early "compile time" errors would always throw a SyntaxError, since what does never compile would be considered syntactically wrong (or if you'd argue that it's only semantically wrong, we'd need a new SemanticError).

bergus commented Sep 14, 2016

I'm in favour of (3) as well.

@allenwb thanks for the history writeup, that's what I figured as well. My investigation actually was triggered by a StackOverflow question about the implementation reality which suggests that there is no consistency anyway. Actually I would prefer if early "compile time" errors would always throw a SyntaxError, since what does never compile would be considered syntactically wrong (or if you'd argue that it's only semantically wrong, we'd need a new SemanticError).

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Sep 14, 2016

Member

I feel like eval is a good justification for always talking about these errors as runtime values.
PerformEval already has the language of explicitly generating appropriate runtime exception object values .

But I see no reason, why we would want to specify that the communications between "parsing" and HostReportErrors is done using ECMAScript language values. This is strictly an implementation level interface and there is noting gained by over-specifying it. In fact it hurts the clean layering of the specification. (In particular, I think it is important to avoid things that my be interpreted as precluding ahead of time compilation or linking).

My reading of the NOTE in 16.1 is that it requires the use of SyntaxError or ReferenceError objects to represent parsing errors and specified errors. (BTW, if that is indeed a requirement, it shouldn't be specified using an informative note).

Finally, an time we have actual exception objects we have to take into account the realm association of the objects. Consider an implementation where the parser is self-hosted implemented using ES code running in its own distinct realm. A implementor, based upon the requirement that the parser produces SyntaxError objects, might decide that they can just throw SyntaxError and that it will bubble through PerformEval and eventually to the user code that called eval. But if they do that, the user code if be getting a foreign realm exception object.

Member

allenwb commented Sep 14, 2016

I feel like eval is a good justification for always talking about these errors as runtime values.
PerformEval already has the language of explicitly generating appropriate runtime exception object values .

But I see no reason, why we would want to specify that the communications between "parsing" and HostReportErrors is done using ECMAScript language values. This is strictly an implementation level interface and there is noting gained by over-specifying it. In fact it hurts the clean layering of the specification. (In particular, I think it is important to avoid things that my be interpreted as precluding ahead of time compilation or linking).

My reading of the NOTE in 16.1 is that it requires the use of SyntaxError or ReferenceError objects to represent parsing errors and specified errors. (BTW, if that is indeed a requirement, it shouldn't be specified using an informative note).

Finally, an time we have actual exception objects we have to take into account the realm association of the objects. Consider an implementation where the parser is self-hosted implemented using ES code running in its own distinct realm. A implementor, based upon the requirement that the parser produces SyntaxError objects, might decide that they can just throw SyntaxError and that it will bubble through PerformEval and eventually to the user code that called eval. But if they do that, the user code if be getting a foreign realm exception object.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Sep 14, 2016

Member

I'd be curious to see a PR for fixing the ParseModule/ParseScript regression, but I'm skeptical that it helps clarity to have two types of errors in the spec.

But I don't understand where the over-specification is. Do you see ParseModule/ParseScript as specifying what is returned by the parser? From my read the parser is invoked in step 2 and we create (perhaps needlessly) the error objects representing any errors it returns. (It's probably a good idea to say here that the error's realm must be set to realm.)

Member

bterlson commented Sep 14, 2016

I'd be curious to see a PR for fixing the ParseModule/ParseScript regression, but I'm skeptical that it helps clarity to have two types of errors in the spec.

But I don't understand where the over-specification is. Do you see ParseModule/ParseScript as specifying what is returned by the parser? From my read the parser is invoked in step 2 and we create (perhaps needlessly) the error objects representing any errors it returns. (It's probably a good idea to say here that the error's realm must be set to realm.)

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Sep 14, 2016

Member

@bterlson Your right that the actual creation of the exception objects occurs in ParseModule/ParseScript rather than specify what is return by the parser. But a List of those exceptions is what is passed to HostReportError. Doesn't that limit want can be passed to HostReportError to what can be represented using standard instances of SyntaxError and ReferenceError? In most implementations it is probably desirable to pass a much richer set of information about detected errors from ParseModule/ParseScript to the "host". Why limit that implementation-specific channel to SyntaxError and ReferenceError instances.

As to language for fixing the ParseModule/Script regress. I suggest the language originally used ES2015's ParseModule:

Otherwise, let body be an indication of one or more parsing errors and/or early errors. Parsing and early error detection may be interweaved in an implementation dependent manner. If more than one parse or early error is present, the number and ordering of reported errors is implementation dependent but at least one error must be reported.

However, I'd replace the word "indication" with "implementation dependent representation"

Member

allenwb commented Sep 14, 2016

@bterlson Your right that the actual creation of the exception objects occurs in ParseModule/ParseScript rather than specify what is return by the parser. But a List of those exceptions is what is passed to HostReportError. Doesn't that limit want can be passed to HostReportError to what can be represented using standard instances of SyntaxError and ReferenceError? In most implementations it is probably desirable to pass a much richer set of information about detected errors from ParseModule/ParseScript to the "host". Why limit that implementation-specific channel to SyntaxError and ReferenceError instances.

As to language for fixing the ParseModule/Script regress. I suggest the language originally used ES2015's ParseModule:

Otherwise, let body be an indication of one or more parsing errors and/or early errors. Parsing and early error detection may be interweaved in an implementation dependent manner. If more than one parse or early error is present, the number and ordering of reported errors is implementation dependent but at least one error must be reported.

However, I'd replace the word "indication" with "implementation dependent representation"

@jmdyck

This comment has been minimized.

Show comment
Hide comment
@jmdyck

jmdyck Feb 11, 2017

Collaborator

I'd be curious to see a PR for fixing the ParseModule/ParseScript regression, but I'm skeptical that it helps clarity to have two types of errors in the spec.

I'm skeptical too, but if we were going to, it could look something like this:

Create another specification type. Call it 'Failure' (say), to better distinguish it from the 'Error' language type. It wouldn't have any specific data model, we'd just say it's implementation-dependent.

  • In Early Error rules, "Syntax Error" and "Reference Error" would be understood (or reworded) to be talking about Failures.

  • In ParseScript and Parse Module, the "parse" step sets body to either a Parse Node or a (non-empty) list of Failures (not ES Error objects). In the latter case, ParseScript and ParseModule also return this list of Failures.

  • A List of Failures is also what's passed to HostReportErrors() in ScriptEvaluationJob and TopLevelModuleEvaluationJob.

  • PerformEval's "parse" step could be reworded slightly to mention Failures. Ditto CreateDynamicFunction.

One problem with this approach is that HostReportErrors is also invoked for 'runtime' errors: in RunJobs(), if a job returns an abrupt completion, its [[Value]] is passed to HostReportErrors. (That's typically an ES Error object, but in general any ES value.) So you could say that HostReportErrors' _errorList_ is either a List of Failures or a List of ES language values. (Alternatively, one could split HostReportErrors into 'compile-time' and a 'run-time' variants.)

Collaborator

jmdyck commented Feb 11, 2017

I'd be curious to see a PR for fixing the ParseModule/ParseScript regression, but I'm skeptical that it helps clarity to have two types of errors in the spec.

I'm skeptical too, but if we were going to, it could look something like this:

Create another specification type. Call it 'Failure' (say), to better distinguish it from the 'Error' language type. It wouldn't have any specific data model, we'd just say it's implementation-dependent.

  • In Early Error rules, "Syntax Error" and "Reference Error" would be understood (or reworded) to be talking about Failures.

  • In ParseScript and Parse Module, the "parse" step sets body to either a Parse Node or a (non-empty) list of Failures (not ES Error objects). In the latter case, ParseScript and ParseModule also return this list of Failures.

  • A List of Failures is also what's passed to HostReportErrors() in ScriptEvaluationJob and TopLevelModuleEvaluationJob.

  • PerformEval's "parse" step could be reworded slightly to mention Failures. Ditto CreateDynamicFunction.

One problem with this approach is that HostReportErrors is also invoked for 'runtime' errors: in RunJobs(), if a job returns an abrupt completion, its [[Value]] is passed to HostReportErrors. (That's typically an ES Error object, but in general any ES value.) So you could say that HostReportErrors' _errorList_ is either a List of Failures or a List of ES language values. (Alternatively, one could split HostReportErrors into 'compile-time' and a 'run-time' variants.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment