diff --git a/go/analysis/passes/usesgenerics/usesgenerics.go b/go/analysis/passes/usesgenerics/usesgenerics.go index 73b53fbefa..4956e0e7e3 100644 --- a/go/analysis/passes/usesgenerics/usesgenerics.go +++ b/go/analysis/passes/usesgenerics/usesgenerics.go @@ -7,15 +7,12 @@ package usesgenerics import ( - "go/ast" - "go/types" "reflect" - "strings" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/inspect" "golang.org/x/tools/go/ast/inspector" - "golang.org/x/tools/internal/typeparams" + "golang.org/x/tools/internal/typeparams/genericfeatures" ) var Analyzer = &analysis.Analyzer{ @@ -32,6 +29,16 @@ const Doc = `detect whether a package uses generics features The usesgenerics analysis reports whether a package directly or transitively uses certain features associated with generic programming in Go.` +type Features = genericfeatures.Features + +const ( + GenericTypeDecls = genericfeatures.GenericTypeDecls + GenericFuncDecls = genericfeatures.GenericFuncDecls + EmbeddedTypeSets = genericfeatures.EmbeddedTypeSets + TypeInstantiation = genericfeatures.TypeInstantiation + FuncInstantiation = genericfeatures.FuncInstantiation +) + // Result is the usesgenerics analyzer result type. The Direct field records // features used directly by the package being analyzed (i.e. contained in the // package source code). The Transitive field records any features used by the @@ -40,53 +47,6 @@ type Result struct { Direct, Transitive Features } -// Features is a set of flags reporting which features of generic Go code a -// package uses, or 0. -type Features int - -const ( - // GenericTypeDecls indicates whether the package declares types with type - // parameters. - GenericTypeDecls Features = 1 << iota - - // GenericFuncDecls indicates whether the package declares functions with - // type parameters. - GenericFuncDecls - - // EmbeddedTypeSets indicates whether the package declares interfaces that - // contain structural type restrictions, i.e. are not fully described by - // their method sets. - EmbeddedTypeSets - - // TypeInstantiation indicates whether the package instantiates any generic - // types. - TypeInstantiation - - // FuncInstantiation indicates whether the package instantiates any generic - // functions. - FuncInstantiation -) - -func (f Features) String() string { - var feats []string - if f&GenericTypeDecls != 0 { - feats = append(feats, "typeDecl") - } - if f&GenericFuncDecls != 0 { - feats = append(feats, "funcDecl") - } - if f&EmbeddedTypeSets != 0 { - feats = append(feats, "typeSet") - } - if f&TypeInstantiation != 0 { - feats = append(feats, "typeInstance") - } - if f&FuncInstantiation != 0 { - feats = append(feats, "funcInstance") - } - return "features{" + strings.Join(feats, ",") + "}" -} - type featuresFact struct { Features Features } @@ -95,7 +55,9 @@ func (f *featuresFact) AFact() {} func (f *featuresFact) String() string { return f.Features.String() } func run(pass *analysis.Pass) (interface{}, error) { - direct := directFeatures(pass) + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + direct := genericfeatures.ForPackage(inspect, pass.TypesInfo) transitive := direct | importedTransitiveFeatures(pass) if transitive != 0 { @@ -108,50 +70,6 @@ func run(pass *analysis.Pass) (interface{}, error) { }, nil } -// directFeatures computes which generic features are used directly by the -// package being analyzed. -func directFeatures(pass *analysis.Pass) Features { - inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) - - nodeFilter := []ast.Node{ - (*ast.FuncType)(nil), - (*ast.InterfaceType)(nil), - (*ast.ImportSpec)(nil), - (*ast.TypeSpec)(nil), - } - - var direct Features - - inspect.Preorder(nodeFilter, func(node ast.Node) { - switch n := node.(type) { - case *ast.FuncType: - if tparams := typeparams.ForFuncType(n); tparams != nil { - direct |= GenericFuncDecls - } - case *ast.InterfaceType: - tv := pass.TypesInfo.Types[n] - if iface, _ := tv.Type.(*types.Interface); iface != nil && !typeparams.IsMethodSet(iface) { - direct |= EmbeddedTypeSets - } - case *ast.TypeSpec: - if tparams := typeparams.ForTypeSpec(n); tparams != nil { - direct |= GenericTypeDecls - } - } - }) - - instances := typeparams.GetInstances(pass.TypesInfo) - for _, inst := range instances { - switch inst.Type.(type) { - case *types.Named: - direct |= TypeInstantiation - case *types.Signature: - direct |= FuncInstantiation - } - } - return direct -} - // importedTransitiveFeatures computes features that are used transitively via // imports. func importedTransitiveFeatures(pass *analysis.Pass) Features { diff --git a/go/internal/gcimporter/bexport_test.go b/go/internal/gcimporter/bexport_test.go index 8a80427c06..a65cd1db3e 100644 --- a/go/internal/gcimporter/bexport_test.go +++ b/go/internal/gcimporter/bexport_test.go @@ -19,10 +19,12 @@ import ( "strings" "testing" + "golang.org/x/tools/go/ast/inspector" "golang.org/x/tools/go/buildutil" "golang.org/x/tools/go/internal/gcimporter" "golang.org/x/tools/go/loader" "golang.org/x/tools/internal/typeparams" + "golang.org/x/tools/internal/typeparams/genericfeatures" ) var isRace = false @@ -66,14 +68,22 @@ type UnknownType undefined } numPkgs := len(prog.AllPackages) - if want := 248; numPkgs < want { + if want := minStdlibPackages; numPkgs < want { t.Errorf("Loaded only %d packages, want at least %d", numPkgs, want) } + checked := 0 for pkg, info := range prog.AllPackages { if info.Files == nil { continue // empty directory } + // Binary export does not support generic code. + inspect := inspector.New(info.Files) + if genericfeatures.ForPackage(inspect, &info.Info) != 0 { + t.Logf("skipping package %q which uses generics", pkg.Path()) + continue + } + checked++ exportdata, err := gcimporter.BExportData(conf.Fset, pkg) if err != nil { t.Fatal(err) @@ -116,6 +126,9 @@ type UnknownType undefined } } } + if want := minStdlibPackages; checked < want { + t.Errorf("Checked only %d packages, want at least %d", checked, want) + } } func fileLine(fset *token.FileSet, obj types.Object) string { diff --git a/go/internal/gcimporter/iexport_test.go b/go/internal/gcimporter/iexport_test.go index f035217b4b..201a070b67 100644 --- a/go/internal/gcimporter/iexport_test.go +++ b/go/internal/gcimporter/iexport_test.go @@ -28,10 +28,11 @@ import ( "strings" "testing" + "golang.org/x/tools/go/ast/inspector" "golang.org/x/tools/go/buildutil" "golang.org/x/tools/go/internal/gcimporter" "golang.org/x/tools/go/loader" - "golang.org/x/tools/internal/testenv" + "golang.org/x/tools/internal/typeparams/genericfeatures" ) func readExportFile(filename string) ([]byte, error) { @@ -69,6 +70,8 @@ func isUnifiedBuilder() bool { return os.Getenv("GO_BUILDER_NAME") == "linux-amd64-unified" } +const minStdlibPackages = 248 + func TestIExportData_stdlib(t *testing.T) { if runtime.Compiler == "gccgo" { t.Skip("gccgo standard library is inaccessible") @@ -90,15 +93,8 @@ func TestIExportData_stdlib(t *testing.T) { Sizes: types.SizesFor(ctxt.Compiler, ctxt.GOARCH), }, } - // Temporarily skip packages that use generics on the unified builder, to fix - // TryBots. - // - // TODO(#48595): fix this test with GOEXPERIMENT=unified. - isUnified := isUnifiedBuilder() for _, path := range buildutil.AllPackages(conf.Build) { - if !(isUnified && testenv.UsesGenerics(path)) { - conf.Import(path) - } + conf.Import(path) } // Create a package containing type and value errors to ensure @@ -117,13 +113,19 @@ type UnknownType undefined t.Fatalf("Load failed: %v", err) } - numPkgs := len(prog.AllPackages) - if want := 248; numPkgs < want { - t.Errorf("Loaded only %d packages, want at least %d", numPkgs, want) - } - var sorted []*types.Package + isUnified := isUnifiedBuilder() for pkg, info := range prog.AllPackages { + // Temporarily skip packages that use generics on the unified builder, to + // fix TryBots. + // + // TODO(#48595): fix this test with GOEXPERIMENT=unified. + inspect := inspector.New(info.Files) + features := genericfeatures.ForPackage(inspect, &info.Info) + if isUnified && features != 0 { + t.Logf("skipping package %q which uses generics", pkg.Path()) + continue + } if info.Files != nil { // non-empty directory sorted = append(sorted, pkg) } @@ -133,6 +135,11 @@ type UnknownType undefined }) version := gcimporter.IExportVersion + numPkgs := len(sorted) + if want := minStdlibPackages; numPkgs < want { + t.Errorf("Loaded only %d packages, want at least %d", numPkgs, want) + } + for _, pkg := range sorted { if exportdata, err := iexport(conf.Fset, version, pkg); err != nil { t.Error(err) diff --git a/go/ssa/stdlib_test.go b/go/ssa/stdlib_test.go index 329f0b382c..aaa158076c 100644 --- a/go/ssa/stdlib_test.go +++ b/go/ssa/stdlib_test.go @@ -16,17 +16,17 @@ package ssa_test import ( "go/ast" - "go/build" "go/token" "runtime" "testing" "time" - "golang.org/x/tools/go/buildutil" - "golang.org/x/tools/go/loader" + "golang.org/x/tools/go/ast/inspector" + "golang.org/x/tools/go/packages" "golang.org/x/tools/go/ssa" "golang.org/x/tools/go/ssa/ssautil" "golang.org/x/tools/internal/testenv" + "golang.org/x/tools/internal/typeparams/genericfeatures" ) func bytesAllocated() uint64 { @@ -46,23 +46,27 @@ func TestStdlib(t *testing.T) { t0 := time.Now() alloc0 := bytesAllocated() - // Load, parse and type-check the program. - ctxt := build.Default // copy - ctxt.GOPATH = "" // disable GOPATH - conf := loader.Config{Build: &ctxt} - for _, path := range buildutil.AllPackages(conf.Build) { - // Temporarily skip packages that use generics until supported by go/ssa. - // - // TODO(#48595): revert this once go/ssa supports generics. - if !testenv.UsesGenerics(path) { - conf.ImportWithTests(path) - } - } - - iprog, err := conf.Load() + cfg := &packages.Config{Mode: packages.LoadSyntax} + pkgs, err := packages.Load(cfg, "std", "cmd") if err != nil { - t.Fatalf("Load failed: %v", err) + t.Fatal(err) } + var nonGeneric int + for i := 0; i < len(pkgs); i++ { + pkg := pkgs[i] + inspect := inspector.New(pkg.Syntax) + features := genericfeatures.ForPackage(inspect, pkg.TypesInfo) + // Skip standard library packages that use generics. This won't be + // sufficient if any standard library packages start _importing_ packages + // that use generics. + if features != 0 { + t.Logf("skipping package %q which uses generics", pkg.PkgPath) + continue + } + pkgs[nonGeneric] = pkg + nonGeneric++ + } + pkgs = pkgs[:nonGeneric] t1 := time.Now() alloc1 := bytesAllocated() @@ -72,7 +76,7 @@ func TestStdlib(t *testing.T) { // Comment out these lines during benchmarking. Approx SSA build costs are noted. mode |= ssa.SanityCheckFunctions // + 2% space, + 4% time mode |= ssa.GlobalDebug // +30% space, +18% time - prog := ssautil.CreateProgram(iprog, mode) + prog, _ := ssautil.Packages(pkgs, mode) t2 := time.Now() @@ -87,8 +91,8 @@ func TestStdlib(t *testing.T) { t.Errorf("Loaded only %d packages, want at least %d", numPkgs, want) } - // Keep iprog reachable until after we've measured memory usage. - if len(iprog.AllPackages) == 0 { + // Keep pkgs reachable until after we've measured memory usage. + if len(pkgs) == 0 { panic("unreachable") } diff --git a/internal/testenv/testenv.go b/internal/testenv/testenv.go index 0010ca4c5d..61735dc2c2 100644 --- a/internal/testenv/testenv.go +++ b/internal/testenv/testenv.go @@ -297,9 +297,3 @@ func SkipAfterGo1Point(t Testing, x int) { t.Skipf("running Go version %q is version 1.%d, newer than maximum 1.%d", runtime.Version(), Go1Point(), x) } } - -// UsesGenerics reports if the standard library package stdlibPkg uses -// generics. -func UsesGenerics(stdlibPkg string) bool { - return stdlibPkg == "constraints" -} diff --git a/internal/typeparams/genericfeatures/features.go b/internal/typeparams/genericfeatures/features.go new file mode 100644 index 0000000000..8ceef86745 --- /dev/null +++ b/internal/typeparams/genericfeatures/features.go @@ -0,0 +1,105 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// The genericfeatures package provides utilities for detecting usage of +// generic programming in Go packages. +package genericfeatures + +import ( + "go/ast" + "go/types" + "strings" + + "golang.org/x/tools/go/ast/inspector" + "golang.org/x/tools/internal/typeparams" +) + +// Features is a set of flags reporting which features of generic Go code a +// package uses, or 0. +type Features int + +const ( + // GenericTypeDecls indicates whether the package declares types with type + // parameters. + GenericTypeDecls Features = 1 << iota + + // GenericFuncDecls indicates whether the package declares functions with + // type parameters. + GenericFuncDecls + + // EmbeddedTypeSets indicates whether the package declares interfaces that + // contain structural type restrictions, i.e. are not fully described by + // their method sets. + EmbeddedTypeSets + + // TypeInstantiation indicates whether the package instantiates any generic + // types. + TypeInstantiation + + // FuncInstantiation indicates whether the package instantiates any generic + // functions. + FuncInstantiation +) + +func (f Features) String() string { + var feats []string + if f&GenericTypeDecls != 0 { + feats = append(feats, "typeDecl") + } + if f&GenericFuncDecls != 0 { + feats = append(feats, "funcDecl") + } + if f&EmbeddedTypeSets != 0 { + feats = append(feats, "typeSet") + } + if f&TypeInstantiation != 0 { + feats = append(feats, "typeInstance") + } + if f&FuncInstantiation != 0 { + feats = append(feats, "funcInstance") + } + return "features{" + strings.Join(feats, ",") + "}" +} + +// ForPackage computes which generic features are used directly by the +// package being analyzed. +func ForPackage(inspect *inspector.Inspector, info *types.Info) Features { + nodeFilter := []ast.Node{ + (*ast.FuncType)(nil), + (*ast.InterfaceType)(nil), + (*ast.ImportSpec)(nil), + (*ast.TypeSpec)(nil), + } + + var direct Features + + inspect.Preorder(nodeFilter, func(node ast.Node) { + switch n := node.(type) { + case *ast.FuncType: + if tparams := typeparams.ForFuncType(n); tparams != nil { + direct |= GenericFuncDecls + } + case *ast.InterfaceType: + tv := info.Types[n] + if iface, _ := tv.Type.(*types.Interface); iface != nil && !typeparams.IsMethodSet(iface) { + direct |= EmbeddedTypeSets + } + case *ast.TypeSpec: + if tparams := typeparams.ForTypeSpec(n); tparams != nil { + direct |= GenericTypeDecls + } + } + }) + + instances := typeparams.GetInstances(info) + for _, inst := range instances { + switch inst.Type.(type) { + case *types.Named: + direct |= TypeInstantiation + case *types.Signature: + direct |= FuncInstantiation + } + } + return direct +}