From 010220ae3d82e34f84b3d15690f219996df26c3f Mon Sep 17 00:00:00 2001 From: Grant Nelson Date: Thu, 21 Nov 2024 09:44:49 -0700 Subject: [PATCH] fixing some issues in analysis around generics --- compiler/functions.go | 2 +- compiler/internal/analysis/defer.go | 19 +- compiler/internal/analysis/info.go | 94 ++++-- compiler/internal/analysis/info_test.go | 406 +++++++++++++++++------ compiler/internal/typeparams/map.go | 18 +- compiler/internal/typeparams/map_test.go | 2 +- compiler/typesutil/typelist.go | 13 + 7 files changed, 393 insertions(+), 161 deletions(-) diff --git a/compiler/functions.go b/compiler/functions.go index 86d3916bf..592992efc 100644 --- a/compiler/functions.go +++ b/compiler/functions.go @@ -82,7 +82,7 @@ func (fc *funcContext) namedFuncContext(inst typeparams.Instance) *funcContext { // go/types doesn't generate *types.Func objects for function literals, we // generate a synthetic one for it. func (fc *funcContext) literalFuncContext(fun *ast.FuncLit) *funcContext { - info := fc.pkgCtx.FuncLitInfo(fun) + info := fc.pkgCtx.FuncLitInfo(fun, fc.TypeArgs()) sig := fc.pkgCtx.TypeOf(fun).(*types.Signature) o := types.NewFunc(fun.Pos(), fc.pkgCtx.Pkg, fc.newLitFuncName(), sig) inst := typeparams.Instance{Object: o} diff --git a/compiler/internal/analysis/defer.go b/compiler/internal/analysis/defer.go index 36b19ef2a..65d153d6e 100644 --- a/compiler/internal/analysis/defer.go +++ b/compiler/internal/analysis/defer.go @@ -2,8 +2,10 @@ package analysis import ( "go/ast" + "go/types" "github.com/gopherjs/gopherjs/compiler/internal/typeparams" + "github.com/gopherjs/gopherjs/compiler/typesutil" ) // deferStmt represents a defer statement that is blocking or not. @@ -49,8 +51,9 @@ import ( // // [CFG]: https://en.wikipedia.org/wiki/Control-flow_graph type deferStmt struct { - inst *typeparams.Instance - lit *ast.FuncLit + obj types.Object + lit *ast.FuncLit + typeArgs typesutil.TypeList } // newBlockingDefer creates a new defer statement that is blocking. @@ -65,13 +68,13 @@ func newBlockingDefer() *deferStmt { // newInstDefer creates a new defer statement for an instances of a method. // The instance is used to look up the blocking information later. func newInstDefer(inst typeparams.Instance) *deferStmt { - return &deferStmt{inst: &inst} + return &deferStmt{obj: inst.Object, typeArgs: inst.TArgs} } // newLitDefer creates a new defer statement for a function literal. // The literal is used to look up the blocking information later. -func newLitDefer(lit *ast.FuncLit) *deferStmt { - return &deferStmt{lit: lit} +func newLitDefer(lit *ast.FuncLit, typeArgs typesutil.TypeList) *deferStmt { + return &deferStmt{lit: lit, typeArgs: typeArgs} } // IsBlocking determines if the defer statement is blocking or not. @@ -79,11 +82,11 @@ func (d *deferStmt) IsBlocking(info *Info) bool { // If the instance or the literal is set then we can look up the blocking, // otherwise assume blocking because otherwise the defer wouldn't // have been recorded. - if d.inst != nil { - return info.FuncInfo(*d.inst).IsBlocking() + if d.obj != nil { + return info.IsBlocking(typeparams.Instance{Object: d.obj, TArgs: d.typeArgs}) } if d.lit != nil { - return info.FuncLitInfo(d.lit).IsBlocking() + return info.FuncLitInfo(d.lit, d.typeArgs).IsBlocking() } return true } diff --git a/compiler/internal/analysis/info.go b/compiler/internal/analysis/info.go index 42918258e..803952b24 100644 --- a/compiler/internal/analysis/info.go +++ b/compiler/internal/analysis/info.go @@ -55,22 +55,24 @@ type Info struct { instanceSets *typeparams.PackageInstanceSets HasPointer map[*types.Var]bool funcInstInfos *typeparams.InstanceMap[*FuncInfo] - funcLitInfos map[*ast.FuncLit]*FuncInfo + funcLitInfos map[*ast.FuncLit][]*FuncInfo InitFuncInfo *FuncInfo // Context for package variable initialization. isImportedBlocking func(typeparams.Instance) bool // For functions from other packages. allInfos []*FuncInfo } -func (info *Info) newFuncInfo(n ast.Node, inst *typeparams.Instance) *FuncInfo { +func (info *Info) newFuncInfo(n ast.Node, obj types.Object, typeArgs typesutil.TypeList, resolver *typeparams.Resolver) *FuncInfo { funcInfo := &FuncInfo{ pkgInfo: info, Flattened: make(map[ast.Node]bool), Blocking: make(map[ast.Node]bool), GotoLabel: make(map[*types.Label]bool), loopReturnIndex: -1, - localInstCallees: new(typeparams.InstanceMap[[]astPath]), + instCallees: new(typeparams.InstanceMap[[]astPath]), literalFuncCallees: make(map[*ast.FuncLit]astPath), + typeArgs: typeArgs, + resolver: resolver, } // Register the function in the appropriate map. @@ -86,13 +88,14 @@ func (info *Info) newFuncInfo(n ast.Node, inst *typeparams.Instance) *FuncInfo { funcInfo.Blocking[n] = true } - if inst == nil { - inst = &typeparams.Instance{Object: info.Defs[n.Name]} + if obj == nil { + obj = info.Defs[n.Name] } - info.funcInstInfos.Set(*inst, funcInfo) + inst := typeparams.Instance{Object: obj, TArgs: typeArgs} + info.funcInstInfos.Set(inst, funcInfo) case *ast.FuncLit: - info.funcLitInfos[n] = funcInfo + info.funcLitInfos[n] = append(info.funcLitInfos[n], funcInfo) } // And add it to the list of all functions. @@ -105,28 +108,40 @@ func (info *Info) newFuncInfoInstances(fd *ast.FuncDecl) []*FuncInfo { obj := info.Defs[fd.Name] instances := info.instanceSets.Pkg(info.Pkg).ForObj(obj) if len(instances) == 0 { - // No instances found, this is a non-generic function. - return []*FuncInfo{info.newFuncInfo(fd, nil)} + if typeparams.HasTypeParams(obj.Type()) { + // This is a generic function, but no instances were found, + // this is an unused function, so skip over it. + return []*FuncInfo{} + } + + // No instances found and this is a non-generic function. + return []*FuncInfo{info.newFuncInfo(fd, nil, nil, nil)} } funcInfos := make([]*FuncInfo, 0, len(instances)) for _, inst := range instances { - fi := info.newFuncInfo(fd, &inst) + var resolver *typeparams.Resolver if sig, ok := obj.Type().(*types.Signature); ok { tp := typeparams.ToSlice(typeparams.SignatureTypeParams(sig)) - fi.resolver = typeparams.NewResolver(info.typeCtx, tp, inst.TArgs) + resolver = typeparams.NewResolver(info.typeCtx, tp, inst.TArgs) } + fi := info.newFuncInfo(fd, inst.Object, inst.TArgs, resolver) funcInfos = append(funcInfos, fi) } return funcInfos } // IsBlocking returns true if the function may contain blocking calls or operations. +// If inst is from a different package, this will use the isImportedBlocking +// to lookup the information from the other package. func (info *Info) IsBlocking(inst typeparams.Instance) bool { + if inst.Object.Pkg() != info.Pkg { + return info.isImportedBlocking(inst) + } if funInfo := info.FuncInfo(inst); funInfo != nil { return funInfo.IsBlocking() } - panic(fmt.Errorf(`info did not have function declaration instance for %q`, inst)) + panic(fmt.Errorf(`info did not have function declaration instance for %q`, inst.TypeString())) } // FuncInfo returns information about the given function declaration instance, or nil if not found. @@ -135,8 +150,16 @@ func (info *Info) FuncInfo(inst typeparams.Instance) *FuncInfo { } // FuncLitInfo returns information about the given function literal, or nil if not found. -func (info *Info) FuncLitInfo(fun *ast.FuncLit) *FuncInfo { - return info.funcLitInfos[fun] +// The given type arguments are used to identify the correct instance of the +// function literal in the case the literal was defined inside a generic function. +func (info *Info) FuncLitInfo(fun *ast.FuncLit, typeArgs typesutil.TypeList) *FuncInfo { + lits := info.funcLitInfos[fun] + for _, fi := range lits { + if fi.typeArgs.Equal(typeArgs) { + return fi + } + } + return nil } // VarsWithInitializers returns a set of package-level variables that have @@ -160,9 +183,9 @@ func AnalyzePkg(files []*ast.File, fileSet *token.FileSet, typesInfo *types.Info HasPointer: make(map[*types.Var]bool), isImportedBlocking: isBlocking, funcInstInfos: new(typeparams.InstanceMap[*FuncInfo]), - funcLitInfos: make(map[*ast.FuncLit]*FuncInfo), + funcLitInfos: make(map[*ast.FuncLit][]*FuncInfo), } - info.InitFuncInfo = info.newFuncInfo(nil, nil) + info.InitFuncInfo = info.newFuncInfo(nil, nil, nil, nil) // Traverse the full AST of the package and collect information about existing // functions. @@ -190,19 +213,19 @@ func (info *Info) propagateFunctionBlocking() bool { done := true for _, caller := range info.allInfos { // Check calls to named functions and function-typed variables. - caller.localInstCallees.Iterate(func(callee typeparams.Instance, callSites []astPath) { - if info.FuncInfo(callee).IsBlocking() { + caller.instCallees.Iterate(func(callee typeparams.Instance, callSites []astPath) { + if info.IsBlocking(callee) { for _, callSite := range callSites { caller.markBlocking(callSite) } - caller.localInstCallees.Delete(callee) + caller.instCallees.Delete(callee) done = false } }) // Check direct calls to function literals. for callee, callSite := range caller.literalFuncCallees { - if info.FuncLitInfo(callee).IsBlocking() { + if info.FuncLitInfo(callee, caller.typeArgs).IsBlocking() { caller.markBlocking(callSite) delete(caller.literalFuncCallees, callee) done = false @@ -250,15 +273,18 @@ type FuncInfo struct { // returns defined before any defers in a loop may still be affected by // those defers because of the loop. See comment on [deferStmt]. loopReturnIndex int - // List of other named functions from the current package this function calls. + // List of other named functions in the current package or another package + // that this function calls. // If any of them are blocking, this function will become blocking too. - localInstCallees *typeparams.InstanceMap[[]astPath] + instCallees *typeparams.InstanceMap[[]astPath] // List of function literals directly called from this function (for example: // `func() { /* do stuff */ }()`). This is distinct from function literals // assigned to named variables (for example: `doStuff := func() {}; // doStuff()`), which are handled by localInstCallees. If any of them are // identified as blocking, this function will become blocking too. literalFuncCallees map[*ast.FuncLit]astPath + // typeArgs are the type arguments for the function instance. + typeArgs typesutil.TypeList // resolver is used by this function instance to resolve any type arguments // for internal function calls. // This may be nil if not an instance of a generic function. @@ -276,6 +302,12 @@ func (fi *FuncInfo) IsBlocking() bool { return fi == nil || len(fi.Blocking) != 0 } +// TypeArgs gets the type arguments of this inside of a function instance +// or empty if not in a function instance. +func (fi *FuncInfo) TypeArgs() typesutil.TypeList { + return fi.typeArgs +} + // propagateReturnBlocking updates the blocking on the return statements. // See comment on [deferStmt]. // @@ -341,7 +373,7 @@ func (fi *FuncInfo) Visit(node ast.Node) ast.Visitor { return nil case *ast.FuncLit: // Analyze the function literal in its own context. - return fi.pkgInfo.newFuncInfo(n, nil) + return fi.pkgInfo.newFuncInfo(n, nil, fi.typeArgs, fi.resolver) case *ast.BranchStmt: switch n.Tok { case token.GOTO: @@ -502,7 +534,7 @@ func (fi *FuncInfo) visitCallExpr(n *ast.CallExpr, deferredCall bool) ast.Visito // Register literal function call site in case it is identified as blocking. fi.literalFuncCallees[f] = fi.visitorStack.copy() if deferredCall { - fi.deferStmts = append(fi.deferStmts, newLitDefer(f)) + fi.deferStmts = append(fi.deferStmts, newLitDefer(f, fi.typeArgs)) } return nil // No need to walk under this CallExpr, we already did it manually. case *ast.IndexExpr: @@ -610,21 +642,11 @@ func (fi *FuncInfo) callToNamedFunc(callee typeparams.Instance, deferredCall boo } } - if o.Pkg() != fi.pkgInfo.Pkg { - if fi.pkgInfo.isImportedBlocking(callee) { - fi.markBlocking(fi.visitorStack) - if deferredCall { - fi.deferStmts = append(fi.deferStmts, newBlockingDefer()) - } - } - return - } - // We probably don't know yet whether the callee function is blocking. // Record the calls site for the later stage. - paths := fi.localInstCallees.Get(callee) + paths := fi.instCallees.Get(callee) paths = append(paths, fi.visitorStack.copy()) - fi.localInstCallees.Set(callee, paths) + fi.instCallees.Set(callee, paths) if deferredCall { fi.deferStmts = append(fi.deferStmts, newInstDefer(callee)) } diff --git a/compiler/internal/analysis/info_test.go b/compiler/internal/analysis/info_test.go index d4e21b501..00c1d1623 100644 --- a/compiler/internal/analysis/info_test.go +++ b/compiler/internal/analysis/info_test.go @@ -156,10 +156,10 @@ func TestBlocking_GoRoutines_WithFuncLiterals(t *testing.T) { }(<-c) }`) bt.assertNotBlocking(`notBlocking`) - bt.assertBlockingLit(4) + bt.assertBlockingLit(4, ``) bt.assertBlocking(`blocking`) - bt.assertNotBlockingLit(10) + bt.assertNotBlockingLit(10, ``) } func TestBlocking_GoRoutines_WithNamedFuncs(t *testing.T) { @@ -210,13 +210,13 @@ func TestBlocking_Defers_WithoutReturns_WithFuncLiterals(t *testing.T) { }(true) }`) bt.assertBlocking(`blockingBody`) - bt.assertBlockingLit(4) + bt.assertBlockingLit(4, ``) bt.assertBlocking(`blockingArg`) - bt.assertNotBlockingLit(10) + bt.assertNotBlockingLit(10, ``) bt.assertNotBlocking(`notBlocking`) - bt.assertNotBlockingLit(16) + bt.assertNotBlockingLit(16, ``) } func TestBlocking_Defers_WithoutReturns_WithNamedFuncs(t *testing.T) { @@ -278,13 +278,13 @@ func TestBlocking_Defers_WithReturns_WithFuncLiterals(t *testing.T) { return 42 }`) bt.assertBlocking(`blockingBody`) - bt.assertBlockingLit(4) + bt.assertBlockingLit(4, ``) bt.assertBlocking(`blockingArg`) - bt.assertNotBlockingLit(11) + bt.assertNotBlockingLit(11, ``) bt.assertNotBlocking(`notBlocking`) - bt.assertNotBlockingLit(18) + bt.assertNotBlockingLit(18, ``) } func TestBlocking_Defers_WithReturns_WithNamedFuncs(t *testing.T) { @@ -317,15 +317,15 @@ func TestBlocking_Defers_WithReturns_WithNamedFuncs(t *testing.T) { bt.assertNotBlocking(`nonBlockingPrint`) bt.assertBlocking(`blockingBody`) - bt.assertBlockingReturn(13) + bt.assertBlockingReturn(13, ``) bt.assertBlocking(`blockingArg`) // The defer is non-blocking so the return is not blocking // even though the function is blocking. - bt.assertNotBlockingReturn(18) + bt.assertNotBlockingReturn(18, ``) bt.assertNotBlocking(`notBlocking`) - bt.assertNotBlockingReturn(23) + bt.assertNotBlockingReturn(23, ``) } func TestBlocking_Defers_WithMultipleReturns(t *testing.T) { @@ -363,15 +363,15 @@ func TestBlocking_Defers_WithMultipleReturns(t *testing.T) { return true // line 31 }`) bt.assertBlocking(`foo`) - bt.assertNotBlockingLit(4) + bt.assertNotBlockingLit(4, ``) // Early escape from function without blocking defers is not blocking. - bt.assertNotBlockingReturn(11) - bt.assertNotBlockingLit(14) + bt.assertNotBlockingReturn(11, ``) + bt.assertNotBlockingLit(14, ``) // Function has had blocking by this point but no blocking defers yet. - bt.assertNotBlockingReturn(20) - bt.assertBlockingLit(24) + bt.assertNotBlockingReturn(20, ``) + bt.assertBlockingLit(24, ``) // The return is blocking because of a blocking defer. - bt.assertBlockingReturn(28) + bt.assertBlockingReturn(28, ``) // Technically the return on line 31 is not blocking since the defer that // is blocking can only exit through the return on line 28, but it would be // difficult to determine which defers would only affect certain returns @@ -384,7 +384,7 @@ func TestBlocking_Defers_WithMultipleReturns(t *testing.T) { // // For now we simply build up the list of defers as we go making // the return on line 31 also blocking. - bt.assertBlockingReturn(31) + bt.assertBlockingReturn(31, ``) } func TestBlocking_Defers_WithReturnsAndDefaultBlocking(t *testing.T) { @@ -453,12 +453,12 @@ func TestBlocking_Defers_WithReturnsAndDefaultBlocking(t *testing.T) { bt.assertBlocking(`deferMappedFuncCall`) // All of these returns are blocking because they have blocking defers. - bt.assertBlockingReturn(17) - bt.assertBlockingReturn(22) - bt.assertBlockingReturn(28) - bt.assertBlockingReturn(34) - bt.assertBlockingReturn(40) - bt.assertBlockingReturn(49) + bt.assertBlockingReturn(17, ``) + bt.assertBlockingReturn(22, ``) + bt.assertBlockingReturn(28, ``) + bt.assertBlockingReturn(34, ``) + bt.assertBlockingReturn(40, ``) + bt.assertBlockingReturn(49, ``) } func TestBlocking_Defers_WithReturnsAndDeferBuiltin(t *testing.T) { @@ -477,7 +477,7 @@ func TestBlocking_Defers_WithReturnsAndDeferBuiltin(t *testing.T) { bt.assertFuncInstCount(1) bt.assertNotBlocking(`deferBuiltinCall`) - bt.assertNotBlockingReturn(10) + bt.assertNotBlockingReturn(10, ``) } func TestBlocking_Defers_WithReturnsInLoops(t *testing.T) { @@ -575,14 +575,14 @@ func TestBlocking_Defers_WithReturnsInLoops(t *testing.T) { // When the following 2 returns are defined there are no defers, however, // because of the loop, the blocking defers defined after the return will // block the returns. - bt.assertBlockingReturn(12) - bt.assertBlockingReturn(22) - bt.assertBlockingReturn(31) - bt.assertBlockingReturn(44) - bt.assertBlockingReturn(52) - bt.assertBlockingReturn(66) - bt.assertBlockingReturn(73) - bt.assertBlockingReturn(77) + bt.assertBlockingReturn(12, ``) + bt.assertBlockingReturn(22, ``) + bt.assertBlockingReturn(31, ``) + bt.assertBlockingReturn(44, ``) + bt.assertBlockingReturn(52, ``) + bt.assertBlockingReturn(66, ``) + bt.assertBlockingReturn(73, ``) + bt.assertBlockingReturn(77, ``) } func TestBlocking_Defers_WithReturnsInLoopsInLoops(t *testing.T) { @@ -652,19 +652,19 @@ func TestBlocking_Defers_WithReturnsInLoopsInLoops(t *testing.T) { bt.assertFuncInstCount(4) bt.assertBlocking(`blocking`) bt.assertBlocking(`forLoopTheLoop`) - bt.assertNotBlockingReturn(9) - bt.assertBlockingReturn(13) - bt.assertBlockingReturn(17) - bt.assertBlockingReturn(21) - bt.assertBlockingReturn(25) - bt.assertBlockingReturn(28) + bt.assertNotBlockingReturn(9, ``) + bt.assertBlockingReturn(13, ``) + bt.assertBlockingReturn(17, ``) + bt.assertBlockingReturn(21, ``) + bt.assertBlockingReturn(25, ``) + bt.assertBlockingReturn(28, ``) bt.assertBlocking(`rangeLoopTheLoop`) - bt.assertBlockingReturn(36) - bt.assertBlockingReturn(41) + bt.assertBlockingReturn(36, ``) + bt.assertBlockingReturn(41, ``) bt.assertBlocking(`noopThenLoop`) - bt.assertNotBlockingReturn(48) - bt.assertBlockingReturn(54) - bt.assertBlockingReturn(58) + bt.assertNotBlockingReturn(48, ``) + bt.assertBlockingReturn(54, ``) + bt.assertBlockingReturn(58, ``) } func TestBlocking_Returns_WithoutDefers(t *testing.T) { @@ -693,19 +693,67 @@ func TestBlocking_Returns_WithoutDefers(t *testing.T) { return true // line 22 }`) bt.assertBlocking(`blocking`) - bt.assertBlockingReturn(4) + bt.assertBlockingReturn(4, ``) bt.assertBlocking(`blockingBeforeReturn`) - bt.assertNotBlockingReturn(9) + bt.assertNotBlockingReturn(9, ``) bt.assertBlocking(`indirectlyBlocking`) - bt.assertBlockingReturn(13) + bt.assertBlockingReturn(13, ``) bt.assertBlocking(`indirectlyBlockingBeforeReturn`) - bt.assertNotBlockingReturn(18) + bt.assertNotBlockingReturn(18, ``) bt.assertNotBlocking(`notBlocking`) - bt.assertNotBlockingReturn(22) + bt.assertNotBlockingReturn(22, ``) +} + +func TestBlocking_Defers_WithReturnsInInstances(t *testing.T) { + // This is an example of a deferred function literal inside of + // an instance of a generic function affecting the return + // differently based on the type arguments of the instance. + bt := newBlockingTest(t, + `package test + + type BazBlocker struct { + c chan bool + } + func (bb BazBlocker) Baz() { + println(<-bb.c) + } + + type BazNotBlocker struct {} + func (bnb BazNotBlocker) Baz() { + println("hi") + } + + type Foo interface { Baz() } + func FooBaz[T Foo]() bool { + defer func() { // line 17 + var foo T + foo.Baz() + }() + return true // line 21 + } + + func main() { + FooBaz[BazBlocker]() + FooBaz[BazNotBlocker]() + }`) + + bt.assertFuncInstCount(5) + bt.assertBlocking(`BazBlocker.Baz`) + bt.assertNotBlocking(`BazNotBlocker.Baz`) + bt.assertBlockingInst(`pkg/test.FooBaz`) + bt.assertNotBlockingInst(`pkg/test.FooBaz`) + bt.assertBlocking(`main`) + + bt.assertFuncLitCount(2) + bt.assertBlockingLit(17, `pkg/test.BazBlocker`) + bt.assertNotBlockingLit(17, `pkg/test.BazNotBlocker`) + + bt.assertBlockingReturn(21, `pkg/test.BazBlocker`) + bt.assertNotBlockingReturn(21, `pkg/test.BazNotBlocker`) } func TestBlocking_Defers_WithReturnsAndOtherPackages(t *testing.T) { @@ -737,10 +785,10 @@ func TestBlocking_Defers_WithReturnsAndOtherPackages(t *testing.T) { bt := newBlockingTestWithOtherPackage(t, testSrc, otherSrc) bt.assertBlocking(`deferOtherBlocking`) - bt.assertBlockingReturn(7) + bt.assertBlockingReturn(7, ``) bt.assertNotBlocking(`deferOtherNotBlocking`) - bt.assertNotBlockingReturn(12) + bt.assertNotBlockingReturn(12, ``) } func TestBlocking_FunctionLiteral(t *testing.T) { @@ -770,13 +818,13 @@ func TestBlocking_FunctionLiteral(t *testing.T) { bt.assertBlocking(`blocking`) bt.assertBlocking(`indirectlyBlocking`) - bt.assertBlockingLit(9) + bt.assertBlockingLit(9, ``) bt.assertBlocking(`directlyBlocking`) - bt.assertBlockingLit(13) + bt.assertBlockingLit(13, ``) bt.assertNotBlocking(`notBlocking`) - bt.assertNotBlockingLit(20) + bt.assertNotBlockingLit(20, ``) } func TestBlocking_LinkedFunction(t *testing.T) { @@ -818,9 +866,9 @@ func TestBlocking_Instances_WithSingleTypeArg(t *testing.T) { // blocking and notBlocking as generics do not have FuncInfo, // only non-generic and instances have FuncInfo. - bt.assertBlockingInst(`test.blocking[int]`) + bt.assertBlockingInst(`pkg/test.blocking`) bt.assertBlocking(`bInt`) - bt.assertNotBlockingInst(`test.notBlocking[uint]`) + bt.assertNotBlockingInst(`pkg/test.notBlocking`) bt.assertNotBlocking(`nbUint`) } @@ -849,9 +897,9 @@ func TestBlocking_Instances_WithMultipleTypeArgs(t *testing.T) { // blocking and notBlocking as generics do not have FuncInfo, // only non-generic and instances have FuncInfo. - bt.assertBlockingInst(`test.blocking[string, int, map[string]int]`) + bt.assertBlockingInst(`pkg/test.blocking`) bt.assertBlocking(`bInt`) - bt.assertNotBlockingInst(`test.notBlocking[string, uint, map[string]uint]`) + bt.assertNotBlockingInst(`pkg/test.notBlocking`) bt.assertNotBlocking(`nbUint`) } @@ -1075,7 +1123,7 @@ func TestBlocking_VarFunctionCall(t *testing.T) { func bar() { foo() }`) - bt.assertNotBlockingLit(3) + bt.assertNotBlockingLit(3, ``) bt.assertBlocking(`bar`) } @@ -1233,12 +1281,12 @@ func TestBlocking_InstantiationBlocking(t *testing.T) { bt.assertBlocking(`BazBlocker.Baz`) bt.assertBlocking(`blockingViaExplicit`) bt.assertBlocking(`blockingViaImplicit`) - bt.assertBlockingInst(`test.FooBaz[pkg/test.BazBlocker]`) + bt.assertBlockingInst(`pkg/test.FooBaz`) bt.assertNotBlocking(`BazNotBlocker.Baz`) bt.assertNotBlocking(`notBlockingViaExplicit`) bt.assertNotBlocking(`notBlockingViaImplicit`) - bt.assertNotBlockingInst(`test.FooBaz[pkg/test.BazNotBlocker]`) + bt.assertNotBlockingInst(`pkg/test.FooBaz`) } func TestBlocking_NestedInstantiations(t *testing.T) { @@ -1272,12 +1320,129 @@ func TestBlocking_NestedInstantiations(t *testing.T) { bt.assertFuncInstCount(8) bt.assertNotBlocking(`bazInt`) bt.assertNotBlocking(`bazString`) - bt.assertNotBlockingInst(`test.Foo[map[int]int]`) - bt.assertNotBlockingInst(`test.Foo[map[int]string]`) - bt.assertNotBlockingInst(`test.Bar[int, int, map[int]int]`) - bt.assertNotBlockingInst(`test.Bar[int, string, map[int]string]`) - bt.assertNotBlockingInst(`test.Baz[int, []int]`) - bt.assertNotBlockingInst(`test.Baz[string, []string]`) + bt.assertNotBlockingInst(`pkg/test.Foo`) + bt.assertNotBlockingInst(`pkg/test.Foo`) + bt.assertNotBlockingInst(`pkg/test.Bar`) + bt.assertNotBlockingInst(`pkg/test.Bar`) + bt.assertNotBlockingInst(`pkg/test.Baz`) + bt.assertNotBlockingInst(`pkg/test.Baz`) +} + +func TestBlocking_UnusedGenericFunctions(t *testing.T) { + // Checking that the type parameters are being propagated down into callee. + // This is based off of go1.19.13/test/typeparam/orderedmap.go + bt := newBlockingTest(t, + `package test + + type node[K, V any] struct { + key K + val V + left, right *node[K, V] + } + + type Tree[K, V any] struct { + root *node[K, V] + eq func(K, K) bool + } + + func New[K, V any](eq func(K, K) bool) *Tree[K, V] { + return &Tree[K, V]{eq: eq} + } + + func NewStrKey[K ~string, V any]() *Tree[K, V] { // unused + return New[K, V](func(k1, k2 K) bool { + return string(k1) == string(k2) + }) + } + + func NewStrStr[V any]() *Tree[string, V] { // unused + return NewStrKey[string, V]() + } + + func main() { + t := New[int, string](func(k1, k2 int) bool { + return k1 == k2 + }) + println(t) + }`) + bt.assertFuncInstCount(2) + // Notice that `NewStrKey` and `NewStrStr` are not called so doesn't have + // any known instances and therefore they don't have any FuncInfos. + bt.assertNotBlockingInst(`pkg/test.New`) + bt.assertNotBlocking(`main`) +} + +func TestBlocking_LitInstanceCalls(t *testing.T) { + // Literals defined inside a generic function must inherit the + // type arguments (resolver) of the enclosing instance it is defined in + // so that things like calls to other generic functions create the + // call to the correct concrete instance. + bt := newBlockingTest(t, + `package test + + func foo[T any](x T) { + println(x) + } + + func bar[T any](x T) { + f := func(v T) { // line 8 + foo[T](v) + } + f(x) + } + + func main() { + bar[int](42) + bar[float64](3.14) + }`) + bt.assertFuncInstCount(5) + + bt.assertNotBlockingInst(`pkg/test.foo`) + bt.assertNotBlockingInst(`pkg/test.foo`) + bt.assertNotBlockingLit(8, `int`) + bt.assertNotBlockingLit(8, `float64`) + // The following are blocking because the function literal call. + bt.assertBlockingInst(`pkg/test.bar`) + bt.assertBlockingInst(`pkg/test.bar`) +} + +func TestBlocking_BlockingLitInstance(t *testing.T) { + bt := newBlockingTest(t, + `package test + + type BazBlocker struct { + c chan bool + } + func (bb BazBlocker) Baz() { + println(<-bb.c) + } + + type BazNotBlocker struct {} + func (bnb BazNotBlocker) Baz() { + println("hi") + } + + type Foo interface { Baz() } + func FooBaz[T Foo](foo T) func() { + return func() { // line 17 + foo.Baz() + } + } + + func main() { + _ = FooBaz(BazBlocker{}) + _ = FooBaz(BazNotBlocker{}) + }`) + bt.assertFuncInstCount(5) + + bt.assertBlocking(`BazBlocker.Baz`) + // THe following is not blocking because the function literal is not called. + bt.assertNotBlockingInst(`pkg/test.FooBaz`) + bt.assertBlockingLit(17, `pkg/test.BazBlocker`) + + bt.assertNotBlocking(`BazNotBlocker.Baz`) + bt.assertNotBlockingInst(`pkg/test.FooBaz`) + bt.assertNotBlockingLit(17, `pkg/test.BazNotBlocker`) } func TestBlocking_MethodSelection(t *testing.T) { @@ -1333,13 +1498,13 @@ func TestBlocking_MethodSelection(t *testing.T) { bt.assertFuncInstCount(8) bt.assertBlocking(`BazBlocker.Baz`) - bt.assertBlockingInst(`test.ByMethodExpression[pkg/test.BazBlocker]`) - bt.assertBlockingInst(`test.ByInstance[pkg/test.BazBlocker]`) + bt.assertBlockingInst(`pkg/test.FooBaz.ByMethodExpression`) + bt.assertBlockingInst(`pkg/test.FooBaz.ByInstance`) bt.assertBlocking(`blocking`) bt.assertNotBlocking(`BazNotBlocker.Baz`) - bt.assertNotBlockingInst(`test.ByMethodExpression[pkg/test.BazNotBlocker]`) - bt.assertNotBlockingInst(`test.ByInstance[pkg/test.BazNotBlocker]`) + bt.assertNotBlockingInst(`pkg/test.FooBaz.ByMethodExpression`) + bt.assertNotBlockingInst(`pkg/test.FooBaz.ByInstance`) bt.assertNotBlocking(`notBlocking`) } @@ -1529,21 +1694,29 @@ func (bt *blockingTest) assertFuncInstCount(expCount int) { if got := bt.pkgInfo.funcInstInfos.Len(); got != expCount { bt.f.T.Errorf(`Got %d function instance infos but expected %d.`, got, expCount) for i, inst := range bt.pkgInfo.funcInstInfos.Keys() { - bt.f.T.Logf(` %d. %q`, i+1, inst.TypeString()) + bt.f.T.Logf(` %d. %q`, i+1, inst.String()) } } } func (bt *blockingTest) assertFuncLitCount(expCount int) { - if got := len(bt.pkgInfo.funcLitInfos); got != expCount { + got := 0 + for _, fis := range bt.pkgInfo.funcLitInfos { + got += len(fis) + } + if got != expCount { bt.f.T.Errorf(`Got %d function literal infos but expected %d.`, got, expCount) - pos := make([]string, 0, len(bt.pkgInfo.funcLitInfos)) - for fl := range bt.pkgInfo.funcLitInfos { - pos = append(pos, bt.f.FileSet.Position(fl.Pos()).String()) + + lits := make([]string, 0, len(bt.pkgInfo.funcLitInfos)) + for fl, fis := range bt.pkgInfo.funcLitInfos { + pos := bt.f.FileSet.Position(fl.Pos()).String() + for _, fi := range fis { + lits = append(lits, pos+`<`+fi.typeArgs.String()+`>`) + } } - sort.Strings(pos) - for i := range pos { - bt.f.T.Logf(` %d. %q`, i+1, pos) + sort.Strings(lits) + for i := range lits { + bt.f.T.Logf(` %d. %q`, i+1, lits[i]) } } } @@ -1597,24 +1770,41 @@ func (bt *blockingTest) isTypesFuncBlocking(funcName string) bool { return bt.pkgInfo.IsBlocking(inst) } -func (bt *blockingTest) assertBlockingLit(lineNo int) { - if !bt.isFuncLitBlocking(lineNo) { - bt.f.T.Errorf(`Got FuncLit at line %d as not blocking but expected it to be blocking.`, lineNo) +func (bt *blockingTest) assertBlockingLit(lineNo int, typeArgsStr string) { + if !bt.isFuncLitBlocking(lineNo, typeArgsStr) { + bt.f.T.Errorf(`Got FuncLit at line %d with type args %q as not blocking but expected it to be blocking.`, lineNo, typeArgsStr) } } -func (bt *blockingTest) assertNotBlockingLit(lineNo int) { - if bt.isFuncLitBlocking(lineNo) { - bt.f.T.Errorf(`Got FuncLit at line %d as blocking but expected it to be not blocking.`, lineNo) +func (bt *blockingTest) assertNotBlockingLit(lineNo int, typeArgsStr string) { + if bt.isFuncLitBlocking(lineNo, typeArgsStr) { + bt.f.T.Errorf(`Got FuncLit at line %d with type args %q as blocking but expected it to be not blocking.`, lineNo, typeArgsStr) } } -func (bt *blockingTest) isFuncLitBlocking(lineNo int) bool { +func (bt *blockingTest) isFuncLitBlocking(lineNo int, typeArgsStr string) bool { fnLit := srctesting.GetNodeAtLineNo[*ast.FuncLit](bt.file, bt.f.FileSet, lineNo) if fnLit == nil { bt.f.T.Fatalf(`FuncLit on line %d not found in the AST.`, lineNo) } - return bt.pkgInfo.FuncLitInfo(fnLit).IsBlocking() + + fis, ok := bt.pkgInfo.funcLitInfos[fnLit] + if !ok { + bt.f.T.Fatalf(`No FuncInfo found for FuncLit at line %d.`, lineNo) + } + + for _, fi := range fis { + if fi.typeArgs.String() == typeArgsStr { + return fi.IsBlocking() + } + } + + bt.f.T.Logf("FuncList instances:") + for i, fi := range fis { + bt.f.T.Logf("\t%d. %q\n", i+1, fi.typeArgs.String()) + } + bt.f.T.Fatalf(`No FuncInfo found for FuncLit at line %d with type args %q.`, lineNo, typeArgsStr) + return false } func (bt *blockingTest) assertBlockingInst(instanceStr string) { @@ -1632,40 +1822,58 @@ func (bt *blockingTest) assertNotBlockingInst(instanceStr string) { func (bt *blockingTest) isFuncInstBlocking(instanceStr string) bool { instances := bt.pkgInfo.funcInstInfos.Keys() for _, inst := range instances { - if inst.TypeString() == instanceStr { + if inst.String() == instanceStr { return bt.pkgInfo.FuncInfo(inst).IsBlocking() } } bt.f.T.Logf(`Function instances found in package info:`) for i, inst := range instances { - bt.f.T.Logf(` %d. %s`, i+1, inst.TypeString()) + bt.f.T.Logf(` %d. %s`, i+1, inst.String()) } bt.f.T.Fatalf(`No function instance found for %q in package info.`, instanceStr) return false } -func (bt *blockingTest) assertBlockingReturn(lineNo int) { - if !bt.isReturnBlocking(lineNo) { - bt.f.T.Errorf(`Got return at line %d as not blocking but expected it to be blocking.`, lineNo) +func (bt *blockingTest) assertBlockingReturn(lineNo int, typeArgsStr string) { + if !bt.isReturnBlocking(lineNo, typeArgsStr) { + bt.f.T.Errorf(`Got return at line %d (%q) as not blocking but expected it to be blocking.`, lineNo, typeArgsStr) } } -func (bt *blockingTest) assertNotBlockingReturn(lineNo int) { - if bt.isReturnBlocking(lineNo) { - bt.f.T.Errorf(`Got return at line %d as blocking but expected it to be not blocking.`, lineNo) +func (bt *blockingTest) assertNotBlockingReturn(lineNo int, typeArgsStr string) { + if bt.isReturnBlocking(lineNo, typeArgsStr) { + bt.f.T.Errorf(`Got return at line %d (%q) as blocking but expected it to be not blocking.`, lineNo, typeArgsStr) } } -func (bt *blockingTest) isReturnBlocking(lineNo int) bool { +func (bt *blockingTest) isReturnBlocking(lineNo int, typeArgsStr string) bool { ret := srctesting.GetNodeAtLineNo[*ast.ReturnStmt](bt.file, bt.f.FileSet, lineNo) if ret == nil { bt.f.T.Fatalf(`ReturnStmt on line %d not found in the AST.`, lineNo) } + + foundInfo := []*FuncInfo{} for _, info := range bt.pkgInfo.allInfos { - if blocking, found := info.Blocking[ret]; found { - return blocking + for _, rs := range info.returnStmts { + if rs.analyzeStack[len(rs.analyzeStack)-1] == ret { + if info.typeArgs.String() == typeArgsStr { + // Found info that matches the type args and + // has the return statement so return the blocking value. + return info.Blocking[ret] + } + + // Wrong instance, record for error message in the case + // that the correct one instance is not found. + foundInfo = append(foundInfo, info) + break + } } } - // If not found in any info.Blocking, then it is not blocking. + + bt.f.T.Logf("FuncInfo instances with ReturnStmt at line %d:", lineNo) + for i, info := range foundInfo { + bt.f.T.Logf("\t%d. %q\n", i+1, info.typeArgs.String()) + } + bt.f.T.Fatalf(`No FuncInfo found for ReturnStmt at line %d with type args %q.`, lineNo, typeArgsStr) return false } diff --git a/compiler/internal/typeparams/map.go b/compiler/internal/typeparams/map.go index 4f6645421..dbe07a54e 100644 --- a/compiler/internal/typeparams/map.go +++ b/compiler/internal/typeparams/map.go @@ -40,7 +40,7 @@ func (im *InstanceMap[V]) findIndex(key Instance) (mapBucket[V], int) { if im != nil && im.data != nil { bucket := im.data[key.Object][typeHash(im.hasher, key.TArgs...)] for i, candidate := range bucket { - if candidate != nil && typeArgsEq(candidate.key.TArgs, key.TArgs) { + if candidate != nil && candidate.key.TArgs.Equal(key.TArgs) { return bucket, i } } @@ -90,7 +90,7 @@ func (im *InstanceMap[V]) Set(key Instance, value V) V { for i, candidate := range bucket { if candidate == nil { hole = i - } else if typeArgsEq(candidate.key.TArgs, key.TArgs) { + } else if candidate.key.TArgs.Equal(key.TArgs) { old := candidate.value candidate.value = value return old @@ -192,17 +192,3 @@ func typeHash(hasher typeutil.Hasher, types ...types.Type) uint32 { } return hash } - -// typeArgsEq returns if both lists of type arguments are identical. -func typeArgsEq(a, b []types.Type) bool { - if len(a) != len(b) { - return false - } - for i := range a { - if !types.Identical(a[i], b[i]) { - return false - } - } - - return true -} diff --git a/compiler/internal/typeparams/map_test.go b/compiler/internal/typeparams/map_test.go index ed65587c7..baa31f64a 100644 --- a/compiler/internal/typeparams/map_test.go +++ b/compiler/internal/typeparams/map_test.go @@ -317,7 +317,7 @@ func keysMatch(a, b []Instance) bool { func keyAt(keys []Instance, target Instance) int { for i, v := range keys { - if v.Object == target.Object && typeArgsEq(v.TArgs, target.TArgs) { + if v.Object == target.Object && v.TArgs.Equal(target.TArgs) { return i } } diff --git a/compiler/typesutil/typelist.go b/compiler/typesutil/typelist.go index 04d0d6869..768677365 100644 --- a/compiler/typesutil/typelist.go +++ b/compiler/typesutil/typelist.go @@ -18,3 +18,16 @@ func (tl TypeList) String() string { } return buf.String() } + +// Equal returns true if both lists of type arguments are identical. +func (tl TypeList) Equal(other TypeList) bool { + if len(tl) != len(other) { + return false + } + for i := range tl { + if !types.Identical(tl[i], other[i]) { + return false + } + } + return true +}