Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Track line and column numbers for expressions #781

Merged
merged 2 commits into from
Jan 5, 2025

Conversation

bnoordhuis
Copy link
Contributor

Commit 73cc00e reduced the number of emitted source locations a great deal but it resulted in at least one observable regression:

export default async function f() {
     return "abc" + x
}
f() // ReferenceError should point to 2:20 but pointed to 1:1

Emit source locations for expressions again. Increases the average number of source locations by about 15%. Non-scientifically tested by counting source locations emitted when parsing the test suite before and after.

No test because we currently cannot easily test stack traces coming from module imports.

Fixes: #779

@@ -7,12 +7,12 @@ function test_exception_source_pos()
var e;

try {
throw new Error(""); // line 10, column 15
throw new Error(""); // line 10, column 19
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Back to what it was.

@bnoordhuis
Copy link
Contributor Author

Hrm, updating s->last_line_num in next_token() breaks ASI (so fragile!); only updating s->last_col_num works but is not a proper fix. Let me think about this one for a bit.

Commit 73cc00e reduced the number of emitted source locations a great
deal but it resulted in at least one observable regression:

    export default async function f() {
         return "abc" + x
    }
    f() // ReferenceError should point to 2:20 but pointed to 1:1

Emit source locations for expressions again. Increases the average
number of source locations by about 15%. Non-scientifically tested
by counting source locations emitted when parsing the test suite
before and after.

No test because we currently cannot easily test stack traces coming
from module imports.

Fixes: quickjs-ng#779
@bnoordhuis bnoordhuis merged commit ac4cd17 into quickjs-ng:master Jan 5, 2025
59 checks passed
@bnoordhuis bnoordhuis deleted the fix779 branch January 5, 2025 21:20
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression: Incorrect line number tracking
2 participants