cmd/compile/internal/syntax: remove dependency on cmd/internal/src

For dependency reasons, the data structure implementing source
positions in the compiler is in cmd/internal/src. It contains
highly compiler specific details (e.g. inlining index).

This change introduces a parallel but simpler position
representation, defined in the syntax package, which removes
that package's dependency on cmd/internal/src, and also removes
the need to deal with certain filename-specific operations
(defined by the needs of the compiler) in the syntax package.
As a result, the syntax package becomes again a compiler-
independent, stand-alone package that at some point might
replace (or augment) the existing top-level go/* syntax-related
packages.

Additionally, line directives that update column numbers
are now correctly tracked through the syntax package, with
additional tests added. (The respective changes also need to
be made in cmd/internal/src; i.e., the compiler accepts but
still ignores column numbers in line directives.)

This change comes at the cost of a new position translation
step, but that step is cheap because it only needs to do real
work if the position base changed (i.e., if there is a new file,
or new line directive).

There is no noticeable impact on overall compiler performance
measured with `compilebench -count 5 -alloc`:

name       old time/op       new time/op       delta
Template         220ms ± 8%        228ms ±18%    ~     (p=0.548 n=5+5)
Unicode          119ms ±11%        113ms ± 5%    ~     (p=0.056 n=5+5)
GoTypes          684ms ± 6%        677ms ± 3%    ~     (p=0.841 n=5+5)
Compiler         3.19s ± 7%        3.01s ± 1%    ~     (p=0.095 n=5+5)
SSA              7.92s ± 8%        7.79s ± 1%    ~     (p=0.690 n=5+5)
Flate            141ms ± 7%        139ms ± 4%    ~     (p=0.548 n=5+5)
GoParser         173ms ±12%        171ms ± 4%    ~     (p=1.000 n=5+5)
Reflect          417ms ± 5%        411ms ± 3%    ~     (p=0.548 n=5+5)
Tar              205ms ± 5%        198ms ± 2%    ~     (p=0.690 n=5+5)
XML              232ms ± 4%        229ms ± 4%    ~     (p=0.690 n=5+5)
StdCmd           28.7s ± 5%        28.2s ± 2%    ~     (p=0.421 n=5+5)

name       old user-time/op  new user-time/op  delta
Template         269ms ± 4%        265ms ± 5%    ~     (p=0.421 n=5+5)
Unicode          153ms ± 7%        149ms ± 3%    ~     (p=0.841 n=5+5)
GoTypes          850ms ± 7%        862ms ± 4%    ~     (p=0.841 n=5+5)
Compiler         4.01s ± 5%        3.86s ± 0%    ~     (p=0.190 n=5+4)
SSA              10.9s ± 4%        10.8s ± 2%    ~     (p=0.548 n=5+5)
Flate            166ms ± 7%        167ms ± 6%    ~     (p=1.000 n=5+5)
GoParser         204ms ± 8%        206ms ± 7%    ~     (p=0.841 n=5+5)
Reflect          514ms ± 5%        508ms ± 4%    ~     (p=0.548 n=5+5)
Tar              245ms ± 6%        244ms ± 3%    ~     (p=0.690 n=5+5)
XML              280ms ± 4%        278ms ± 4%    ~     (p=0.841 n=5+5)

name       old alloc/op      new alloc/op      delta
Template        37.9MB ± 0%       37.9MB ± 0%    ~     (p=0.841 n=5+5)
Unicode         28.8MB ± 0%       28.8MB ± 0%    ~     (p=0.841 n=5+5)
GoTypes          113MB ± 0%        113MB ± 0%    ~     (p=0.151 n=5+5)
Compiler         468MB ± 0%        468MB ± 0%  -0.01%  (p=0.032 n=5+5)
SSA             1.50GB ± 0%       1.50GB ± 0%    ~     (p=0.548 n=5+5)
Flate           24.4MB ± 0%       24.4MB ± 0%    ~     (p=1.000 n=5+5)
GoParser        30.7MB ± 0%       30.7MB ± 0%    ~     (p=1.000 n=5+5)
Reflect         76.5MB ± 0%       76.5MB ± 0%    ~     (p=0.548 n=5+5)
Tar             38.9MB ± 0%       38.9MB ± 0%    ~     (p=0.222 n=5+5)
XML             41.6MB ± 0%       41.6MB ± 0%    ~     (p=0.548 n=5+5)

name       old allocs/op     new allocs/op     delta
Template          382k ± 0%         382k ± 0%  +0.01%  (p=0.008 n=5+5)
Unicode           343k ± 0%         343k ± 0%    ~     (p=0.841 n=5+5)
GoTypes          1.19M ± 0%        1.19M ± 0%  +0.01%  (p=0.008 n=5+5)
Compiler         4.53M ± 0%        4.53M ± 0%  +0.03%  (p=0.008 n=5+5)
SSA              12.4M ± 0%        12.4M ± 0%  +0.00%  (p=0.008 n=5+5)
Flate             235k ± 0%         235k ± 0%    ~     (p=0.079 n=5+5)
GoParser          318k ± 0%         318k ± 0%    ~     (p=0.730 n=5+5)
Reflect           978k ± 0%         978k ± 0%    ~     (p=1.000 n=5+5)
Tar               393k ± 0%         393k ± 0%    ~     (p=0.056 n=5+5)
XML               405k ± 0%         405k ± 0%    ~     (p=0.548 n=5+5)

name       old text-bytes    new text-bytes    delta
HelloSize        672kB ± 0%        672kB ± 0%    ~     (all equal)
CmdGoSize       7.12MB ± 0%       7.12MB ± 0%    ~     (all equal)

name       old data-bytes    new data-bytes    delta
HelloSize        133kB ± 0%        133kB ± 0%    ~     (all equal)
CmdGoSize        390kB ± 0%        390kB ± 0%    ~     (all equal)

name       old exe-bytes     new exe-bytes     delta
HelloSize       1.07MB ± 0%       1.07MB ± 0%    ~     (all equal)
CmdGoSize       11.2MB ± 0%       11.2MB ± 0%    ~     (all equal)

Passes toolstash compare.

For #22662.

Change-Id: I19edb53dd9675af57f7122cb7dba2a6d8bdcc3da
Reviewed-on: https://go-review.googlesource.com/94515
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

1 files changed, 55 insertions, 47 deletions

diff --git a/src/cmd/compile/internal/syntax/parser_test.go b/src/cmd/compile/internal/syntax/parser_test.go
index 592163bb51..cfac2e0118 100644
--- a/src/cmd/compile/internal/syntax/parser_test.go
+++ b/src/cmd/compile/internal/syntax/parser_test.go
@@ -6,7 +6,6 @@ package syntax
 
 import (
 	"bytes"
-	"cmd/internal/src"
 	"flag"
 	"fmt"
 	"io/ioutil"
@@ -131,7 +130,7 @@ func verifyPrint(filename string, ast1 *File) {
 		panic(err)
 	}
 
-	ast2, err := Parse(src.NewFileBase(filename, filename), &buf1, nil, nil, nil, 0)
+	ast2, err := Parse(NewFileBase(filename), &buf1, nil, nil, 0)
 	if err != nil {
 		panic(err)
 	}
@@ -155,7 +154,7 @@ func verifyPrint(filename string, ast1 *File) {
 }
 
 func TestIssue17697(t *testing.T) {
-	_, err := Parse(nil, bytes.NewReader(nil), nil, nil, nil, 0) // return with parser error, don't panic
+	_, err := Parse(nil, bytes.NewReader(nil), nil, nil, 0) // return with parser error, don't panic
 	if err == nil {
 		t.Errorf("no error reported")
 	}
@@ -181,6 +180,11 @@ func TestParseFile(t *testing.T) {
 	}
 }
 
+// Make sure (PosMax + 1) doesn't overflow when converted to default
+// type int (when passed as argument to fmt.Sprintf) on 32bit platforms
+// (see test cases below).
+var tooLarge int = PosMax + 1
+
 func TestLineDirectives(t *testing.T) {
 	// valid line directives lead to a syntax error after them
 	const valid = "syntax error: package statement must be first"
@@ -191,11 +195,11 @@ func TestLineDirectives(t *testing.T) {
 		line, col uint // 0-based
 	}{
 		// ignored //line directives
-		{"//\n", valid, "", 2 - linebase, 0},            // no directive
-		{"//line\n", valid, "", 2 - linebase, 0},        // missing colon
-		{"//line foo\n", valid, "", 2 - linebase, 0},    // missing colon
-		{"  //line foo:\n", valid, "", 2 - linebase, 0}, // not a line start
-		{"//  line foo:\n", valid, "", 2 - linebase, 0}, // space between // and line
+		{"//\n", valid, "", 1, 0},            // no directive
+		{"//line\n", valid, "", 1, 0},        // missing colon
+		{"//line foo\n", valid, "", 1, 0},    // missing colon
+		{"  //line foo:\n", valid, "", 1, 0}, // not a line start
+		{"//  line foo:\n", valid, "", 1, 0}, // space between // and line
 
 		// invalid //line directives with one colon
 		{"//line :\n", "invalid line number: ", "", 0, 8},
@@ -206,7 +210,7 @@ func TestLineDirectives(t *testing.T) {
 		{"//line foo:1 \n", "invalid line number: 1 ", "", 0, 11},
 		{"//line foo:-12\n", "invalid line number: -12", "", 0, 11},
 		{"//line C:foo:0\n", "invalid line number: 0", "", 0, 13},
-		{fmt.Sprintf("//line foo:%d\n", lineMax+1), fmt.Sprintf("invalid line number: %d", lineMax+1), "", 0, 11},
+		{fmt.Sprintf("//line foo:%d\n", tooLarge), fmt.Sprintf("invalid line number: %d", tooLarge), "", 0, 11},
 
 		// invalid //line directives with two colons
 		{"//line ::\n", "invalid line number: ", "", 0, 9},
@@ -217,27 +221,33 @@ func TestLineDirectives(t *testing.T) {
 
 		{"//line :123:0\n", "invalid column number: 0", "", 0, 12},
 		{"//line foo:123:0\n", "invalid column number: 0", "", 0, 15},
+		{fmt.Sprintf("//line foo:10:%d\n", tooLarge), fmt.Sprintf("invalid column number: %d", tooLarge), "", 0, 14},
 
-		// effect of valid //line directives on positions
+		// effect of valid //line directives on lines
 		{"//line foo:123\n   foo", valid, "foo", 123 - linebase, 3},
 		{"//line  foo:123\n   foo", valid, " foo", 123 - linebase, 3},
 		{"//line foo:123\n//line bar:345\nfoo", valid, "bar", 345 - linebase, 0},
 		{"//line C:foo:123\n", valid, "C:foo", 123 - linebase, 0},
-		{"//line " + runtime.GOROOT() + "/src/a/a.go:123\n   foo", valid, "$GOROOT/src/a/a.go", 123 - linebase, 3},
-		{"//line :x:1\n", valid, ":x", 0, 0},
-		{"//line foo ::1\n", valid, "foo :", 0, 0},
+		{"//line /src/a/a.go:123\n   foo", valid, "/src/a/a.go", 123 - linebase, 3},
+		{"//line :x:1\n", valid, ":x", 1 - linebase, 0},
+		{"//line foo ::1\n", valid, "foo :", 1 - linebase, 0},
 		{"//line foo:123abc:1\n", valid, "foo:123abc", 0, 0},
 		{"//line foo :123:1\n", valid, "foo ", 123 - linebase, 0},
 		{"//line ::123\n", valid, ":", 123 - linebase, 0},
 
-		// TODO(gri) add tests to verify correct column changes, once implemented
+		// effect of valid //line directives on columns
+		{"//line :x:1:10\n", valid, ":x", 1 - linebase, 10 - colbase},
+		{"//line foo ::1:2\n", valid, "foo :", 1 - linebase, 2 - colbase},
+		{"//line foo:123abc:1:1000\n", valid, "foo:123abc", 1 - linebase, 1000 - colbase},
+		{"//line foo :123:1000\n\n", valid, "foo ", 124 - linebase, 0},
+		{"//line ::123:1234\n", valid, ":", 123 - linebase, 1234 - colbase},
 
 		// ignored /*line directives
-		{"/**/", valid, "", 1 - linebase, 4},             // no directive
-		{"/*line*/", valid, "", 1 - linebase, 8},         // missing colon
-		{"/*line foo*/", valid, "", 1 - linebase, 12},    // missing colon
-		{"  //line foo:*/", valid, "", 1 - linebase, 15}, // not a line start
-		{"/*  line foo:*/", valid, "", 1 - linebase, 15}, // space between // and line
+		{"/**/", valid, "", 0, 4},             // no directive
+		{"/*line*/", valid, "", 0, 8},         // missing colon
+		{"/*line foo*/", valid, "", 0, 12},    // missing colon
+		{"  //line foo:*/", valid, "", 0, 15}, // not a line start
+		{"/*  line foo:*/", valid, "", 0, 15}, // space between // and line
 
 		// invalid /*line directives with one colon
 		{"/*line :*/", "invalid line number: ", "", 0, 8},
