Skip to content

Commit 003a68b

Browse files
cmd/vet: remove copylock warning about result types and calls
Don't issue a copylock warning about a result type; the function may return a composite literal with a zero value, which is OK. Don't issue a copylock warning about a function call on the RHS, or an indirection of a function call; the function may return a composite literal with a zero value, which is OK. Updates #16227. Change-Id: I94f0e066bbfbca5d4f8ba96106210083e36694a2 Reviewed-on: https://go-review.googlesource.com/24711 Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com> Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Rob Pike <r@golang.org>
1 parent 878e002 commit 003a68b

File tree

2 files changed

+24
-10
lines changed

2 files changed

+24
-10
lines changed

src/cmd/vet/copylock.go

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,10 @@ func checkCopyLocksFunc(f *File, name string, recv *ast.FieldList, typ *ast.Func
125125
}
126126
}
127127

128-
if typ.Results != nil {
129-
for _, field := range typ.Results.List {
130-
expr := field.Type
131-
if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); path != nil {
132-
f.Badf(expr.Pos(), "%s returns lock by value: %v", name, path)
133-
}
134-
}
135-
}
128+
// Don't check typ.Results. If T has a Lock field it's OK to write
129+
// return T{}
130+
// because that is returning the zero value. Leave result checking
131+
// to the return statement.
136132
}
137133

138134
// checkCopyLocksRange checks whether a range statement
@@ -194,6 +190,16 @@ func lockPathRhs(f *File, x ast.Expr) typePath {
194190
if _, ok := x.(*ast.CompositeLit); ok {
195191
return nil
196192
}
193+
if _, ok := x.(*ast.CallExpr); ok {
194+
// A call may return a zero value.
195+
return nil
196+
}
197+
if star, ok := x.(*ast.StarExpr); ok {
198+
if _, ok := star.X.(*ast.CallExpr); ok {
199+
// A call may return a pointer to a zero value.
200+
return nil
201+
}
202+
}
197203
return lockPath(f.pkg.typesPkg, f.pkg.types[x].Type)
198204
}
199205

src/cmd/vet/testdata/copylock_func.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import "sync"
1212
func OkFunc(*sync.Mutex) {}
1313
func BadFunc(sync.Mutex) {} // ERROR "BadFunc passes lock by value: sync.Mutex"
1414
func OkRet() *sync.Mutex {}
15-
func BadRet() sync.Mutex {} // ERROR "BadRet returns lock by value: sync.Mutex"
15+
func BadRet() sync.Mutex {} // Don't warn about results
1616

1717
var (
1818
OkClosure = func(*sync.Mutex) {}
@@ -28,7 +28,7 @@ func (EmbeddedRWMutex) BadMeth() {} // ERROR "BadMeth passes lock by value: test
2828
func OkFunc(e *EmbeddedRWMutex) {}
2929
func BadFunc(EmbeddedRWMutex) {} // ERROR "BadFunc passes lock by value: testdata.EmbeddedRWMutex"
3030
func OkRet() *EmbeddedRWMutex {}
31-
func BadRet() EmbeddedRWMutex {} // ERROR "BadRet returns lock by value: testdata.EmbeddedRWMutex"
31+
func BadRet() EmbeddedRWMutex {} // Don't warn about results
3232

3333
type FieldMutex struct {
3434
s sync.Mutex
@@ -107,6 +107,14 @@ func ReturnViaInterface(x int) (int, interface{}) {
107107
}
108108
}
109109

110+
// Some cases that we don't warn about.
111+
112+
func AcceptedCases() {
113+
x := EmbeddedRwMutex{} // composite literal on RHS is OK (#16227)
114+
x = BadRet() // function call on RHS is OK (#16227)
115+
x = *OKRet() // indirection of function call on RHS is OK (#16227)
116+
}
117+
110118
// TODO: Unfortunate cases
111119

112120
// Non-ideal error message:

0 commit comments

Comments
 (0)