Skip to content

Fixing issues in analysis around generics #1351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 9, 2024
Merged

Fixing issues in analysis around generics #1351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/functions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
19 changes: 11 additions & 8 deletions compiler/internal/analysis/defer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -65,25 +68,25 @@ 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.
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
}
Expand Down
94 changes: 58 additions & 36 deletions compiler/internal/analysis/info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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].
//
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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))
}
Expand Down
Loading
Loading