Skip to content

Regression: Incorrect line number tracking #779

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

Closed
richarddd opened this issue Jan 1, 2025 · 0 comments · Fixed by #781
Closed

Regression: Incorrect line number tracking #779

richarddd opened this issue Jan 1, 2025 · 0 comments · Fixed by #781
Labels
bug Something isn't working

Comments

@richarddd
Copy link
Contributor

Regression introduced by: #660

Line numbers are incorrectly reported

Reproducable

a.js

import a from "./b.mjs";
a();

b.js

export default async function () {
  return "abc" + a;
}

Output

qjs --unhandled-rejection ./a.mjs
Possibly unhandled promise rejection: ReferenceError: a is not defined
    at default (./b.mjs:1:1)
    at <anonymous> (./a.mjs:2:1)

Expectation (version 0.6.0, or parent commit c8be383)

qjs --unhandled-rejection ./a.mjs
Possibly unhandled promise rejection: ReferenceError: a is not defined
    at default (b.mjs:2:18)
    at <anonymous> (a.mjs:2:1)

Related issues:
DelSkayn/rquickjs#406

bnoordhuis added a commit to bnoordhuis/quickjs that referenced this issue Jan 2, 2025
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 added the bug Something isn't working label Jan 2, 2025
bnoordhuis added a commit to bnoordhuis/quickjs that referenced this issue Jan 5, 2025
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 added a commit to bnoordhuis/quickjs that referenced this issue Jan 5, 2025
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 added a commit that referenced this issue Jan 5, 2025
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants