diff --git a/cl/builtin_test.go b/cl/builtin_test.go index 65b1cc8890..c8df3966db 100644 --- a/cl/builtin_test.go +++ b/cl/builtin_test.go @@ -459,24 +459,30 @@ func setRefs(v unsafe.Pointer, refs ...ssa.Instruction) { } func TestRecvTypeName(t *testing.T) { - if ret := recvTypeName(&ast.IndexExpr{ + if ret, pointer, ok := recvTypeName(&ast.IndexExpr{ X: &ast.Ident{Name: "Pointer"}, Index: &ast.Ident{Name: "T"}, - }); ret != "Pointer" { - t.Fatal("recvTypeName IndexExpr:", ret) + }); !ok || pointer || ret != "Pointer" { + t.Fatal("recvTypeName IndexExpr:", ret, pointer, ok) } - if ret := recvTypeName(&ast.IndexListExpr{ - X: &ast.Ident{Name: "Pointer"}, + if ret, pointer, ok := recvTypeName(&ast.StarExpr{X: &ast.IndexListExpr{ + X: &ast.ParenExpr{X: &ast.Ident{Name: "Pointer"}}, Indices: []ast.Expr{&ast.Ident{Name: "T"}}, - }); ret != "Pointer" { - t.Fatal("recvTypeName IndexListExpr:", ret) + }}); !ok || !pointer || ret != "Pointer" { + t.Fatal("recvTypeName pointer IndexListExpr:", ret, pointer, ok) + } + if _, _, ok := recvTypeName(&ast.SelectorExpr{ + X: &ast.Ident{Name: "bufio"}, + Sel: &ast.Ident{Name: "Reader"}, + }); ok { + t.Fatal("recvTypeName accepted a non-local selector") + } + if _, _, ok := recvTypeName(&ast.StarExpr{X: &ast.StarExpr{X: &ast.Ident{Name: "T"}}}); ok { + t.Fatal("recvTypeName accepted a pointer-to-pointer receiver") + } + if _, _, ok := recvTypeName(&ast.BadExpr{}); ok { + t.Fatal("recvTypeName accepted a bad expression") } - defer func() { - if r := recover(); r == nil { - t.Fatal("recvTypeName: no error?") - } - }() - recvTypeName(&ast.BadExpr{}) } func TestRecvType(t *testing.T) { diff --git a/cl/compile.go b/cl/compile.go index 8115f58b06..ebf6393cff 100644 --- a/cl/compile.go +++ b/cl/compile.go @@ -585,8 +585,9 @@ func (p *context) compileFuncDecl(pkg llssa.Package, f *ssa.Function) (llssa.Fun // ParsePkgSyntax is the sole //llgo:env extractor. Lowering only consumes // its source-declaration cache; imported env entries use NewEnvFunc. if decl, ok := f.Syntax().(*ast.FuncDecl); ok { - fullName, _ := astFuncName(llssa.PathOf(pkgTypes), decl) - hasExplicitEnv = p.prog.HasClosureEnvDirective(p.goProg.Fset, fullName, decl.Pos()) + if fullName, _, ok := astFuncName(llssa.PathOf(pkgTypes), decl); ok { + hasExplicitEnv = p.prog.HasClosureEnvDirective(p.goProg.Fset, fullName, decl.Pos()) + } } hasCtx := hasFreeVars && !elideFreeVarEnv || hasExplicitEnv var ctx *types.Var diff --git a/cl/import.go b/cl/import.go index e7c2d38b40..53cc3a34a1 100644 --- a/cl/import.go +++ b/cl/import.go @@ -191,7 +191,10 @@ func (p *context) initFiles(pkgPath string, files []*ast.File, cPkg bool) { for _, decl := range file.Decls { switch decl := decl.(type) { case *ast.FuncDecl: - fullName, inPkgName := astFuncName(pkgPath, decl) + fullName, inPkgName, ok := astFuncName(pkgPath, decl) + if !ok { + continue + } if preloaded { if exportName, ok := p.prog.PackageExport(fullName); ok { p.pkg.SetExport(fullName, exportName) @@ -453,26 +456,27 @@ func (p *context) initLink(line string, prefix int, export bool, f func(inPkgNam } } -func recvTypeName(typ ast.Expr) string { -retry: +// recvTypeName extracts the named base and pointer form from receiver syntax +// that can denote a locally declared type. Other syntax is left to the type +// checker to report. The pointer result is meaningful only when ok is true. +func recvTypeName(typ ast.Expr) (name string, pointer, ok bool) { switch t := typ.(type) { case *ast.Ident: - return t.Name + return t.Name, false, true case *ast.IndexExpr: - return trecvTypeName(t.X, t.Index) + return recvTypeName(t.X) case *ast.IndexListExpr: - return trecvTypeName(t.X, t.Indices...) + return recvTypeName(t.X) case *ast.ParenExpr: - typ = t.X - goto retry + return recvTypeName(t.X) + case *ast.StarExpr: + name, nestedPointer, ok := recvTypeName(t.X) + if !ok || nestedPointer { + return "", false, false + } + return name, true, true } - panic("unreachable") -} - -// TODO(xsw): support generic type -func trecvTypeName(t ast.Expr, indices ...ast.Expr) string { - _ = indices - return t.(*ast.Ident).Name + return "", false, false } // inPkgName: @@ -481,19 +485,25 @@ func trecvTypeName(t ast.Expr, indices ...ast.Expr) string { // fullName: // - func: pkg.name // - method: pkg.(T).name, pkg.(*T).name -func astFuncName(pkgPath string, fn *ast.FuncDecl) (fullName, inPkgName string) { +// Invalid receiver syntax returns ok=false so declaration metadata collection +// does not mask the type-checking diagnostic with an internal panic. +func astFuncName(pkgPath string, fn *ast.FuncDecl) (fullName, inPkgName string, ok bool) { name := fn.Name.Name - if recv := fn.Recv; recv != nil && len(recv.List) == 1 { - var method string - t := recv.List[0].Type - if tp, ok := t.(*ast.StarExpr); ok { - method = "(*" + recvTypeName(tp.X) + ")." + name - } else { - method = recvTypeName(t) + "." + name + if recv := fn.Recv; recv != nil { + if len(recv.List) != 1 { + return "", "", false } - return pkgPath + "." + method, method + recvName, pointer, recvOK := recvTypeName(recv.List[0].Type) + if !recvOK { + return "", "", false + } + method := recvName + "." + name + if pointer { + method = "(*" + recvName + ")." + name + } + return pkgPath + "." + method, method, true } - return pkgPath + "." + name, name + return pkgPath + "." + name, name, true } func typesFuncName(pkgPath string, fn *types.Func) (fullName, inPkgName string) { @@ -857,7 +867,10 @@ func ParsePkgSyntaxWithOptions(prog llssa.Program, fset *token.FileSet, pkg *typ if err := locality.ValidateFuncBody(fset, decl.Body); err != nil { return err } - fullName, inPkgName := astFuncName(pkgPath, decl) + fullName, inPkgName, ok := astFuncName(pkgPath, decl) + if !ok { + continue + } syms[inPkgName] = fullName hasLinkname, err := collectDeclarationDirectivesWithOptions(prog, fset, decl.Doc, fullName, inPkgName, decl.Pos(), options) if err != nil { diff --git a/cl/import_coverage_test.go b/cl/import_coverage_test.go index c2d7a2fd78..d2dad7e8ea 100644 --- a/cl/import_coverage_test.go +++ b/cl/import_coverage_test.go @@ -178,8 +178,8 @@ func TestPkgSymInfoAddSymAndInitLinknamesCoverage(t *testing.T) { } func TestAstAndTypesFuncNameCoverage(t *testing.T) { - full, inPkg := astFuncName("example.com/p", &ast.FuncDecl{Name: &ast.Ident{Name: "F"}}) - if full != "example.com/p.F" || inPkg != "F" { + full, inPkg, ok := astFuncName("example.com/p", &ast.FuncDecl{Name: &ast.Ident{Name: "F"}}) + if !ok || full != "example.com/p.F" || inPkg != "F" { t.Fatalf("astFuncName(func)=(%q,%q), want (%q,%q)", full, inPkg, "example.com/p.F", "F") } @@ -189,8 +189,8 @@ func TestAstAndTypesFuncNameCoverage(t *testing.T) { {Type: &ast.StarExpr{X: &ast.ParenExpr{X: &ast.Ident{Name: "T"}}}}, }}, } - full, inPkg = astFuncName("example.com/p", ptrRecv) - if full != "example.com/p.(*T).M" || inPkg != "(*T).M" { + full, inPkg, ok = astFuncName("example.com/p", ptrRecv) + if !ok || full != "example.com/p.(*T).M" || inPkg != "(*T).M" { t.Fatalf("astFuncName(method ptr)=(%q,%q), want (%q,%q)", full, inPkg, "example.com/p.(*T).M", "(*T).M") } @@ -221,6 +221,29 @@ func TestAstAndTypesFuncNameCoverage(t *testing.T) { } } +func TestParsePkgSyntaxSkipsNonLocalMethodReceiver(t *testing.T) { + const src = `package p + +import "bufio" + +func (b *bufio.Reader) Buffered() int { return -1 } +` + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "issue5089.go", src, parser.ParseComments) + if err != nil { + t.Fatal(err) + } + prog := llssa.NewProgram(nil) + defer prog.Dispose() + pkg := types.NewPackage("example.com/p", "p") + if err := ParsePkgSyntax(prog, fset, pkg, []*ast.File{file}); err != nil { + t.Fatal(err) + } + if !prog.PackageSyntaxParsed(pkg) { + t.Fatal("package syntax was not marked parsed") + } +} + func TestParsePkgSyntaxCollectsLinknames(t *testing.T) { cases := []struct { name string @@ -308,7 +331,10 @@ func plain() {} want := map[string]bool{"env": true, "spaced": true, "plain": false} for _, node := range file.Decls { decl := node.(*ast.FuncDecl) - fullName, _ := astFuncName(pkg.Path(), decl) + fullName, _, ok := astFuncName(pkg.Path(), decl) + if !ok { + t.Fatalf("astFuncName(%s) rejected valid declaration", decl.Name.Name) + } got := prog.HasClosureEnvDirective(fset, fullName, decl.Pos()) if got != want[decl.Name.Name] { t.Fatalf("HasClosureEnvDirective(%s) = %v, want %v", decl.Name.Name, got, want[decl.Name.Name])