cmd/compile/internal/syntax: resolve //line filenames relative to source directory

The behaviour of the compiler and go/scanner is unified:
Relative //line directives are now relative to the directory of
the scanned file.

Fixes #70478

Change-Id: I804edbb5f9540d386e0fb1a4e64b10b717125d1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/706795
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

doc/next/5-toolchain.md

@@ -1,5 +1,10 @@
 ## Compiler {#compiler}
 
+The compiler now resolves a relative filename in a `//line` or `/*line*/`
+directive against the directory of the file containing the directive,
+matching [go/scanner]. Absolute filenames are unaffected.
+See [#70478](/issue/70478).
+
 ## Assembler {#assembler}
 
 ## Linker {#linker}

src/cmd/compile/doc.go

@@ -198,9 +198,15 @@ all other compiler directives are of the form
 // is peeled off the same way if it is valid. Anything before that is considered the filename
 // (possibly including blanks and colons). Invalid line or column values are reported as errors.
 //
+// A relative filename is interpreted relative to the directory of the file
+// containing the directive. Absolute filenames are used as given. A filename
+// inherited from a previous directive (the empty-filename form //line :line:col)
+// is reused verbatim and is not re-resolved.
+//
 // Examples:
 //
 //	//line foo.go:10      the filename is foo.go, and the line number is 10 for the next line
+//	//line ../foo.go:10   relative filenames are resolved against the directive's source directory
 //	//line C:foo.go:10    colons are permitted in filenames, here the filename is C:foo.go, and the line is 10
 //	//line  a:100 :10     blanks are permitted in filenames, here the filename is " a:100 " (excluding quotes)
 //	/*line :10:20*/x      the position of x is in the current file with line number 10 and column number 20

src/cmd/compile/internal/syntax/parser.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"go/build/constraint"
 	"io"
+	"path/filepath"
 	"strconv"
 	"strings"
 )
