internal/typeparams: guard against generics in stdlib tests
Add a new package internal/typeparams/genericfeatures, factoring out the
check for usage of generics from the usesgenerics checker. For tests
operating on the standard library, this allows us to explicitly detect
usage of generics, rather than relying on a hard-coded list.
To make this work, I switched the go/ssa standard library test to use
go/packages rather than go/loader. This resulted in finding 457 packages
versus 676 (likely due to tests), but my assumption is that this is
still enough coverage.
Change-Id: I6ba32fb4068dd7f5eedb55c2081c642665ed124a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/355972
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>
diff --git a/go/analysis/passes/usesgenerics/usesgenerics.go b/go/analysis/passes/usesgenerics/usesgenerics.go
index 73b53fb..4956e0e 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 @@
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 @@
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) 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 @@
}, 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 8a80427..a65cd1d 100644
--- a/go/internal/gcimporter/bexport_test.go
+++ b/go/internal/gcimporter/bexport_test.go
@@ -19,10 +19,12 @@
"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 @@
}
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 @@
}
}
}
+ 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 f035217..201a070 100644
--- a/go/internal/gcimporter/iexport_test.go
+++ b/go/internal/gcimporter/iexport_test.go
@@ -28,10 +28,11 @@
"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 @@
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 @@
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 @@
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 @@
})
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 329f0b3..aaa1580 100644
--- a/go/ssa/stdlib_test.go
+++ b/go/ssa/stdlib_test.go
@@ -16,17 +16,17 @@
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 @@
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 @@
// 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 @@
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 0010ca4..61735dc 100644
--- a/internal/testenv/testenv.go
+++ b/internal/testenv/testenv.go
@@ -297,9 +297,3 @@
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 0000000..8ceef86
--- /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
+}
