Call-site columns on the first line of a script are one less than on other lines

Start in quickjs.c at the lexer’s token.col_num calculations and js_parse_init(); compare how line_start is tracked with the first-line behavior described here. Run the new tests/first-line-column.js regression test and the project’s api-test; the requested result is consistent, 1-based source columns on the first line, with the reported test passing. #1656 changes the same lines, so account for that overlapping work.

On the first line of a script, QuickJS reports a column one less than it would for the same code on any later line. This affects call sites and the other source positions taken from token.col_num, including Function.prototype.columnNumber.

$ printf 'let o = {}; o.missing();\n' > l1.js
$ printf '\nlet o = {}; o.missing();\n' > l2.js
$ qjs l1.js
TypeError: not a function
    at <anonymous> (l1.js:1:14)
$ qjs l2.js
TypeError: not a function
    at <anonymous> (l2.js:2:15)

missing starts at column 15 in both files. The caller frame of an ordinary call shows the same thing. function f() { throw new Error("x") } f(); puts the f() frame at 1:40 on the first line and at 2:41 on the second, and 41 is correct. The positions recorded for throw statements, expression statements and binary operators are off in the same way: var z = 0; throw null.x; gives 1:19 on the first line and 2:20 on the second. So is columnNumber for a function defined on the first line: (0, eval)(" function f() {} f").columnNumber is 2 instead of 3. ReferenceErrors for undefined globals and SyntaxErrors are not affected, because since #1585 and #1582 they compute the column from the line start.

This is with qjs built from the v0.17.0 release tarball. The code involved is unchanged on master (19dbe85).

The cause is in the lexer. token.col_num is computed as s->mark - s->eol, where eol points at the most recent newline character. From line 2 onward that gives a 1-based column. Before the first newline there is nothing for eol to point at, so js_parse_init() sets it to buf_ptr, and the same subtraction becomes 0-based. Two things hide the case of a token at the very start of the input: the max_int(1, ...) in next_token() and the mark = buf_ptr + min_int(1, input_len) initialisation. As a result the error only shows from column 2 onward.

Since #1630 the lexer also tracks line_start. Wherever the lexer consumes a line terminator, it sets eol to the terminator and line_start to the byte after it. So mark - line_start + 1 gives the same result as mark - eol on every line except the first, and on the first line it gives the correct column. js_parse_error() and the identifier case in js_parse_postfix_expr() already count columns from line_start in this way. After switching to it, nothing reads eol any more, so the patch below removes it. mark never falls below line_start, so the clamp is no longer needed either.

There is one other place where line_start and eol disagree today: a lone \r inside a block comment. There line_start moves past the \r but eol does not. line_num is not incremented there either, which is a separate issue. With the patch, token columns after such a comment are counted from the byte after the \r, which is how identifier and syntax-error columns are already counted.

