x/tools/go/packages: "unsafe" should contain GoFiles=["unsafe/unsafe.go"]

[adonovan]

Since its inception, go/packages has discarded the GoFiles field of the unsafe package. (I don't remember why I implemented it this way: perhaps I thought that GoFiles was for files that contain valid Go syntax but are de-selected from the build due to non-matching build tags, whereas it's debatable whether unsafe.go is valid Go. It can't be compiled without a .s file because it has missing function bodies, for example.) In any case, the unsafe package nearly always needs special handling by client applications, and it is inconvenient for some (e.g. gopls) to know the location of the unsafe.go file, so that it can be presented as documentation, and there is insufficient information to compute it from the packages.Package record for package "unsafe", which contains only the ID, Path, and Name fields, but no file names at all.

In hindsight I think it makes sense to include unsafe.go among the non-compiled GoFiles. I suspect the change is low risk because (as I mentioned) unsafe nearly always requires special handling anyway, and the GoFiles are never processed by the type checker.

[gopherbot]

Change https://go.dev/cl/491375 mentions this issue: go/packages: include "unsafe".GoFiles=["unsafe.go"]

[bcmills]

On the cmd/go side, I see:

~$ gotip list -json unsafe
{
        "Dir": "/usr/local/google/home/bcmills/sdk/gotip/src/unsafe",
        "ImportPath": "unsafe",
        "Name": "unsafe",
        "Doc": "Package unsafe contains operations that step around the type safety of Go programs.",
        "Root": "/usr/local/google/home/bcmills/sdk/gotip",
        "Match": [
                "unsafe"
        ],
        "Goroot": true,
        "Standard": true,
        "GoFiles": [
                "unsafe.go"
        ]
}

But perhaps we should be reporting unsafe.go in IgnoredGoFiles instead, since we don't actually present that file to the compiler in order to build the package? (I'm not sure.)

[adonovan]

But perhaps we should be reporting unsafe.go in IgnoredGoFiles instead, since we don't actually present that file to the compiler in order to build the package? (I'm not sure.)

I'm not sure either. IgnoredGoFiles says it's for "files that might be part of the package using other build configurations" and are thus valid syntax, whereas GoFiles (as a matter of fact---it's not documented) holds the cgo source files, which are not really valid Go syntax. So I think GoFiles may be a better fit.

[bcmills]

IgnoredGoFiles is not necessarily valid syntax for the Go version in current use.

(For example, the files could contain //go:build go1.24 constraints, in which case they would be syntactically valid for Go 1.24 but not necessarily for Go 1.20.)

[adonovan]

True. But the unsafe.go and builtin.go source files are never supposed to be seen by the compiler.