-
Notifications
You must be signed in to change notification settings - Fork 239
Improve column number tracking #406
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
Conversation
58d8f73 to
1a220e7
Compare
|
I'll defer this review to @bnoordhuis who worked on adding column numbers :-P |
cac9960 to
cfcc36d
Compare
- simplify column number tracking using a pointer to the beginning of line instead of `eol` + `mark`. - add `js_parse_error_pos` to report syntax errors with exact source position for token parsing errors. This makes the syntax error reports much more precise. eg: exact position of UTF-8 encoding error, invalid escape sequence, etc. - add `JSSourcePos` type to use single opaque object for token source position - add `emit_pos` to set the precise source position in code generation - change `emit_op` to no longer emit source positions from `s->last_line_num` and `s->last_col_num`. - remove `last_line_num` and `last_col_num` `JSParserState` members - runtime errors on calls report the column number of calling function or method name. - runtime errors on `new` expressions report the column number of the `neẁ` keyword. - do not show source position in backtrace if debug information is missing - fix spurious parsing bugs when `js_parse_skip_parens_token` could not reparse the current token because of stack overflow detection. - `js_parse_save_pos` now saves the current token and `js_parse_seek_back` always restores the token, hence never fails, while `js_parse_seek_token` reparses the saved token. This is needed to handle the weird semantics of `"\1"; "use strict";` - simplify html comment detection - update **tests/test_builtin,js** with more informative messages - improve `assert()` and **tests/test_language.js** tests - update **v8.txt** for updated column numbers in remaining errors
cfcc36d to
5946221
Compare
saghul
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a rebase, but it should be good to go otherwise. Sorry for the delay @chqrlie !
| === json-errors.js | ||
| Failure: | ||
| expected: | ||
| "Unexpected end of JSON input" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these new?
|
Is this expected to improve the issue in #236? |
Yes it should. Let me try this test after I fix the conflicts. |
|
Hi, I am working on a project that is blocked due to this, can we possibly get this merged anytime soon. |
|
@chqrlie are you planning on picking this back up? It likely needs a rebase at least 😅 |
|
I fixed this just now in #660. Please file a new issue if anything still doesn't work as expected. |
instead of
eol+mark.js_parse_error_posto report syntax errors with correct column numberfor token parsing errors. This makes the syntax error reports much more precise.
JSSourcePostype to keep track of token source positionemit_op_posto set the precise source position in code generationthis is work in progress: it should be the only way to generate source positions
and we should get rid of
s->last_line_numands->last_col_num.newexpressions report the column number of theneẁkeyword.assert()and tests/test_language.js tests