cmd/compile, cmd/compile/internal/syntax: print relative column info

This change enables printing of relative column information if a
prior line directive specified a valid column. If there was no
line directive, or the line directive didn't specify a column
(or the -C flag is specified), no column information is shown in
file positions.

Implementation: Column values (and line values, for that matter)
that are zero are interpreted as "unknown". A line directive that
doesn't specify a column records that as a zero column in the
respective PosBase data structure. When computing relative columns,
a relative value is zero of the base's column value is zero.
When formatting a position, a zero column value is not printed.

To make this work without special cases, the PosBase for a file
is given a concrete (non-0:0) position 1:1 with the PosBase's
line and column also being 1:1. In other words, at the position
1:1 of a file, it's relative positions are starting with 1:1 as
one would expect.

In the package syntax, this requires self-recursive PosBases for
file bases, matching what cmd/internal/src.PosBase was already
doing. In src.PosBase, file and inlining bases also need to be
based at 1:1 to indicate "known" positions.

This change completes the cmd/compiler part of the issue below.

Fixes #22662.

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

3 files changed, 153 insertions, 105 deletions

diff --git a/src/cmd/compile/internal/syntax/parser.go b/src/cmd/compile/internal/syntax/parser.go
index 3bed97b31c..db0fb39c8c 100644
--- a/src/cmd/compile/internal/syntax/parser.go
+++ b/src/cmd/compile/internal/syntax/parser.go
@@ -85,8 +85,8 @@ func (p *parser) init(file *PosBase, r io.Reader, errh ErrorHandler, pragh Pragm
 
 // updateBase sets the current position base to a new line base at pos.
 // The base's filename, line, and column values are extracted from text
-// which is positioned at (line, col) (only needed for error messages).
-func (p *parser) updateBase(pos Pos, line, col uint, text string) {
+// which is positioned at (tline, tcol) (only needed for error messages).
+func (p *parser) updateBase(pos Pos, tline, tcol uint, text string) {
 	i, n, ok := trailingDigits(text)
 	if i == 0 {
 		return // ignore (not a line directive)
@@ -95,38 +95,39 @@ func (p *parser) updateBase(pos Pos, line, col uint, text string) {
 
 	if !ok {
 		// text has a suffix :xxx but xxx is not a number
-		p.errorAt(p.posAt(line, col+i), "invalid line number: "+text[i:])
+		p.errorAt(p.posAt(tline, tcol+i), "invalid line number: "+text[i:])
 		return
 	}
 
+	var line, col uint
 	i2, n2, ok2 := trailingDigits(text[:i-1])
 	if ok2 {
 		//line filename:line:col
 		i, i2 = i2, i
-		n, n2 = n2, n
-		if n2 == 0 || n2 > PosMax {
-			p.errorAt(p.posAt(line, col+i2), "invalid column number: "+text[i2:])
+		line, col = n2, n
+		if col == 0 || col > PosMax {
+			p.errorAt(p.posAt(tline, tcol+i2), "invalid column number: "+text[i2:])
 			return
 		}
-		text = text[:i2-1] // lop off :col
+		text = text[:i2-1] // lop off ":col"
 	} else {
 		//line filename:line
-		n2 = colbase // use start of line for column
+		line = n
 	}
 
-	if n == 0 || n > PosMax {
-		p.errorAt(p.posAt(line, col+i), "invalid line number: "+text[i:])
+	if line == 0 || line > PosMax {
+		p.errorAt(p.posAt(tline, tcol+i), "invalid line number: "+text[i:])
 		return
 	}
 
 	// If we have a column (//line filename:line:col form),
 	// an empty filename means to use the previous filename.
-	filename := text[:i-1] // lop off :line
+	filename := text[:i-1] // lop off ":line"
 	if filename == "" && ok2 {
 		filename = p.base.Filename()
 	}
 
-	p.base = NewLineBase(pos, filename, n, n2)
+	p.base = NewLineBase(pos, filename, line, col)
 }
 
 func commentText(s string) string {
diff --git a/src/cmd/compile/internal/syntax/parser_test.go b/src/cmd/compile/internal/syntax/parser_test.go
index c253a5c345..3cf55defc7 100644
--- a/src/cmd/compile/internal/syntax/parser_test.go
+++ b/src/cmd/compile/internal/syntax/parser_test.go
@@ -193,113 +193,113 @@ func TestLineDirectives(t *testing.T) {
 	for _, test := range []struct {
 		src, msg  string
 		filename  string
-		line, col uint // 0-based
+		line, col uint // 1-based; 0 means unknown
 	}{
 		// ignored //line directives
-		{"//\n", valid, filename, 1, 0},            // no directive
-		{"//line\n", valid, filename, 1, 0},        // missing colon
-		{"//line foo\n", valid, filename, 1, 0},    // missing colon
-		{"  //line foo:\n", valid, filename, 1, 0}, // not a line start
-		{"//  line foo:\n", valid, filename, 1, 0}, // space between // and line
+		{"//\n", valid, filename, 2, 1},            // no directive
+		{"//line\n", valid, filename, 2, 1},        // missing colon
+		{"//line foo\n", valid, filename, 2, 1},    // missing colon
+		{"  //line foo:\n", valid, filename, 2, 1}, // not a line start
+		{"//  line foo:\n", valid, filename, 2, 1}, // space between // and line
 
 		// invalid //line directives with one colon
-		{"//line :\n", "invalid line number: ", filename, 0, 8},
-		{"//line :x\n", "invalid line number: x", filename, 0, 8},
-		{"//line foo :\n", "invalid line number: ", filename, 0, 12},
-		{"//line foo:x\n", "invalid line number: x", filename, 0, 11},
-		{"//line foo:0\n", "invalid line number: 0", filename, 0, 11},
-		{"//line foo:1 \n", "invalid line number: 1 ", filename, 0, 11},
-		{"//line foo:-12\n", "invalid line number: -12", filename, 0, 11},
-		{"//line C:foo:0\n", "invalid line number: 0", filename, 0, 13},
-		{fmt.Sprintf("//line foo:%d\n", tooLarge), fmt.Sprintf("invalid line number: %d", tooLarge), filename, 0, 11},
+		{"//line :\n", "invalid line number: ", filename, 1, 9},
+		{"//line :x\n", "invalid line number: x", filename, 1, 9},
+		{"//line foo :\n", "invalid line number: ", filename, 1, 13},
+		{"//line foo:x\n", "invalid line number: x", filename, 1, 12},
+		{"//line foo:0\n", "invalid line number: 0", filename, 1, 12},
+		{"//line foo:1 \n", "invalid line number: 1 ", filename, 1, 12},
+		{"//line foo:-12\n", "invalid line number: -12", filename, 1, 12},
+		{"//line C:foo:0\n", "invalid line number: 0", filename, 1, 14},
+		{fmt.Sprintf("//line foo:%d\n", tooLarge), fmt.Sprintf("invalid line number: %d", tooLarge), filename, 1, 12},
 
 		// invalid //line directives with two colons
-		{"//line ::\n", "invalid line number: ", filename, 0, 9},
-		{"//line ::x\n", "invalid line number: x", filename, 0, 9},
-		{"//line foo::123abc\n", "invalid line number: 123abc", filename, 0, 12},
-		{"//line foo::0\n", "invalid line number: 0", filename, 0, 12},
-		{"//line foo:0:1\n", "invalid line number: 0", filename, 0, 11},
+		{"//line ::\n", "invalid line number: ", filename, 1, 10},
+		{"//line ::x\n", "invalid line number: x", filename, 1, 10},
+		{"//line foo::123abc\n", "invalid line number: 123abc", filename, 1, 13},
+		{"//line foo::0\n", "invalid line number: 0", filename, 1, 13},
+		{"//line foo:0:1\n", "invalid line number: 0", filename, 1, 12},
 
-		{"//line :123:0\n", "invalid column number: 0", filename, 0, 12},
-		{"//line foo:123:0\n", "invalid column number: 0", filename, 0, 15},
-		{fmt.Sprintf("//line foo:10:%d\n", tooLarge), fmt.Sprintf("invalid column number: %d", tooLarge), filename, 0, 14},
+		{"//line :123:0\n", "invalid column number: 0", filename, 1, 13},
+		{"//line foo:123:0\n", "invalid column number: 0", filename, 1, 16},
+		{fmt.Sprintf("//line foo:10:%d\n", tooLarge), fmt.Sprintf("invalid column number: %d", tooLarge), filename, 1, 15},
 
 		// 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 /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},
+		{"//line foo:123\n   foo", valid, "foo", 123, 0},
+		{"//line  foo:123\n   foo", valid, " foo", 123, 0},
+		{"//line foo:123\n//line bar:345\nfoo", valid, "bar", 345, 0},
+		{"//line C:foo:123\n", valid, "C:foo", 123, 0},
+		{"//line /src/a/a.go:123\n   foo", valid, "/src/a/a.go", 123, 0},
+		{"//line :x:1\n", valid, ":x", 1, 0},
+		{"//line foo ::1\n", valid, "foo :", 1, 0},
+		{"//line foo:123abc:1\n", valid, "foo:123abc", 1, 0},
+		{"//line foo :123:1\n", valid, "foo ", 123, 1},
+		{"//line ::123\n", valid, ":", 123, 0},
 
 		// 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},
+		{"//line :x:1:10\n", valid, ":x", 1, 10},
+		{"//line foo ::1:2\n", valid, "foo :", 1, 2},
+		{"//line foo:123abc:1:1000\n", valid, "foo:123abc", 1, 1000},
+		{"//line foo :123:1000\n\n", valid, "foo ", 124, 1},
+		{"//line ::123:1234\n", valid, ":", 123, 1234},
 
 		// //line directives with omitted filenames lead to empty filenames
-		{"//line :10\n", valid, "", 10 - linebase, 0},
-		{"//line :10:20\n", valid, filename, 10 - linebase, 20 - colbase},
-		{"//line bar:1\n//line :10\n", valid, "", 10 - linebase, 0},
-		{"//line bar:1\n//line :10:20\n", valid, "bar", 10 - linebase, 20 - colbase},
+		{"//line :10\n", valid, "", 10, 0},
+		{"//line :10:20\n", valid, filename, 10, 20},
+		{"//line bar:1\n//line :10\n", valid, "", 10, 0},
+		{"//line bar:1\n//line :10:20\n", valid, "bar", 10, 20},
 
 		// ignored /*line directives
-		{"/**/", valid, filename, 0, 4},             // no directive
-		{"/*line*/", valid, filename, 0, 8},         // missing colon
-		{"/*line foo*/", valid, filename, 0, 12},    // missing colon
-		{"  //line foo:*/", valid, filename, 0, 15}, // not a line start
-		{"/*  line foo:*/", valid, filename, 0, 15}, // space between // and line
+		{"/**/", valid, filename, 1, 5},             // no directive
+		{"/*line*/", valid, filename, 1, 9},         // missing colon
+		{"/*line foo*/", valid, filename, 1, 13},    // missing colon
+		{"  //line foo:*/", valid, filename, 1, 16}, // not a line start
+		{"/*  line foo:*/", valid, filename, 1, 16}, // space between // and line
 
 		// invalid /*line directives with one colon
-		{"/*line :*/", "invalid line number: ", filename, 0, 8},
-		{"/*line :x*/", "invalid line number: x", filename, 0, 8},
-		{"/*line foo :*/", "invalid line number: ", filename, 0, 12},
-		{"/*line foo:x*/", "invalid line number: x", filename, 0, 11},
-		{"/*line foo:0*/", "invalid line number: 0", filename, 0, 11},
-		{"/*line foo:1 */", "invalid line number: 1 ", filename, 0, 11},
-		{"/*line C:foo:0*/", "invalid line number: 0", filename, 0, 13},
-		{fmt.Sprintf("/*line foo:%d*/", tooLarge), fmt.Sprintf("invalid line number: %d", tooLarge), filename, 0, 11},
+		{"/*line :*/", "invalid line number: ", filename, 1, 9},
+		{"/*line :x*/", "invalid line number: x", filename, 1, 9},
+		{"/*line foo :*/", "invalid line number: ", filename, 1, 13},
+		{"/*line foo:x*/", "invalid line number: x", filename, 1, 12},
+		{"/*line foo:0*/", "invalid line number: 0", filename, 1, 12},
+		{"/*line foo:1 */", "invalid line number: 1 ", filename, 1, 12},
+		{"/*line C:foo:0*/", "invalid line number: 0", filename, 1, 14},
+		{fmt.Sprintf("/*line foo:%d*/", tooLarge), fmt.Sprintf("invalid line number: %d", tooLarge), filename, 1, 12},
 
 		// invalid /*line directives with two colons
-		{"/*line ::*/", "invalid line number: ", filename, 0, 9},
-		{"/*line ::x*/", "invalid line number: x", filename, 0, 9},
-		{"/*line foo::123abc*/", "invalid line number: 123abc", filename, 0, 12},
-		{"/*line foo::0*/", "invalid line number: 0", filename, 0, 12},
-		{"/*line foo:0:1*/", "invalid line number: 0", filename, 0, 11},
+		{"/*line ::*/", "invalid line number: ", filename, 1, 10},
+		{"/*line ::x*/", "invalid line number: x", filename, 1, 10},
+		{"/*line foo::123abc*/", "invalid line number: 123abc", filename, 1, 13},
+		{"/*line foo::0*/", "invalid line number: 0", filename, 1, 13},
+		{"/*line foo:0:1*/", "invalid line number: 0", filename, 1, 12},
 
-		{"/*line :123:0*/", "invalid column number: 0", filename, 0, 12},
-		{"/*line foo:123:0*/", "invalid column number: 0", filename, 0, 15},
-		{fmt.Sprintf("/*line foo:10:%d*/", tooLarge), fmt.Sprintf("invalid column number: %d", tooLarge), filename, 0, 14},
+		{"/*line :123:0*/", "invalid column number: 0", filename, 1, 13},
+		{"/*line foo:123:0*/", "invalid column number: 0", filename, 1, 16},
+		{fmt.Sprintf("/*line foo:10:%d*/", tooLarge), fmt.Sprintf("invalid column number: %d", tooLarge), filename, 1, 15},
 
 		// 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*/", 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},
+		{"/*line foo:123*/   foo", valid, "foo", 123, 0},
+		{"/*line foo:123*/\n//line bar:345\nfoo", valid, "bar", 345, 0},
+		{"/*line C:foo:123*/", valid, "C:foo", 123, 0},
+		{"/*line /src/a/a.go:123*/   foo", valid, "/src/a/a.go", 123, 0},
+		{"/*line :x:1*/", valid, ":x", 1, 0},
+		{"/*line foo ::1*/", valid, "foo :", 1, 0},
+		{"/*line foo:123abc:1*/", valid, "foo:123abc", 1, 0},
+		{"/*line foo :123:10*/", valid, "foo ", 123, 10},
+		{"/*line ::123*/", valid, ":", 123, 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},
+		{"/*line :x:1:10*/", valid, ":x", 1, 10},
+		{"/*line foo ::1:2*/", valid, "foo :", 1, 2},
+		{"/*line foo:123abc:1:1000*/", valid, "foo:123abc", 1, 1000},
+		{"/*line foo :123:1000*/\n", valid, "foo ", 124, 1},
+		{"/*line ::123:1234*/", valid, ":", 123, 1234},
 
 		// /*line directives with omitted filenames lead to the previously used filenames
-		{"/*line :10*/", valid, "", 10 - linebase, 0},
-		{"/*line :10:20*/", valid, filename, 10 - linebase, 20 - colbase},
-		{"//line bar:1\n/*line :10*/", valid, "", 10 - linebase, 0},
-		{"//line bar:1\n/*line :10:20*/", valid, "bar", 10 - linebase, 20 - colbase},
+		{"/*line :10*/", valid, "", 10, 0},
+		{"/*line :10:20*/", valid, filename, 10, 20},
+		{"//line bar:1\n/*line :10*/", valid, "", 10, 0},
+		{"//line bar:1\n/*line :10:20*/", valid, "bar", 10, 20},
 	} {
 		base := NewFileBase(filename)
 		_, err := Parse(base, strings.NewReader(test.src), nil, nil, 0)
@@ -320,11 +320,11 @@ func TestLineDirectives(t *testing.T) {
 		if filename := pos.RelFilename(); filename != test.filename {
 			t.Errorf("%s: got filename = %q; want %q", test.src, filename, test.filename)
 		}
-		if line := pos.RelLine(); line != test.line+linebase {
-			t.Errorf("%s: got line = %d; want %d", test.src, line, test.line+linebase)
+		if line := pos.RelLine(); line != test.line {
+			t.Errorf("%s: got line = %d; want %d", test.src, line, test.line)
 		}
-		if col := pos.RelCol(); col != test.col+colbase {
-			t.Errorf("%s: got col = %d; want %d", test.src, col, test.col+colbase)
+		if col := pos.RelCol(); col != test.col {
+			t.Errorf("%s: got col = %d; want %d", test.src, col, test.col)
 		}
 	}
 }
diff --git a/src/cmd/compile/internal/syntax/pos.go b/src/cmd/compile/internal/syntax/pos.go
index 00e603e8d3..c683c7fcfc 100644
--- a/src/cmd/compile/internal/syntax/pos.go
+++ b/src/cmd/compile/internal/syntax/pos.go
@@ -31,10 +31,26 @@ func (pos Pos) Base() *PosBase { return pos.base }
 func (pos Pos) Line() uint     { return uint(pos.line) }
 func (pos Pos) Col() uint      { return uint(pos.col) }
 
-func (pos Pos) RelFilename() string { b := pos.Base(); return b.Filename() }
-func (pos Pos) RelLine() uint       { b := pos.Base(); return b.Line() + (pos.Line() - b.Pos().Line()) }
+func (pos Pos) RelFilename() string { return pos.base.Filename() }
+
+func (pos Pos) RelLine() uint {
+	b := pos.base
+	if b.Line() == 0 {
+		// base line is unknown => relative line is unknown
+		return 0
+	}
+	return b.Line() + (pos.Line() - b.Pos().Line())
+}
+
 func (pos Pos) RelCol() uint {
-	b := pos.Base()
+	b := pos.base
+	if b.Col() == 0 {
+		// base column is unknown => relative column is unknown
+		// (the current specification for line directives requires
+		// this to apply until the next PosBase/line directive,
+		// not just until the new newline)
+		return 0
+	}
 	if pos.Line() == b.Pos().Line() {
 		// pos on same line as pos base => column is relative to pos base
 		return b.Col() + (pos.Col() - b.Pos().Col())
@@ -43,13 +59,34 @@ func (pos Pos) RelCol() uint {
 }
 
 func (pos Pos) String() string {
-	s := fmt.Sprintf("%s:%d:%d", pos.RelFilename(), pos.RelLine(), pos.RelCol())
-	if bpos := pos.Base().Pos(); bpos.IsKnown() {
-		s += fmt.Sprintf("[%s:%d:%d]", bpos.RelFilename(), pos.Line(), pos.Col())
+	rel := position_{pos.RelFilename(), pos.RelLine(), pos.RelCol()}
+	abs := position_{pos.Base().Pos().RelFilename(), pos.Line(), pos.Col()}
+	s := rel.String()
+	if rel != abs {
+		s += "[" + abs.String() + "]"
 	}
 	return s
 }
 
+// TODO(gri) cleanup: find better name, avoid conflict with position in error_test.go
+type position_ struct {
+	filename  string
+	line, col uint
+}
+
+func (p position_) String() string {
+	if p.line == 0 {
+		if p.filename == "" {
+			return "<unknown position>"
+		}
+		return p.filename
+	}
+	if p.col == 0 {
+		return fmt.Sprintf("%s:%d", p.filename, p.line)
+	}
+	return fmt.Sprintf("%s:%d:%d", p.filename, p.line, p.col)
+}
+
 // A PosBase represents the base for relative position information:
 // At position pos, the relative position is filename:line:col.
 type PosBase struct {
@@ -59,9 +96,12 @@ type PosBase struct {
 }
 
 // NewFileBase returns a new PosBase for the given filename.
-// The PosBase position is unknown in this case.
+// A file PosBase's position is relative to itself, with the
+// position being filename:1:1.
 func NewFileBase(filename string) *PosBase {
-	return &PosBase{filename: filename}
+	base := &PosBase{MakePos(nil, linebase, colbase), filename, linebase, colbase}
+	base.pos.base = base
+	return base
 }
 
 // NewLineBase returns a new PosBase for a line directive "line filename:line:col"
@@ -73,6 +113,13 @@ func NewLineBase(pos Pos, filename string, line, col uint) *PosBase {
 	return &PosBase{pos, filename, sat32(line), sat32(col)}
 }
 
+func (base *PosBase) IsFileBase() bool {
+	if base == nil {
+		return false
+	}
+	return base.pos.base == base
+}
+
 func (base *PosBase) Pos() (_ Pos) {
 	if base == nil {
 		return