x/tools/go/pacakges: missing TypesInfo when NeedTypesInfo was set while NeedSyntax & NeedTypes were not

xrxtzz commented 4 days ago

Go version

go1.22.7 darwin/arm64

Output of `go env` in your module/workspace:

GO111MODULE='on'
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/xrxtzz/Library/Caches/go-build'
GOENV='/Users/xrxtzz/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/xrxtzz/go/pkg/mod'
GONOPROXY='`*.everphoto.cn,git.smartisan.com'
GONOSUMDB='`*.everphoto.cn,git.smartisan.com'
GOOS='darwin'
GOPATH='/Users/xrxtzz/go'
GOPRIVATE='`*.everphoto.cn,git.smartisan.com'
GOPROXY='https://goproxy.cn,direct'
GOROOT='/opt/homebrew/Cellar/go@1.22/1.22.7/libexec'
GOSUMDB='sum.golang.google.cn'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/opt/homebrew/Cellar/go@1.22/1.22.7/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.7'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/xrxtzz/Documents/code/code_analysis/irfronts/go/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/lk/nhh_2w2n2sv2kdzk75mfbrcr0000gp/T/go-build3609621157=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

The NeedTypesInfo option indicates that packages.Load would add TypesInfo field to result packages. But it turns out that when NeedTypesInfo was used separately from NeedSyntax or NeedTypes, the TypesInfo fields would simply be nil.

package main

import (
    "fmt"

    "golang.org/x/tools/go/packages"
)

func main() {
    packs, _ := loadProgramPackageWithTypesInfo("strings")
    fmt.Println(packs[0].TypesInfo)
}

func loadProgramPackageWithTypesInfo(pattern string) ([]*packages.Package, error) {
    cfg := packages.Config{
        Mode: packages.NeedName | packages.NeedFiles | packages.NeedCompiledGoFiles |
            packages.NeedImports | packages.NeedTypesInfo | packages.NeedTypesSizes,
    }
    return packages.Load(&cfg, pattern)
}

What did you see happen?

When NeedTypesInfo was used separately from NeedSyntax or NeedTypes, in LoadMode of packages.Load, the TypesInfo fields would simply be nil.

What did you expect to see?

The TypesInfo were produced properly.

gabyhelp commented 4 days ago

Related Issues and Documentation

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

adonovan commented 3 days ago

types.Info is, fundamentally, the set of type-checker annotations on the syntax tree. You can neither compute it nor meaningfully use it without syntax trees. Indeed, the keys of the types.Info.Scopes mapping is a superset of the list of ast.Files in Packages.Syntax, so if you have Packages.TypesInfo you could derive Packages.Syntax from it. How did you plan to use TypesInfo without Syntax?

While the Need interface of go/packages is complicated and certainly has bugs, in this case I think it is working as intended. Perhaps the documentation could be improved.

xrxtzz commented 3 days ago

Indeed, the keys of the types.Info.Scopes mapping is a superset of the list of ast.Files in Packages.Syntax, so if you have Packages.TypesInfo you could derive Packages.Syntax from it. How did you plan to use TypesInfo without Syntax?

Agree on that, but the behaviors of different combinations of LoadModes still make no sense, for example:

NeedSyntax & NeedTypesInfo won't actually give TypesInfo, though we do have Syntax here and can use TypesInfo well.
...but NeedTypes & NeedTypesInfo can output TypesInfo, and in this case no Syntax was obtained.

Reason for above two cases is probably because *loader.loadPackage would simply return on tools/go/packages/packages.go:1162 unless NeedTypes was set. When I look into it deeper I found that indeed the LoadMode controlling here is pretty complicated and buggy (as indicated in other related issues)...

adonovan commented 21 hours ago

Possibly the right fix here is just:

--- a/go/packages/packages.go
+++ b/go/packages/packages.go
@@ -1158,7 +1158,7 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) {
        }

        lpkg.Syntax = files
-       if ld.Config.Mode&NeedTypes == 0 {
+       if ld.Config.Mode&(NeedTypes|NeedTypesInfo) == 0 {
                return
        }

It's a little weird to ask for only TypesInfo but not Syntax or Types, but I don't see any reason we can't honor the request.

@matloob

golang / go