cmd/vet: lostcancel: treat naked return as a use of named results

+ test.

Fixes #16230

Change-Id: Idac995437146a9df9e73f094d2a31abc25b1fa62
Reviewed-on: https://go-review.googlesource.com/24681
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/cmd/vet/lostcancel.go b/src/cmd/vet/lostcancel.go
index 11c3c477836340d2424c5add5c8347037f15b626..d049a3e8880fd27f77c18fd0161ce361de4362db 100644 (file)
--- a/src/cmd/vet/lostcancel.go
+++ b/src/cmd/vet/lostcancel.go
@@ -101,10 +101,13 @@ func checkLostCancel(f *File, node ast.Node) {
 
        // Build the CFG.
        var g *cfg.CFG
+       var sig *types.Signature
        switch node := node.(type) {
        case *ast.FuncDecl:
+               sig, _ = f.pkg.defs[node.Name].Type().(*types.Signature)
                g = cfg.New(node.Body, mayReturn)
        case *ast.FuncLit:
+               sig, _ = f.pkg.types[node.Type].Type.(*types.Signature)
                g = cfg.New(node.Body, mayReturn)
        }
 
@@ -117,7 +120,7 @@ func checkLostCancel(f *File, node ast.Node) {
        // (It would be more efficient to analyze all cancelvars in a
        // single pass over the AST, but seldom is there more than one.)
        for v, stmt := range cancelvars {
-               if ret := lostCancelPath(f, g, v, stmt); ret != nil {
+               if ret := lostCancelPath(f, g, v, stmt, sig); ret != nil {
                        lineno := f.fset.Position(stmt.Pos()).Line
                        f.Badf(stmt.Pos(), "the %s function is not used on all paths (possible context leak)", v.Name())
                        f.Badf(ret.Pos(), "this return statement may be reached without using the %s var defined on line %d", v.Name(), lineno)
@@ -159,14 +162,24 @@ func isContextWithCancel(f *File, n ast.Node) bool {
 // lostCancelPath finds a path through the CFG, from stmt (which defines
 // the 'cancel' variable v) to a return statement, that doesn't "use" v.
 // If it finds one, it returns the return statement (which may be synthetic).
-func lostCancelPath(f *File, g *cfg.CFG, v *types.Var, stmt ast.Node) *ast.ReturnStmt {
+// sig is the function's type, if known.
+func lostCancelPath(f *File, g *cfg.CFG, v *types.Var, stmt ast.Node, sig *types.Signature) *ast.ReturnStmt {
+       vIsNamedResult := sig != nil && tupleContains(sig.Results(), v)
+
        // uses reports whether stmts contain a "use" of variable v.
        uses := func(f *File, v *types.Var, stmts []ast.Node) bool {
                found := false
                for _, stmt := range stmts {
                        ast.Inspect(stmt, func(n ast.Node) bool {
-                               if id, ok := n.(*ast.Ident); ok {
-                                       if f.pkg.uses[id] == v {
+                               switch n := n.(type) {
+                               case *ast.Ident:
+                                       if f.pkg.uses[n] == v {
+                                               found = true
+                                       }
+                               case *ast.ReturnStmt:
+                                       // A naked return statement counts as a use
+                                       // of the named result variables.
+                                       if n.Results == nil && vIsNamedResult {
                                                found = true
                                        }
                                }
@@ -251,6 +264,15 @@ outer:
        return search(defblock.Succs)
 }
 
+func tupleContains(tuple *types.Tuple, v *types.Var) bool {
+       for i := 0; i < tuple.Len(); i++ {
+               if tuple.At(i) == v {
+                       return true
+               }
+       }
+       return false
+}
+
 var noReturnFuncs = map[string]bool{
        "(*testing.common).FailNow": true,
        "(*testing.common).Fatal":   true,

diff --git a/src/cmd/vet/testdata/lostcancel.go b/src/cmd/vet/testdata/lostcancel.go
index 213dd1832d45851a1de9fcbbced4473d68990a2d..b7549c00511127819183818ba3895b5c03536ba8 100644 (file)
--- a/src/cmd/vet/testdata/lostcancel.go
+++ b/src/cmd/vet/testdata/lostcancel.go
@@ -141,3 +141,15 @@ func _() {
        var x struct{ f func() }
        x.f()
 }
+
+// Regression test for Go issue 16230.
+func _() (ctx context.Context, cancel func()) {
+       ctx, cancel = context.WithCancel()
+       return // a naked return counts as a load of the named result values
+}
+
+// Same as above, but for literal function.
+var _ = func() (ctx context.Context, cancel func()) {
+       ctx, cancel = context.WithCancel()
+       return
+}