Skip to content

Commit 36a5c6a

Browse files
timothy-kingtimothy-king
committed
go/ssa: build generic function bodies
ssa now always builds the bodies of generic functions and methods. Within the bodies of generic functions, parameterized types may appear in Instructions. When ssa.InstantiateGenerics is on, calls to generic functions with ground (non-parameterized) types are built as instantiations of the generic function bodies. Otherwise, calls to generic functions are built as a wrapper function that delegates to the generic function body. The ChangeType instruction can now represent a coercion to or from a parameterized type to an instance of the type. *ssa.Const values may now represent the zero value of any type. The build mode of go/analysis/passes/buildssa is again ssa.BuilderMode(0) (i.e. ssa.InstantiateGenerics is off). This change is a stack of already reviewed CLs. Fixes golang/go#48525 Change-Id: Ib516eba43963674c804a63e3dcdae6d4116353c9 Reviewed-on: https://go-review.googlesource.com/c/tools/+/425496 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Zvonimir Pavlinovic <[email protected]> Run-TryBot: Tim King <[email protected]> gopls-CI: kokoro <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
1 parent 85bf7a8 commit 36a5c6a

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+2405
-418
lines changed

Diff for: ‎go/analysis/passes/buildssa/buildssa.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
4848

4949
// Some Analyzers may need GlobalDebug, in which case we'll have
5050
// to set it globally, but let's wait till we need it.
51-
// Monomorphize at least until type parameters are available.
52-
mode := ssa.InstantiateGenerics
51+
mode := ssa.BuilderMode(0)
5352

5453
prog := ssa.NewProgram(pass.Fset, mode)
5554

