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

Improve line:column tracking #660

Merged
merged 2 commits into from
Nov 7, 2024
Merged

Conversation

bnoordhuis
Copy link
Contributor

Emit source locations manually for more precise tracking. Don't infer them from emitted bytecode opcodes because that leads to inaccurate and sometimes surprising results.

Speeds up code generation (although infinitesimally) as a bonus.

Fixes: #236


I initially planned to open this as a draft but in my (admittedly light) testing it works remarkably well. We now emit the exact same stack trace for the test case from #236.

Emit source locations manually for more precise tracking. Don't infer
them from emitted bytecode opcodes because that leads to inaccurate
and sometimes surprising results.

Speeds up code generation (although infinitesimally) as a bonus.

Fixes: quickjs-ng#236
@@ -7,12 +7,12 @@ function test_exception_source_pos()
var e;

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

Choose a reason for hiding this comment

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

This change is because we now point to new instead of Error, like V8 and other engines do.

Copy link
Contributor

@saghul saghul left a comment

Choose a reason for hiding this comment

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

Really, this is it?! Awesome! 👏 I think you can probably close the other PR too.

@bnoordhuis bnoordhuis merged commit 73cc00e into quickjs-ng:master Nov 7, 2024
47 checks passed
@bnoordhuis bnoordhuis deleted the fix236 branch November 7, 2024 21:03
@ammarahm-ed
Copy link

Thank you Ben! 💥

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 this pull request may close these issues.

Incorrect column number in stack traces repro
3 participants