Proper source locations for parser errors.#4104
Conversation
erak
left a comment
There was a problem hiding this comment.
Could this be a bit more specific? Something like: "Use the tokens entire location in case of a parser error."
Otherwise it's looks good!
7f9479a to
305fc06
Compare
|
@bit-shift What about now? |
| } | ||
| // ---- | ||
| // ParserError: (25-25): enum with no members is not allowed. | ||
| // ParserError: (25-26): enum with no members is not allowed. |
There was a problem hiding this comment.
This seems to be short. emit foo { } seems to be longer than 2 characters.
There was a problem hiding this comment.
The source location of the error is the whole parser token at which the error is recognized. In this case this is only the closing bracket. enum foo { is still valid - only at the following } the parser can recognize that the enum is empty, so that's the reported error location.
I split this PR into two commits - the first one is the code changes and the second one is the update of the tests. If you compile only the first commit and run isoltest, it will mark all the source locations that are updated in red, so this way it is easy to inspect all new source locations.
One may argue that the location here should be enum foo { } entirely, but this would require major changes in parserError, resp. fatalParserError (they would have to be supplied with the source location as arguments, instead of merely using the location of the current token).
There was a problem hiding this comment.
I am not saying there's a bug in the changes of the PR, but rather an issue to be fixed in general.
Closes #3874.
It may be good to merge this quickly, since all future parser tests will be affected.