Fix columns being off by one / implement line cache (#1174)

AssemblyScript · Mar 15, 2020 · 960890e · 960890e
1 parent 0910982
commit 960890e
Show file tree

Hide file tree

Showing 188 changed files with 13,698 additions and 13,664 deletions.
diff --git a/src/ast.ts b/src/ast.ts
@@ -1718,6 +1718,46 @@ export class Source extends Node {
     var kind = this.sourceKind;
     return kind == SourceKind.LIBRARY || kind == SourceKind.LIBRARY_ENTRY;
   }
+
+  /** Cached line starts. */
+  private lineCache: i32[] | null = null;
+
+  /** Rememberd column number. */
+  private lineColumn: i32 = 0;
+
+  /** Determines the line number at the specified position. */
+  lineAt(pos: i32): i32 {
+    assert(pos >= 0 && pos < 0x7fffffff);
+    var lineCache = this.lineCache;
+    if (!lineCache) {
+      this.lineCache = lineCache = [0];
+      let text = this.text;
+      let off = 0;
+      let end = text.length;
+      while (off < end) {
+        if (text.charCodeAt(off++) == CharCode.LINEFEED) lineCache.push(off);
+      }
+      lineCache.push(0x7fffffff);
+    }
+    var l = 0;
+    var r = lineCache.length - 1;
+    while (l < r) {
+      let m = l + ((r - l) >> 1);
+      let s = unchecked(lineCache[m]);
+      if (pos < s) r = m;
+      else if (pos < unchecked(lineCache[m + 1])) {
+        this.lineColumn = pos - s + 1;
+        return m + 1;
+      }
+      else l = m + 1;
+    }
+    return assert(0);
+  }
+
+  /** Gets the column number at the last position queried with `lineAt`. */
+  columnAt(): i32 {
+    return this.lineColumn;
+  }
 }
 
 /** Base class of all declaration statements. */

diff --git a/src/compiler.ts b/src/compiler.ts
@@ -10091,13 +10091,15 @@ export class Compiler extends DiagnosticEmitter {
     }
 
     var filenameArg = this.ensureStaticString(codeLocation.range.source.normalizedPath);