@@ -247,7 +257,7 @@ func TestLineDirectives(t *testing.T) {
 		{"/*line foo:0*/", "invalid line number: 0", "", 0, 11},
 		{"/*line foo:1 */", "invalid line number: 1 ", "", 0, 11},
 		{"/*line C:foo:0*/", "invalid line number: 0", "", 0, 13},
-		{fmt.Sprintf("/*line foo:%d*/", lineMax+1), fmt.Sprintf("invalid line number: %d", lineMax+1), "", 0, 11},
+		{fmt.Sprintf("/*line foo:%d*/", tooLarge), fmt.Sprintf("invalid line number: %d", tooLarge), "", 0, 11},
 
 		// invalid /*line directives with two colons
 		{"/*line ::*/", "invalid line number: ", "", 0, 9},
@@ -258,31 +268,27 @@ func TestLineDirectives(t *testing.T) {
 
 		{"/*line :123:0*/", "invalid column number: 0", "", 0, 12},
 		{"/*line foo:123:0*/", "invalid column number: 0", "", 0, 15},
+		{fmt.Sprintf("/*line foo:10:%d*/", tooLarge), fmt.Sprintf("invalid column number: %d", tooLarge), "", 0, 14},
 
-		// effect of valid /*line directives on positions
-		// TODO(gri) remove \n after directives once line number is computed correctly
-		{"/*line foo:123*/\n   foo", valid, "foo", 123 - linebase, 3},
+		// effect of valid /*line directives on lines
+		{"/*line foo:123*/   foo", valid, "foo", 123 - linebase, 3},
 		{"/*line foo:123*/\n//line bar:345\nfoo", valid, "bar", 345 - linebase, 0},
-		{"/*line C:foo:123*/\n", valid, "C:foo", 123 - linebase, 0},
-		{"/*line " + runtime.GOROOT() + "/src/a/a.go:123*/\n   foo", valid, "$GOROOT/src/a/a.go", 123 - linebase, 3},
-		{"/*line :x:1*/\n", valid, ":x", 1 - linebase, 0},
-		{"/*line foo ::1*/\n", valid, "foo :", 1 - linebase, 0},
-		{"/*line foo:123abc:1*/\n", valid, "foo:123abc", 1 - linebase, 0},
-		{"/*line foo :123:1*/\n", valid, "foo ", 123 - linebase, 0},
-		{"/*line ::123*/\n", valid, ":", 123 - linebase, 0},
-
-		// test effect of /*line directive on (relative) position information for this line
-		// TODO(gri) add these tests
-
-		// TODO(gri) add tests to verify correct column changes, once implemented
+		{"/*line C:foo:123*/", valid, "C:foo", 123 - linebase, 0},
+		{"/*line /src/a/a.go:123*/   foo", valid, "/src/a/a.go", 123 - linebase, 3},
+		{"/*line :x:1*/", valid, ":x", 1 - linebase, 0},
+		{"/*line foo ::1*/", valid, "foo :", 1 - linebase, 0},
+		{"/*line foo:123abc:1*/", valid, "foo:123abc", 1 - linebase, 0},
+		{"/*line foo :123:10*/", valid, "foo ", 123 - linebase, 10 - colbase},
+		{"/*line ::123*/", valid, ":", 123 - linebase, 0},
+
+		// effect of valid /*line directives on columns
+		{"/*line :x:1:10*/", valid, ":x", 1 - linebase, 10 - colbase},
+		{"/*line foo ::1:2*/", valid, "foo :", 1 - linebase, 2 - colbase},
+		{"/*line foo:123abc:1:1000*/", valid, "foo:123abc", 1 - linebase, 1000 - colbase},
+		{"/*line foo :123:1000*/\n", valid, "foo ", 124 - linebase, 0},
+		{"/*line ::123:1234*/", valid, ":", 123 - linebase, 1234 - colbase},
 	} {
-		fileh := func(name string) string {
-			if strings.HasPrefix(name, runtime.GOROOT()) {
-				return "$GOROOT" + name[len(runtime.GOROOT()):]
-			}
-			return name
-		}
-		_, err := Parse(nil, strings.NewReader(test.src), nil, nil, fileh, 0)
+		_, err := Parse(nil, strings.NewReader(test.src), nil, nil, 0)
 		if err == nil {
 			t.Errorf("%s: no error reported", test.src)
 			continue
@@ -295,14 +301,16 @@ func TestLineDirectives(t *testing.T) {
 		if msg := perr.Msg; msg != test.msg {
 			t.Errorf("%s: got msg = %q; want %q", test.src, msg, test.msg)
 		}
-		if filename := perr.Pos.AbsFilename(); filename != test.filename {
+
+		pos := perr.Pos
+		if filename := pos.RelFilename(); filename != test.filename {
 			t.Errorf("%s: got filename = %q; want %q", test.src, filename, test.filename)
 		}
-		if line := perr.Pos.RelLine(); line-linebase != test.line {
-			t.Errorf("%s: got line = %d; want %d", test.src, line-linebase, test.line)
+		if line := pos.RelLine(); line != test.line+linebase {
+			t.Errorf("%s: got line = %d; want %d", test.src, line, test.line+linebase)
 		}
-		if col := perr.Pos.Col(); col-colbase != test.col {
-			t.Errorf("%s: got col = %d; want %d", test.src, col-colbase, test.col)
+		if col := pos.RelCol(); col != test.col+colbase {
+			t.Errorf("%s: got col = %d; want %d", test.src, col, test.col+colbase)
 		}
 	}
 }