-
Notifications
You must be signed in to change notification settings - Fork 143
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
Record source column positions #193
Conversation
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.
Very nice work! Random observation: I think we can safely go ahead and remove has_debug
since we remove the strip mode. It'll save us a whole bit!
quickjs.c
Outdated
@@ -18271,6 +18295,7 @@ static void free_token(JSParseState *s, JSToken *token) | |||
static void __attribute((unused)) dump_token(JSParseState *s, | |||
const JSToken *token) | |||
{ | |||
printf("%d:%d ", token->line_num, token->col_num); |
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.
Debug leftover or intentional?
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.
Intentional! And it's a debug helper in the first place.
quickjs.c
Outdated
@@ -18826,7 +18861,8 @@ static __exception int next_token(JSParseState *s) | |||
if (*p == '\n') { | |||
s->line_num++; | |||
s->got_lf = TRUE; /* considered as LF for ASI */ | |||
p++; | |||
s->eol = p++; |
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.
Any chance of simplifying the logic with got_lf now? I didn't look into it but there might be some overlap?
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.
Not easily, I think. I can go into detail but they mean/describe slightly different things.
And: - display them in stack traces - expose them as Function.prototype.columnNumber OP_line_num is renamed to OP_source_loc and the pc2line data structure is extended with the column number in zigzag encoding. The bytecode version number BC_VERSION is incremented because pc2line data is read and written by JS_ReadObject() and JS_WriteObject() when it is present. Fixes: quickjs-ng#149
Agreed. I'll follow up with another PR. |
And merge the debug struct into JSFunctionBytecode because it is now always present. Refs: quickjs-ng#193 (review)
And merge the debug struct into JSFunctionBytecode because it is now always present. Refs: quickjs-ng#193 (review)
And merge the debug struct into JSFunctionBytecode because it is now always present. Refs: #193 (review)
And merge the debug struct into JSFunctionBytecode because it is now always present. Refs: quickjs-ng/quickjs#193 (review)
And:
OP_line_num is renamed to OP_source_loc and the pc2line data structure is extended with the column number in zigzag encoding.
The bytecode version number BC_VERSION is incremented because pc2line data is read and written by JS_ReadObject() and JS_WriteObject() when it is present.
Fixes: #149