Skip to content
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

Closed
bergus opened this issue Sep 14, 2016 · 18 comments · Fixed by #1527
Closed

Wording for early reference errors #691

bergus opened this issue Sep 14, 2016 · 18 comments · Fixed by #1527

Comments

@bergus
Copy link

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

@domenic
Copy link
Member

domenic commented Sep 14, 2016

I like (3) personally.

@allenwb
Copy link
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
Copy link
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
Copy link
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
Copy link
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
Copy link
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
Copy link
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.

@bergus
Copy link
Author

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

@allenwb
Copy link
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
Copy link
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.)

@rkirsling
Copy link
Member

rkirsling commented Mar 10, 2019

This came up on IRC today, so I'll add a couple of data points:

  • Within the 40 "Static Semantics: Early Errors" sections, there are only four ReferenceError cases (0++, ++0, 0 = 0, and 0 += 0) and they all boil down to "AssignmentTargetType is invalid".

  • Engine status:

early ReferenceError early SyntaxError late ReferenceError
UpdateExpression V8, Ch SM JSC
AssignmentExpression V8, Ch, SM JSC

I think it's worth asking whether the notion of "early ReferenceError" really carries its own weight (though I might have assumed it too controversial to suggest had @bakkot not brought it up 😄).

@bergus
Copy link
Author

bergus commented Mar 11, 2019

@rkirsling It came up lately in a StackOverflow question that there are also some syntactical errors in destructuring patterns that are reported as ReferenceErrors.

@rkirsling
Copy link
Member

rkirsling commented Mar 11, 2019

@bergus Reading over section 12.15 on AssignmentExpression, I believe it's a V8 bug that ({a}) = {} and ([a]) = [] are being viewed as destructuring at all, as:

  1. This still applies:

    It is an early Reference Error if LeftHandSideExpression is neither an ObjectLiteral nor an ArrayLiteral and AssignmentTargetType of LeftHandSideExpression is invalid.

    (The LeftHandSideExpression is a ParenthesizedExpression, which has the AssignmentTargetType of the expression it contains, which for ObjectLiteral and ArrayLiteral is invalid.)

  2. The refinement grammar for AssignmentPattern suggests that destructuring brackets/braces are never wrapped in parentheses.

So those two should be the same case as 0 = 0, but we evidently don't have Test262 tests for them.


Note though, that ({0} = {}) and ([0] = []) are both SyntaxErrors—the former is "unexpected token" since {0} is an invalid ObjectLiteral, but the latter is "invalid destructuring assignment target" by the refinement grammar (which is surprising given that "invalid assignment target" is a ReferenceError!).

@anba
Copy link
Contributor

anba commented Mar 11, 2019

(FYI: SpiderMonkey is intentionally not fixing 0++ to throw an early ReferenceError → https://bugzilla.mozilla.org/show_bug.cgi?id=1340307)

@bergus
Copy link
Author

bergus commented Mar 11, 2019

@rkirsling Oops I didn't get what you meant by "AssignmentTargetType is invalid", I was studying ES2018 which only had IsSimpleAssignmentTarget. Sorry for the misunderstanding, yes I now agree that they all boil down to the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants