Skip to content

Commit

Permalink
Track line and column numbers for expressions
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bnoordhuis committed Jan 2, 2025
1 parent 99c02eb commit 218630f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 6 deletions.
3 changes: 3 additions & 0 deletions quickjs.c
Original file line number Diff line number Diff line change
Expand Up @@ -19928,6 +19928,8 @@ static __exception int next_token(JSParseState *s)
}
s->token.col_num = max_int(1, s->mark - s->eol);
s->buf_ptr = p;
s->last_line_num = s->token.line_num;
s->last_col_num = s->token.col_num;

// dump_token(s, &s->token);
return 0;
Expand Down Expand Up @@ -24436,6 +24438,7 @@ static __exception int js_parse_expr_binary(JSParseState *s, int level,
}
if (next_token(s))
return -1;
emit_source_loc(s);
if (js_parse_expr_binary(s, level - 1, parse_flags))
return -1;
emit_op(s, opcode);
Expand Down
12 changes: 6 additions & 6 deletions tests/test_builtin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
} catch(_e) {
e = _e;
}

assert(e.stack.includes("test_builtin.js:10:15"));
assert(e.stack.includes("test_builtin.js:10:19"));
}

// Keep this at the top; it tests source positions.
Expand All @@ -36,7 +36,7 @@ function test_exception_prepare_stack()
};

try {
throw new Error(""); // line 39, column 15
throw new Error(""); // line 39, column 19
} catch(_e) {
e = _e;
}
Expand All @@ -48,7 +48,7 @@ function test_exception_prepare_stack()
assert(f.getFunctionName(), 'test_exception_prepare_stack');
assert(f.getFileName().endsWith('test_builtin.js'));
assert(f.getLineNumber(), 39);
assert(f.getColumnNumber(), 15);
assert(f.getColumnNumber(), 19);
assert(!f.isNative());
}

Expand All @@ -64,7 +64,7 @@ function test_exception_stack_size_limit()
};

try {
throw new Error(""); // line 67, column 15
throw new Error(""); // line 67, column 19
} catch(_e) {
e = _e;
}
Expand All @@ -77,7 +77,7 @@ function test_exception_stack_size_limit()
assert(f.getFunctionName(), 'test_exception_stack_size_limit');
assert(f.getFileName().endsWith('test_builtin.js'));
assert(f.getLineNumber(), 67);
assert(f.getColumnNumber(), 15);
assert(f.getColumnNumber(), 19);
assert(!f.isNative());
}

Expand Down

0 comments on commit 218630f

Please sign in to comment.