diff --git a/doc/next/5-toolchain.md b/doc/next/5-toolchain.md index 0f4a816479..888f43c5de 100644 --- a/doc/next/5-toolchain.md +++ b/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} diff --git a/src/cmd/compile/doc.go b/src/cmd/compile/doc.go index 81e6189a26..502f658e98 100644 --- a/src/cmd/compile/doc.go +++ b/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 diff --git a/src/cmd/compile/internal/syntax/parser.go b/src/cmd/compile/internal/syntax/parser.go index aade7b9fd3..f1f15c60a2 100644 --- a/src/cmd/compile/internal/syntax/parser.go +++ b/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) diff --git a/src/cmd/compile/internal/syntax/parser_test.go b/src/cmd/compile/internal/syntax/parser_test.go index b6c4b8fd56..e086f56f4c 100644 --- a/src/cmd/compile/internal/syntax/parser_test.go +++ b/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") diff --git a/src/cmd/go/testdata/script/build_line_directive_relative.txt b/src/cmd/go/testdata/script/build_line_directive_relative.txt new file mode 100644 index 0000000000..d821875515 --- /dev/null +++ b/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 diff --git a/src/cmd/internal/testdir/testdir_test.go b/src/cmd/internal/testdir/testdir_test.go index 07984396b7..d81b3a849c 100644 --- a/src/cmd/internal/testdir/testdir_test.go +++ b/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 diff --git a/test/fixedbugs/issue18149.go b/test/fixedbugs/issue18149.go index 112cd52530..4b6eea6fd0 100644 --- a/test/fixedbugs/issue18149.go +++ b/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)) } } diff --git a/test/fixedbugs/issue22660.go b/test/fixedbugs/issue22660.go index 10622033f0..1d89805e9e 100644 --- a/test/fixedbugs/issue22660.go +++ b/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 /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) } } diff --git a/test/fixedbugs/issue22662.go b/test/fixedbugs/issue22662.go index a1f00bfac3..16e8abfa0e 100644 --- a/test/fixedbugs/issue22662.go +++ b/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)) } } diff --git a/test/fixedbugs/issue22662b.go b/test/fixedbugs/issue22662b.go index a58a2e0a15..15c28ec95e 100644 --- a/test/fixedbugs/issue22662b.go +++ b/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) }