+    var range = codeLocation.range;
+    var source = range.source;
     return module.block(null, [
       module.call(
         abortInstance.internalName, [
           messageArg,
           filenameArg,
-          module.i32(codeLocation.range.line),
-          module.i32(codeLocation.range.column)
+          module.i32(source.lineAt(range.start)),
+          module.i32(source.columnAt())
         ],
         NativeType.None
       ),

diff --git a/src/diagnostics.ts b/src/diagnostics.ts
@@ -128,18 +128,19 @@ export class DiagnosticMessage {
   toString(): string {
     var range = this.range;
     if (range) {
+      let source = range.source;
       return (
         diagnosticCategoryToString(this.category) +
         " " +
         this.code.toString() +
         ": \"" +
         this.message +
         "\" in " +
-        range.source.normalizedPath +
+        source.normalizedPath +
         ":" +
-        range.line.toString() +
+        source.lineAt(range.start).toString() +
         ":" +
-        range.column.toString()
+        source.columnAt().toString()
       );
     }
     return (
@@ -172,6 +173,7 @@ export function formatDiagnosticMessage(
   // include range information if available
   var range = message.range;
   if (range) {
+    let source = range.source;
 
     // include context information if requested
     if (showContext) {
@@ -180,26 +182,27 @@ export function formatDiagnosticMessage(
     }
     sb.push("\n");
     sb.push(" in ");
-    sb.push(range.source.normalizedPath);
+    sb.push(source.normalizedPath);
     sb.push("(");
-    sb.push(range.line.toString());
+    sb.push(source.lineAt(range.start).toString());
     sb.push(",");
-    sb.push(range.column.toString());
+    sb.push(source.columnAt().toString());
     sb.push(")");
 
     let relatedRange = message.relatedRange;
     if (relatedRange) {
+      let relatedSource = relatedRange.source;
       if (showContext) {
         sb.push("\n");
         sb.push(formatDiagnosticContext(relatedRange, useColors));
       }
       sb.push("\n");
       sb.push(" in ");
-      sb.push(relatedRange.source.normalizedPath);
+      sb.push(relatedSource.normalizedPath);
       sb.push("(");
-      sb.push(relatedRange.line.toString());
+      sb.push(relatedSource.lineAt(relatedRange.start).toString());
       sb.push(",");
-      sb.push(relatedRange.column.toString());
+      sb.push(relatedSource.columnAt().toString());
       sb.push(")");
     }
   }

diff --git a/src/parser.ts b/src/parser.ts
@@ -932,7 +932,8 @@ export class Parser extends DiagnosticEmitter {
     if ((flags & CommonFlags.DEFINITE_ASSIGNMENT) != 0 && initializer !== null) {
       this.error(
         DiagnosticCode.A_definite_assignment_assertion_is_not_permitted_in_this_context,
-        range);
+        range
+      );
     }
     return Node.createVariableDeclaration(
       identifier,
@@ -1775,11 +1776,11 @@ export class Parser extends DiagnosticEmitter {
     //   ('get' | 'set')?
     //   Identifier ...
 
-    var startPos = tn.pos;
     var isInterface = parent.kind == NodeKind.INTERFACEDECLARATION;
-
+    var startPos = 0;
     var decorators: DecoratorNode[] | null = null;
     if (tn.skip(Token.AT)) {
+      startPos = tn.tokenPos;
       do {
         let decorator = this.parseDecorator(tn);
         if (!decorator) break;
@@ -1812,6 +1813,7 @@ export class Parser extends DiagnosticEmitter {
       flags |= CommonFlags.PUBLIC;
       accessStart = tn.tokenPos;
       accessEnd = tn.pos;
+      if (!startPos) startPos = accessStart;
     } else if (tn.skip(Token.PRIVATE)) {
       if (isInterface) {
         this.error(
@@ -1822,6 +1824,7 @@ export class Parser extends DiagnosticEmitter {
       flags |= CommonFlags.PRIVATE;
       accessStart = tn.tokenPos;
       accessEnd = tn.pos;
+      if (!startPos) startPos = accessStart;
     } else if (tn.skip(Token.PROTECTED)) {
       if (isInterface) {
         this.error(
@@ -1832,6 +1835,7 @@ export class Parser extends DiagnosticEmitter {
       flags |= CommonFlags.PROTECTED;
       accessStart = tn.tokenPos;
       accessEnd = tn.pos;
+      if (!startPos) startPos = accessStart;
     }
 
     var staticStart = 0;
@@ -1848,6 +1852,7 @@ export class Parser extends DiagnosticEmitter {
       flags |= CommonFlags.STATIC;
       staticStart = tn.tokenPos;
       staticEnd = tn.pos;
+      if (!startPos) startPos = staticStart;
     } else {
       flags |= CommonFlags.INSTANCE;
       if (tn.skip(Token.ABSTRACT)) {
@@ -1860,6 +1865,7 @@ export class Parser extends DiagnosticEmitter {
         flags |= CommonFlags.ABSTRACT;
         abstractStart = tn.tokenPos;
         abstractEnd = tn.pos;
+        if (!startPos) startPos = abstractStart;
       }
       if (parent.flags & CommonFlags.GENERIC) flags |= CommonFlags.GENERIC_CONTEXT;
     }
@@ -1874,6 +1880,7 @@ export class Parser extends DiagnosticEmitter {
         flags |= CommonFlags.READONLY;
         readonlyStart = tn.tokenPos;
         readonlyEnd = tn.pos;
+        if (!startPos) startPos = readonlyStart;
       } else { // identifier
         tn.reset(state);
       }
@@ -1893,8 +1900,9 @@ export class Parser extends DiagnosticEmitter {
         if (tn.peek(true, IdentifierHandling.PREFER) == Token.IDENTIFIER && !tn.nextTokenOnNewLine) {
           flags |= CommonFlags.GET;
           isGetter = true;
-          setStart = tn.tokenPos;
-          setEnd = tn.pos;
+          getStart = tn.tokenPos;
+          getEnd = tn.pos;
+          if (!startPos) startPos = getStart;
           if (flags & CommonFlags.READONLY) {
             this.error(
               DiagnosticCode._0_modifier_cannot_be_used_here,
@@ -1910,6 +1918,7 @@ export class Parser extends DiagnosticEmitter {
           isSetter = true;
           setStart = tn.tokenPos;
           setEnd = tn.pos;
+          if (!startPos) startPos = setStart;
           if (flags & CommonFlags.READONLY) {
             this.error(
               DiagnosticCode._0_modifier_cannot_be_used_here,
@@ -1922,6 +1931,7 @@ export class Parser extends DiagnosticEmitter {
       } else if (tn.skip(Token.CONSTRUCTOR)) {
         flags |= CommonFlags.CONSTRUCTOR;
         isConstructor = true;
+        if (!startPos) startPos = tn.tokenPos;
         if (flags & CommonFlags.STATIC) {
           this.error(
             DiagnosticCode._0_modifier_cannot_be_used_here,
@@ -1948,6 +1958,7 @@ export class Parser extends DiagnosticEmitter {
       name = Node.createConstructorExpression(tn.range());
     } else {
       if (!(isGetter || isSetter) && tn.skip(Token.OPENBRACKET)) {
+        if (!startPos) startPos = tn.tokenPos;
         // TODO: also handle symbols, which might have some of these modifiers
         if (flags & CommonFlags.PUBLIC) {
           this.error(
@@ -1997,6 +2008,7 @@ export class Parser extends DiagnosticEmitter {
         );
         return null;
       }
+      if (!startPos) startPos = tn.tokenPos;
       name = Node.createIdentifierExpression(tn.readIdentifier(), tn.range());
     }
     var typeParameters: TypeParameterNode[] | null = null;

diff --git a/src/program.ts b/src/program.ts
@@ -3106,13 +3106,14 @@ export class Function extends TypedElement {
     if (this.program.options.sourceMap) {
       let debugLocations = this.debugLocations;
       for (let i = 0, k = debugLocations.length; i < k; ++i) {
-        let debugLocation = debugLocations[i];
+        let range = debugLocations[i];
+        let source = range.source;
         module.setDebugLocation(
           ref,
-          debugLocation.debugInfoRef,
-          debugLocation.source.debugInfoIndex,
-          debugLocation.line,
-          debugLocation.column
+          range.debugInfoRef,
+          source.debugInfoIndex,
+          source.lineAt(range.start),
+          source.columnAt()
         );
       }
     }

diff --git a/src/tokenizer.ts b/src/tokenizer.ts
@@ -402,10 +402,7 @@ export class Range {
   source: Source;
   start: i32;
   end: i32;
-
-  // TODO: set these while tokenizing
-  // line: i32;
-  // column: i32;
+  debugInfoRef: usize = 0;
 
   constructor(source: Source, start: i32, end: i32) {
     this.source = source;
@@ -429,30 +426,9 @@ export class Range {
     return new Range(this.source, this.end, this.end);
   }
 
-  get line(): i32 {
-    var text = this.source.text;
-    var line = 1;
-    for (let pos = this.start; pos >= 0; --pos) {
-      if (text.charCodeAt(pos) == CharCode.LINEFEED) line++;
-    }
-    return line;
-  }
-
-  get column(): i32 {
-    var text = this.source.text;
-    var column = 0;
-    for (let pos = this.start - 1; pos >= 0; --pos) {
-      if (text.charCodeAt(pos) == CharCode.LINEFEED) break;
-      ++column;
-    }
-    return column;
-  }
-
   toString(): string {
     return this.source.text.substring(this.start, this.end);
   }
-
-  debugInfoRef: usize = 0;
 }
 
 /** Handler for intercepting comments while tokenizing. */

diff --git a/tests/compiler/abi.untouched.wat b/tests/compiler/abi.untouched.wat
@@ -37,7 +37,7 @@
    i32.const 0
    i32.const 32
    i32.const 32
-   i32.const 2
+   i32.const 3
    call $~lib/builtins/abort
    unreachable
   end
@@ -74,7 +74,7 @@
    i32.const 0
    i32.const 32
    i32.const 45
-   i32.const 2
+   i32.const 3
    call $~lib/builtins/abort
    unreachable
   end
@@ -103,7 +103,7 @@
    i32.const 0
    i32.const 32
    i32.const 58
-   i32.const 2
+   i32.const 3
    call $~lib/builtins/abort
    unreachable
   end
@@ -120,7 +120,7 @@
    i32.const 0
    i32.const 32
    i32.const 65
-   i32.const 2
+   i32.const 3
    call $~lib/builtins/abort
    unreachable
   end
@@ -135,7 +135,7 @@
    i32.const 0
    i32.const 32
    i32.const 72
-   i32.const 2
+   i32.const 3
    call $~lib/builtins/abort
    unreachable
   end
@@ -150,7 +150,7 @@
    i32.const 0
    i32.const 32
    i32.const 74
-   i32.const 2
+   i32.const 3
    call $~lib/builtins/abort
    unreachable
   end
@@ -163,7 +163,7 @@
    i32.const 0
    i32.const 32
    i32.const 77
-   i32.const 2
+   i32.const 3
    call $~lib/builtins/abort
    unreachable
   end
@@ -176,7 +176,7 @@
    i32.const 0
    i32.const 32
    i32.const 79
-   i32.const 2
+   i32.const 3
    call $~lib/builtins/abort
    unreachable
   end