@@ -161,6 +162,13 @@ func (p *parser) updateBase(pos Pos, tline, tcol uint, text string) {
 	if filename == "" && ok2 {
 		filename = p.base.Filename()
 		trimmed = p.base.Trimmed()
+	} else if filename != "" {
+		filename = filepath.Clean(filename)
+		if !filepath.IsAbs(filename) {
+			if dir := filepath.Dir(p.file.Filename()); dir != "." {
+				filename = filepath.Join(dir, filename)
+			}
+		}
 	}
 
 	p.base = NewLineBase(pos, filename, trimmed, line, col)

src/cmd/compile/internal/syntax/parser_test.go

@@ -274,8 +274,6 @@ func TestLineDirectives(t *testing.T) {
 		{"//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},
@@ -326,8 +324,6 @@ func TestLineDirectives(t *testing.T) {
 		// effect of valid /*line directives on lines
 		{"/*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},
@@ -375,6 +371,106 @@ func TestLineDirectives(t *testing.T) {
 	}
 }
 
+func TestLineDirectivesWithDir(t *testing.T) {
+	const valid = "syntax error: package statement must be first"
+	srcFile := filepath.Join("dir", "directives.go")
+
+	check := func(src, want string) {
+		t.Helper()
+		base := NewFileBase(srcFile)
+		_, err := Parse(base, strings.NewReader(src), nil, nil, 0)
+		if err == nil {
+			t.Errorf("%s: no error reported", src)
+			return
+		}
+		perr, ok := err.(Error)
+		if !ok {
+			t.Errorf("%s: got %v; want parser error", src, err)
+			return
+		}
+		if perr.Msg != valid {
+			t.Errorf("%s: got msg = %q; want %q", src, perr.Msg, valid)
+			return
+		}
+		if got := perr.Pos.RelFilename(); got != want {
+			t.Errorf("%s: got filename = %q; want %q", src, got, want)
+		}
+	}
+
+	for _, test := range []struct {
+		src      string
+		filename string
+	}{
+		{"//line foo:1\n   x", filepath.Join("dir", "foo")},
+		{"//line ./foo:1\n   x", filepath.Join("dir", "foo")},
+		{"//line ../foo:1\n   x", "foo"},
+		{"//line sub/foo:1\n   x", filepath.Join("dir", "sub", "foo")},
+		{"/*line foo:1*/   x", filepath.Join("dir", "foo")},
+		{"//line bar:1\n//line :2:1\n   x", filepath.Join("dir", "bar")},
+	} {
+		check(test.src, test.filename)
+	}
+
+	var absCases []struct {
+		src, filename string
+	}
+	if runtime.GOOS == "windows" {
+		absCases = append(absCases, struct{ src, filename string }{
+			"//line c:\\bar:1\n   x", "c:\\bar",
+		})
+	} else {
+		absCases = append(absCases,
+			struct{ src, filename string }{"//line /abs/foo:1\n   x", "/abs/foo"},
+			struct{ src, filename string }{"//line /src/a/a.go:1\n   x", "/src/a/a.go"},
+		)
+	}
+	for _, test := range absCases {
+		check(test.src, test.filename)
+	}
+}
+
+func TestLineDirectivesPaths(t *testing.T) {
+	const valid = "syntax error: package statement must be first"
+	const filename = "directives.go"
+
+	type tc struct {
+		src      string
+		filename string
+		line     uint
+	}
+	var cases []tc
+	cases = []tc{
+		{"//line C:foo:123\n", "C:foo", 123},
+		{"//line /src/a/a.go:123\n   foo", filepath.Clean("/src/a/a.go"), 123},
+		{"/*line C:foo:123*/", "C:foo", 123},
+		{"/*line /src/a/a.go:123*/   foo", filepath.Clean("/src/a/a.go"), 123},
+		{"//line foo/../bar:1\n   x", "bar", 1},
+	}
+	for _, test := range cases {
+		base := NewFileBase(filename)
+		_, err := Parse(base, strings.NewReader(test.src), nil, nil, 0)
+		if err == nil {
+			t.Errorf("%s: no error reported", test.src)
+			continue
+		}
+		perr, ok := err.(Error)
+		if !ok {
+			t.Errorf("%s: got %v; want parser error", test.src, err)
+			continue
+		}
+		if perr.Msg != valid {
+			t.Errorf("%s: got msg = %q; want %q", test.src, perr.Msg, valid)
+			continue
+		}
+		if got := perr.Pos.RelFilename(); got != test.filename {
+			t.Errorf("%s: got filename = %q; want %q", test.src, got, test.filename)
+		}
+		if got := perr.Pos.RelLine(); got != test.line {
+			t.Errorf("%s: got line = %d; want %d", test.src, got, test.line)
+		}
+	}
+}
+
 // Test that typical uses of UnpackListExpr don't allocate.
 func TestUnpackListExprAllocs(t *testing.T) {
 	var x Expr = NewName(Pos{}, "x")

src/cmd/go/testdata/script/build_line_directive_relative.txt

@@ -0,0 +1,32 @@
+# Verify that the compiler resolves a relative filename in a //line directive
+# against the directory of the file containing the directive, not against
+# the compiler's working directory.
+# See go.dev/issue/70478.
+
+# Build from the module root: the resolved path should include the package dir.
+! go build ./sub
+stderr 'sub[/\\]README\.md:1: '
+
+# Build from a sibling directory (still inside the module): same resolved path.
+mkdir other
+cd other
+! go build ../sub
+stderr 'sub[/\\]README\.md:1: '
+cd ..
+
+# Build from inside the package directory: the resolved path is anchored at
+# that directory (which would not be the case under the pre-#70478 behavior,
+# where the bare README.md was reported regardless of the build CWD).
+cd sub
+! go build .
+stderr '\.[/\\]README\.md:1: '
+
+-- go.mod --
+module example.com/linedirtest
+
+go 1.21
+-- sub/foo.go --
+package sub
+
+//line README.md:1
+invalid

src/cmd/internal/testdir/testdir_test.go

@@ -1363,11 +1363,12 @@ func (test) updateErrors(out, file string) {
 // That is, it needs the file name prefix followed by a : or a [,
 // and possibly preceded by a directory name.
 func matchPrefix(s, prefix string) bool {
+	s = s[len(filepath.VolumeName(s)):]
 	i := strings.Index(s, ":")
 	if i < 0 {
 		return false
 	}
-	j := strings.LastIndex(s[:i], "/")
+	j := strings.LastIndex(s[:i], string(filepath.Separator))
 	s = s[j+1:]
 	if len(s) <= len(prefix) || s[:len(prefix)] != prefix {
 		return false

test/fixedbugs/issue18149.go

@@ -13,15 +13,24 @@ package main
 import (
 	"fmt"
 	"runtime"
+	"strings"
 )
 
+// Since go.dev/issue/70478, the compiler resolves a relative filename in a
+// //line directive against the directory of the source file, so the expected
+// path may be a suffix of the actual filename rather than equal to it.
+// Accept either an exact match or the file as a path-component suffix.
+// Compiler-emitted paths are always slash-normalized (cmd/internal/objabi.AbsFile).
 func check(file string, line int) {
 	_, f, l, ok := runtime.Caller(1)
 	if !ok {
 		panic("runtime.Caller(1) failed")
 	}
-	if f != file || l != line {
-		panic(fmt.Sprintf("got %s:%d; want %s:%d", f, l, file, line))
+	// Prepend exactly one "/" even if file already starts with one, so that
+	// e.g. file="/foo/bar.go" looks for "/foo/bar.go" as the suffix, not "//".
+	want := "/" + strings.TrimPrefix(file, "/")
+	if (f != file && !strings.HasSuffix(f, want)) || l != line {
+		panic(fmt.Sprintf("got %s:%d; want %s:%d (or suffix %s)", f, l, file, line, want))
 	}
 }
 

test/fixedbugs/issue22660.go

@@ -27,7 +27,9 @@ func main() {
 	f.Close()
 	defer os.Remove(f.Name())
 
-	// path must appear in error messages even if we strip them with -trimpath
+	// path components from the //line directive must survive -trimpath and
+	// appear in error messages (since go.dev/issue/70478, as a path-component
+	// suffix of the resolved path, not as the bare prefix).
 	path := filepath.Join("users", "xxx", "go")
 	var src bytes.Buffer
 	fmt.Fprintf(&src, "//line %s:1\n", filepath.Join(path, "foo.go"))
@@ -41,7 +43,10 @@ func main() {
 		log.Fatalf("expected compiling %s to fail", f.Name())
 	}
 
-	if !strings.HasPrefix(string(out), path) {
-		log.Fatalf("expected full path (%s) in error message, got:\n%s", path, out)
+	// After #70478 the resolved path is <tempdir>/users/xxx/go/foo.go,
+	// so the directive's components must appear as a path suffix.
+	want := filepath.Join(path, "foo.go")
+	if !strings.Contains(string(out), string(filepath.Separator)+want) {
+		log.Fatalf("expected path component (%s) in error message, got:\n%s", want, out)
 	}
 }

test/fixedbugs/issue22662.go

@@ -12,15 +12,24 @@ package main
 import (
 	"fmt"
 	"runtime"
+	"strings"
 )
 
+// Since go.dev/issue/70478, the compiler resolves a relative filename in a
+// //line directive against the directory of the source file, so the expected
+// path may be a suffix of the actual filename rather than equal to it.
+// Accept either an exact match or the file as a path-component suffix.
+// Compiler-emitted paths are always slash-normalized (cmd/internal/objabi.AbsFile).
 func check(file string, line int) {
 	_, f, l, ok := runtime.Caller(1)
 	if !ok {
 		panic("runtime.Caller(1) failed")
 	}
-	if f != file || l != line {
-		panic(fmt.Sprintf("got %s:%d; want %s:%d", f, l, file, line))
+	// Prepend exactly one "/" even if file already starts with one, so that
+	// e.g. file="/foo/bar.go" looks for "/foo/bar.go" as the suffix, not "//".
+	want := "/" + strings.TrimPrefix(file, "/")
+	if (f != file && !strings.HasSuffix(f, want)) || l != line {
+		panic(fmt.Sprintf("got %s:%d; want %s:%d (or suffix %s)", f, l, file, line, want))
 	}
 }
 

test/fixedbugs/issue22662b.go

@@ -15,6 +15,7 @@ import (
 	"log"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strings"
 )
 
@@ -54,7 +55,12 @@ func main() {
 			log.Fatalf("expected compiling\n---\n%s\n---\nto fail", test.src)
 		}
 
-		errmsg := strings.Replace(string(out), f.Name(), "filename", -1) // use "filename" instead of actual (long) filename
+		// Replace the temp filename (used when the line directive inherits
+		// it via the empty-filename form), and strip the temp directory
+		// prefix that is now joined onto relative line-directive filenames
+		// since go.dev/issue/70478.
+		errmsg := strings.Replace(string(out), f.Name(), "filename", -1)
+		errmsg = strings.Replace(errmsg, filepath.Dir(f.Name())+string(filepath.Separator), "", -1)
 		if !strings.HasPrefix(errmsg, test.pos) {
 			log.Fatalf("%q: got %q; want position %q", test.src, errmsg, test.pos)
 		}