Diff for: ‎go/analysis/passes/nilness/nilness.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ var Analyzer = &analysis.Analyzer{
6262

6363
func run(pass *analysis.Pass) (interface{}, error) {
6464
ssainput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
65-
// TODO(48525): ssainput.SrcFuncs is missing fn._Instances(). runFunc will be skipped.
6665
for _, fn := range ssainput.SrcFuncs {
6766
runFunc(pass, fn)
6867
}
@@ -307,9 +306,9 @@ func nilnessOf(stack []fact, v ssa.Value) nilness {
307306
return isnonnil
308307
case *ssa.Const:
309308
if v.IsNil() {
310-
return isnil
309+
return isnil // nil or zero value of a pointer-like type
311310
} else {
312-
return isnonnil
311+
return unknown // non-pointer
313312
}
314313
}
315314

Diff for: ‎go/analysis/passes/nilness/testdata/src/a/a.go

+7
Original file line numberDiff line numberDiff line change
@@ -209,3 +209,10 @@ func f13() {
209209
var d *Y
210210
print(d.value) // want "nil dereference in field selection"
211211
}
212+
213+
func f14() {
214+
var x struct{ f string }
215+
if x == struct{ f string }{} { // we don't catch this tautology as we restrict to reference types
216+
print(x)
217+
}
218+
}

Diff for: ‎go/analysis/passes/nilness/testdata/src/c/c.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package c
22

33
func instantiated[X any](x *X) int {
44
if x == nil {
5-
print(*x) // not reported until _Instances are added to SrcFuncs
5+
print(*x) // want "nil dereference in load"
66
}
77
return 1
88
}

Diff for: ‎go/callgraph/callgraph.go

+2
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ package callgraph // import "golang.org/x/tools/go/callgraph"
3737
// More generally, we could eliminate "uninteresting" nodes such as
3838
// nodes from packages we don't care about.
3939

40+
// TODO(zpavlinovic): decide how callgraphs handle calls to and from generic function bodies.
41+
4042
import (
4143
"fmt"
4244
"go/token"

Diff for: ‎go/callgraph/cha/cha.go

+2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
// partial programs, such as libraries without a main or test function.
2323
package cha // import "golang.org/x/tools/go/callgraph/cha"
2424

25+
// TODO(zpavlinovic): update CHA for how it handles generic function bodies.
26+
2527
import (
2628
"go/types"
2729

Diff for: ‎go/callgraph/cha/testdata/generics.go

+4
Original file line numberDiff line numberDiff line change
@@ -41,5 +41,9 @@ func f(h func(), g func(I), k func(A), a A, b B) {
4141
// f --> instantiated[main.A]
4242
// f --> instantiated[main.A]
4343
// f --> instantiated[main.B]
44+
// instantiated --> (*A).Foo
45+
// instantiated --> (*B).Foo
46+
// instantiated --> (A).Foo
47+
// instantiated --> (B).Foo
4448
// instantiated[main.A] --> (A).Foo
4549
// instantiated[main.B] --> (B).Foo

Diff for: ‎go/callgraph/static/static.go

+2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
// only static call edges.
77
package static // import "golang.org/x/tools/go/callgraph/static"
88

9+
// TODO(zpavlinovic): update static for how it handles generic function bodies.
10+
911
import (
1012
"golang.org/x/tools/go/callgraph"
1113
"golang.org/x/tools/go/ssa"

Diff for: ‎go/callgraph/vta/utils.go

+19-10
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"golang.org/x/tools/go/callgraph"
1111
"golang.org/x/tools/go/ssa"
12+
"golang.org/x/tools/internal/typeparams"
1213
)
1314

1415
func canAlias(n1, n2 node) bool {
@@ -117,19 +118,27 @@ func functionUnderPtr(t types.Type) types.Type {
117118
}
118119

119120
// sliceArrayElem returns the element type of type `t` that is
120-
// expected to be a (pointer to) array or slice, consistent with
121+
// expected to be a (pointer to) array, slice or string, consistent with
121122
// the ssa.Index and ssa.IndexAddr instructions. Panics otherwise.
122123
func sliceArrayElem(t types.Type) types.Type {
123-
u := t.Underlying()
124-
125-
if p, ok := u.(*types.Pointer); ok {
126-
u = p.Elem().Underlying()
127-
}
128-
129-
if a, ok := u.(*types.Array); ok {
130-
return a.Elem()
124+
switch u := t.Underlying().(type) {
125+
case *types.Pointer:
126+
return u.Elem().Underlying().(*types.Array).Elem()
127+
case *types.Array:
128+
return u.Elem()
129+
case *types.Slice:
130+
return u.Elem()
131+
case *types.Basic:
132+
return types.Typ[types.Byte]
133+
case *types.Interface: // type param.
134+
terms, err := typeparams.InterfaceTermSet(u)
135+
if err != nil || len(terms) == 0 {
136+
panic(t)
137+
}
138+
return sliceArrayElem(terms[0].Type()) // Element types must match.
139+
default:
140+
panic(t)
131141
}
132-
return u.(*types.Slice).Elem()
133142
}
134143

135144
// siteCallees computes a set of callees for call site `c` given program `callgraph`.

Diff for: ‎go/callgraph/vta/vta.go

+2
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@
5454
// reaching the node representing the call site to create a set of callees.
5555
package vta
5656

57+
// TODO(zpavlinovic): update VTA for how it handles generic function bodies and instantiation wrappers.
58+
5759
import (
5860
"go/types"
5961

Diff for: ‎go/pointer/analysis.go

+18-2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"runtime"
1717
"runtime/debug"
1818
"sort"
19+
"strings"
1920

2021
"golang.org/x/tools/go/callgraph"
2122
"golang.org/x/tools/go/ssa"
@@ -377,12 +378,27 @@ func (a *analysis) callEdge(caller *cgnode, site *callsite, calleeid nodeid) {
377378
fmt.Fprintf(a.log, "\tcall edge %s -> %s\n", site, callee)
378379
}
379380

380-
// Warn about calls to non-intrinsic external functions.
381+
// Warn about calls to functions that are handled unsoundly.
381382
// TODO(adonovan): de-dup these messages.
382-
if fn := callee.fn; fn.Blocks == nil && a.findIntrinsic(fn) == nil {
383+
fn := callee.fn
384+
385+
// Warn about calls to non-intrinsic external functions.
386+
if fn.Blocks == nil && a.findIntrinsic(fn) == nil {
383387
a.warnf(site.pos(), "unsound call to unknown intrinsic: %s", fn)
384388
a.warnf(fn.Pos(), " (declared here)")
385389
}
390+
391+
// Warn about calls to generic function bodies.
392+
if fn.TypeParams().Len() > 0 && len(fn.TypeArgs()) == 0 {
393+
a.warnf(site.pos(), "unsound call to generic function body: %s (build with ssa.InstantiateGenerics)", fn)
394+
a.warnf(fn.Pos(), " (declared here)")
395+
}
396+
397+
// Warn about calls to instantiation wrappers of generics functions.
398+
if fn.Origin() != nil && strings.HasPrefix(fn.Synthetic, "instantiation wrapper ") {
399+
a.warnf(site.pos(), "unsound call to instantiation wrapper of generic: %s (build with ssa.InstantiateGenerics)", fn)
400+
a.warnf(fn.Pos(), " (declared here)")
401+
}
386402
}
387403

388404
// dumpSolution writes the PTS solution to the specified file.

Diff for: ‎go/pointer/api.go

+4
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,11 @@ type Config struct {
2828
// dependencies of any main package may still affect the
2929
// analysis result, because they contribute runtime types and
3030
// thus methods.
31+
//
3132
// TODO(adonovan): investigate whether this is desirable.
33+
//
34+
// Calls to generic functions will be unsound unless packages
35+
// are built using the ssa.InstantiateGenerics builder mode.
3236
Mains []*ssa.Package
3337

3438
// Reflection determines whether to handle reflection

Diff for: ‎go/pointer/doc.go

+8
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,14 @@ A. Control-flow joins would merge interfaces ({T1}, {V1}) and ({T2},
358358
type-unsafe combination (T1,V2). Treating the value and its concrete
359359
type as inseparable makes the analysis type-safe.)
360360
361+
Type parameters:
362+
363+
Type parameters are not directly supported by the analysis.
364+
Calls to generic functions will be left as if they had empty bodies.
365+
Users of the package are expected to use the ssa.InstantiateGenerics
366+
builder mode when building code that uses or depends on code
367+
containing generics.
368+
361369
reflect.Value:
362370
363371
A reflect.Value is modelled very similar to an interface{}, i.e. as

Diff for: ‎go/pointer/gen.go

+19-1
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@ import (
1414
"fmt"
1515
"go/token"
1616
"go/types"
17+
"strings"
1718

1819
"golang.org/x/tools/go/callgraph"
1920
"golang.org/x/tools/go/ssa"
21+
"golang.org/x/tools/internal/typeparams"
2022
)
2123

2224
var (
@@ -978,7 +980,10 @@ func (a *analysis) genInstr(cgn *cgnode, instr ssa.Instruction) {
978980
a.sizeof(instr.Type()))
979981

980982
case *ssa.Index:
981-
a.copy(a.valueNode(instr), 1+a.valueNode(instr.X), a.sizeof(instr.Type()))
983+
_, isstring := typeparams.CoreType(instr.X.Type()).(*types.Basic)
984+
if !isstring {
985+
a.copy(a.valueNode(instr), 1+a.valueNode(instr.X), a.sizeof(instr.Type()))
986+
}
982987

983988
case *ssa.Select:
984989
recv := a.valueOffsetNode(instr, 2) // instr : (index, recvOk, recv0, ... recv_n-1)
@@ -1202,6 +1207,19 @@ func (a *analysis) genFunc(cgn *cgnode) {
12021207
return
12031208
}
12041209

1210+
if fn.TypeParams().Len() > 0 && len(fn.TypeArgs()) == 0 {
1211+
// Body of generic function.
1212+
// We'll warn about calls to such functions at the end.
1213+
return
1214+
}
1215+
1216+
if strings.HasPrefix(fn.Synthetic, "instantiation wrapper ") {
1217+
// instantiation wrapper of a generic function.
1218+
// These may contain type coercions which are not currently supported.
1219+
// We'll warn about calls to such functions at the end.
1220+
return
1221+
}
1222+
12051223
if a.log != nil {
12061224
fmt.Fprintln(a.log, "; Creating nodes for local values")
12071225
}

Diff for: ‎go/pointer/pointer_test.go

+15-1
Original file line numberDiff line numberDiff line change
@@ -240,9 +240,14 @@ func doOneInput(t *testing.T, input, fpath string) bool {
240240
// Find all calls to the built-in print(x). Analytically,
241241
// print is a no-op, but it's a convenient hook for testing
242242
// the PTS of an expression, so our tests use it.
243+
// Exclude generic bodies as these should be dead code for pointer.
244+
// Instance of generics are included.
243245
probes := make(map[*ssa.CallCommon]bool)
244246
for fn := range ssautil.AllFunctions(prog) {
245-
// TODO(taking): Switch to a more principled check like fn.declaredPackage() == mainPkg if _Origin is exported.
247+
if isGenericBody(fn) {
248+
continue // skip generic bodies
249+
}
250+
// TODO(taking): Switch to a more principled check like fn.declaredPackage() == mainPkg if Origin is exported.
246251
if fn.Pkg == mainpkg || (fn.Pkg == nil && mainFiles[prog.Fset.File(fn.Pos())]) {
247252
for _, b := range fn.Blocks {
248253
for _, instr := range b.Instrs {
@@ -656,6 +661,15 @@ func TestInput(t *testing.T) {
656661
}
657662
}
658663

664+
// isGenericBody returns true if fn is the body of a generic function.
665+
func isGenericBody(fn *ssa.Function) bool {
666+
sig := fn.Signature
667+
if typeparams.ForSignature(sig).Len() > 0 || typeparams.RecvTypeParams(sig).Len() > 0 {
668+
return fn.Synthetic == ""
669+
}
670+
return false
671+
}
672+
659673
// join joins the elements of multiset with " | "s.
660674
func join(set map[string]int) string {
661675
var buf bytes.Buffer

Diff for: ‎go/pointer/reflect.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -1024,7 +1024,7 @@ func ext۰reflect۰ChanOf(a *analysis, cgn *cgnode) {
10241024
var dir reflect.ChanDir // unknown
10251025
if site := cgn.callersite; site != nil {
10261026
if c, ok := site.instr.Common().Args[0].(*ssa.Const); ok {
1027-
v, _ := constant.Int64Val(c.Value)
1027+
v := c.Int64()
10281028
if 0 <= v && v <= int64(reflect.BothDir) {
10291029
dir = reflect.ChanDir(v)
10301030
}
@@ -1751,8 +1751,7 @@ func ext۰reflect۰rtype۰InOut(a *analysis, cgn *cgnode, out bool) {
17511751
index := -1
17521752
if site := cgn.callersite; site != nil {
17531753
if c, ok := site.instr.Common().Args[0].(*ssa.Const); ok {
1754-
v, _ := constant.Int64Val(c.Value)
1755-
index = int(v)
1754+
index = int(c.Int64())
17561755
}
17571756
}
17581757
a.addConstraint(&rtypeInOutConstraint{

Diff for: ‎go/pointer/util.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,13 @@ import (
88
"bytes"
99
"fmt"
1010
"go/types"
11-
exec "golang.org/x/sys/execabs"
1211
"log"
1312
"os"
1413
"runtime"
1514
"time"
1615

16+
exec "golang.org/x/sys/execabs"
17+
1718
"golang.org/x/tools/container/intsets"
1819
)
1920

@@ -125,7 +126,7 @@ func (a *analysis) flatten(t types.Type) []*fieldInfo {
125126
// Debuggability hack: don't remove
126127
// the named type from interfaces as
127128
// they're very verbose.
128-
fl = append(fl, &fieldInfo{typ: t})
129+
fl = append(fl, &fieldInfo{typ: t}) // t may be a type param
129130
} else {
130131
fl = a.flatten(u)
131132
}

Diff for: ‎go/ssa/TODO

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
-*- text -*-
2+
3+
SSA Generics to-do list
4+
===========================
5+
6+
DOCUMENTATION:
7+
- Read me for internals
8+
9+
TYPE PARAMETERIZED GENERIC FUNCTIONS:
10+
- sanity.go updates.
11+
- Check source functions going to generics.
12+
- Tests, tests, tests...
13+
14+
USAGE:
15+
- Back fill users for handling ssa.InstantiateGenerics being off.
16+

0 commit comments

Comments
 (0)