There is one visible side effect. A function created with the Function constructor now reports columnNumber 2 instead of 1, because the source text the constructor builds starts with (function anonymous(.

The patch is against v0.17.0. It also applies to master, where the affected lines are at the same positions. It includes the output of make codegen: examples/hello.js has a call on its first line, so gen/hello.c changes, and the codegen check in CI would fail without it.

--- a/gen/hello.c
+++ b/gen/hello.c
@@ -5,7 +5,7 @@
 const uint32_t qjsc_hello_size = 108;
 
 const uint8_t qjsc_hello[108] = {
- 0x1c, 0x9f, 0x9e, 0xb6, 0xf5, 0x04, 0x01, 0x22,
+ 0x1c, 0x9f, 0xa0, 0xb6, 0x63, 0x04, 0x01, 0x22,
  0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x73,
  0x2f, 0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x2e, 0x6a,
  0x73, 0x01, 0x0e, 0x63, 0x6f, 0x6e, 0x73, 0x6f,
@@ -18,7 +18,7 @@
  0xf3, 0x00, 0x00, 0x00, 0x41, 0xf4, 0x00, 0x00,
  0x00, 0x04, 0xf5, 0x00, 0x00, 0x00, 0x24, 0x01,
  0x00, 0x0e, 0x06, 0x2f, 0xe4, 0x03, 0x01, 0x01,
- 0x02, 0x48, 0x0e, 0x00,
+ 0x02, 0x48, 0x10, 0x00,
 };
 
 static JSContext *JS_NewCustomContext(JSRuntime *rt)
--- a/quickjs.c
+++ b/quickjs.c
@@ -22423,8 +22423,7 @@
     const uint8_t *buf_ptr;
     const uint8_t *buf_end;
     const uint8_t *line_start; /* first character of the current line */
-    const uint8_t *eol;  // most recently seen end-of-line character
-    const uint8_t *mark; // first token character, invariant: eol < mark
+    const uint8_t *mark; // first token character, invariant: line_start <= mark
 
     /* current function code */
     JSFunctionDef *cur_func;
@@ -22685,7 +22684,6 @@
         if (c == '\n') {
             s->line_num++;
             s->line_start = p;
-            s->eol = &p[-1];
             s->mark = p;
         } else if (c >= 0x80) {
             c = utf8_decode(p - 1, &p_next);
@@ -22779,7 +22777,6 @@
                 if (sep != '`') {
                     s->line_num++;
                     s->line_start = p;
-                    s->eol = &p[-1];
                     s->mark = p;
                 }
                 continue;
@@ -23135,7 +23132,6 @@
         p++;
     line_terminator:
         s->line_start = p;
-        s->eol = &p[-1];
         s->mark = p;
         s->got_lf = true;
         s->line_num++;
@@ -23162,7 +23158,7 @@
                 if (*p == '\n') {
                     s->line_num++;
                     s->got_lf = true; /* considered as LF for ASI */
-                    s->eol = p++;
+                    p++;
                     s->line_start = p;
                     s->mark = p;
                 } else if (*p == '\r') {
@@ -23319,7 +23315,7 @@
             if (JS_VALUE_IS_NAN(ret) ||
                 lre_js_is_ident_next(utf8_decode(p, &p1))) {
                 JS_FreeValue(s->ctx, ret);
-                s->col_num = max_int(1, s->mark - s->eol);
+                s->col_num = s->mark - s->line_start + 1;
                 js_parse_error(s, "invalid number literal");
                 goto fail;
             }
@@ -23539,7 +23535,7 @@
         p++;
         break;
     }
-    s->token.col_num = max_int(1, s->mark - s->eol);
+    s->token.col_num = s->mark - s->line_start + 1;
     s->buf_ptr = p;
 
     //    dump_token(s, &s->token);
@@ -23781,7 +23777,7 @@
         /* fall thru */
     case '\n':
         s->line_num++;
-        s->eol = p++;
+        p++;
         s->line_start = p;
         s->mark = p;
         goto redo;
@@ -23861,7 +23857,7 @@
         p++;
         break;
     }
-    s->token.col_num = s->mark - s->eol;
+    s->token.col_num = s->mark - s->line_start + 1;
     s->buf_ptr = p;
 
     //    dump_token(s, &s->token);
@@ -25016,7 +25012,6 @@
     bool got_lf;
     const uint8_t *ptr;
     const uint8_t *line_start;
-    const uint8_t *eol;
     const uint8_t *mark;
 } JSParsePos;
 
@@ -25028,7 +25023,6 @@
     sp->col_num = s->token.col_num;
     sp->ptr = s->token.ptr;
     sp->line_start = s->line_start;
-    sp->eol = s->eol;
     sp->mark = s->mark;
     sp->got_lf = s->got_lf;
     return 0;
@@ -25042,7 +25036,6 @@
     s->col_num = sp->col_num;
     s->buf_ptr = sp->ptr;
     s->line_start = sp->line_start;
-    s->eol = sp->eol;
     s->mark = sp->mark;
     s->got_lf = sp->got_lf;
     return next_token(s);
@@ -38112,8 +38105,7 @@
     s->buf_start = s->buf_ptr = (const uint8_t *)input;
     s->buf_end = s->buf_ptr + input_len;
     s->line_start = s->buf_ptr;
-    s->mark = s->buf_ptr + min_int(1, input_len);
-    s->eol = s->buf_ptr;
+    s->mark = s->buf_ptr;
     s->token.val = ' ';
     s->token.line_num = 1;
     s->token.col_num = 1;
--- /dev/null
+++ b/tests/first-line-column.js
@@ -0,0 +1,20 @@
+import { assert } from "./assert.js";
+
+/* Source positions on the first line of the input must be 1-based, like on
+   every other line. They are exposed through an error's stack trace and
+   through Function.prototype.columnNumber. */
+function errorColumn(src) {
+    try {
+        (0, eval)(src);
+    } catch (e) {
+        const m = /<input>:(\d+):(\d+)/.exec(e.stack);
+        if (m)
+            return +m[2];
+    }
+    return -1;
+}
+
+assert(errorColumn("let o = {}; o.missing()"), 15);
+assert(errorColumn("\nlet o = {}; o.missing()"), 15);
+assert(errorColumn("  null.f()"), 3);
+assert((0, eval)("  function f() {} f").columnNumber, 3);

With the patch, run-test262 -c tests.conf and api-test pass. Without it, the new test fails with got |14|, expected |15|. I did not run test262 locally. The change only affects the column numbers recorded for tokens.

#1656 changes the same lines. From reading its diff, it adds the column offset on top of mark - eol. With that PR as it stands, a call site on the first line would still be one column before an identifier on the same line. Whichever change lands first, the other will need a small rebase.

I'm happy to open this as a PR if that is easier.