Golang cmd/compile: line directives not recorded properly in unified export data

Reproducer (using the x/tools/go/packages testing API):

func TestReproducer(t *testing.T) {
    exported := packagestest.Export(t, packagestest.Modules, []packagestest.Module{
        {
            Name: "test",
            Files: map[string]any{
                "test.go": `package test; import "test/other"; var _ = other.Test;`,
                "other/file.go": `package other

//line other.go:1:1
var Test int
`,
            },
        },
    })
    t.Cleanup(exported.Cleanup)

    exported.Config.Mode = packages.LoadSyntax
    exported.Config.Fset = token.NewFileSet()
    pkgs, err := packages.Load(exported.Config, "test")
    if err != nil {
        t.Fatal(err)
    }
    packages.PrintErrors(pkgs)

    p := pkgs[0].Imports["test/other"]
    pos := p.Types.Scope().Lookup("Test").Pos()
    t.Log(exported.Config.Fset.Position(pos)) // other.go:4:1
}

The Mode is set to packages.LoadSyntax, thus go/packages is going to load test/other from the ExportFile.

$ go test ./go/packages/ -run TestReproducer -v
=== RUN   TestReproducer
    (....)
    packages_test.go:3455: other.go:4:1
--- PASS: TestReproducer (0.15s)

The file name (other.go) comes from the line directive, but the line/col (4:1) information does not.

I think that the issue is somewhere in cmd/go, while debugging, i saw the same values here (i.e. other.go:4:1):

https://github.com/golang/tools/blob/4f820dbaf9859eaafa01a17d133583f4d8c5a73c/internal/gcimporter/ureader_yes.go#L201-L205

Comment From: gabyhelp

Related Issues

• x/tools/cmd/goimports: line directive can cause file corruption #51119

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

Comment From: ianlancetaylor

CC @griesemer @adonovan @findleyr

Comment From: adonovan

Thanks for investigating. Here's a smaller reproducer that doesn't depend on x/tools/go/packages:

xtools$ cat a/a.go 
package a

//line b.go:1:1
var A int
xtools$ go run ./go/gcexportdata/main.go $(go list -export -f {{.Export}} ./a)
package a
b.go:4:1: var A int

Adding logging to reader.pos in x/tools/internal/gcimporter/ureader_yes.go reveals that the export data contains (file "b.go", line 4, col 5) for this declaration, so the problem is on the encoding side, i.e. in the compiler.

The following change to the compiler fixes the problem:

// pos writes the position of p into the element bitstream.
func (w *writer) pos(p poser) {
    w.Sync(pkgbits.SyncPos)
    pos := p.Pos()

    // TODO(mdempsky): Track down the remaining cases here and fix them.
    if !w.Bool(pos.IsKnown()) {
        return
    }

    // TODO(mdempsky): Delta encoding.
    w.posBase(pos.Base())
-   w.Uint(pos.Line())
+   w.Uint(pos.RelLine())
-   w.Uint(pos.Col())
+   w.Uint(pos.RelCol())
}

but I'm not sure whether it is the right fix overall.

@griesemer