Commit ff8f3f0f authored by Josh Bleecher Snyder's avatar Josh Bleecher Snyder

cmd/vet: extend copylocks to anonymous functions

Running -copylocks over a large corpus generates 1507 warnings.
Of those, only 3 are from the new anonymous function check,
but they are all bugs.

Fixes #10927.

Change-Id: I2672f6871036bed711beec5f88bc39aa8b3b6a94
Reviewed-on: https://go-review.googlesource.com/11051Reviewed-by: default avatarRob Pike <r@golang.org>
parent cb2d02c8
...@@ -18,7 +18,7 @@ func init() { ...@@ -18,7 +18,7 @@ func init() {
register("copylocks", register("copylocks",
"check that locks are not passed by value", "check that locks are not passed by value",
checkCopyLocks, checkCopyLocks,
funcDecl, rangeStmt) funcDecl, rangeStmt, funcLit)
} }
// checkCopyLocks checks whether node might // checkCopyLocks checks whether node might
...@@ -28,7 +28,9 @@ func checkCopyLocks(f *File, node ast.Node) { ...@@ -28,7 +28,9 @@ func checkCopyLocks(f *File, node ast.Node) {
case *ast.RangeStmt: case *ast.RangeStmt:
checkCopyLocksRange(f, node) checkCopyLocksRange(f, node)
case *ast.FuncDecl: case *ast.FuncDecl:
checkCopyLocksFunc(f, node) checkCopyLocksFunc(f, node.Name.Name, node.Recv, node.Type)
case *ast.FuncLit:
checkCopyLocksFunc(f, "func", nil, node.Type)
} }
} }
...@@ -36,28 +38,28 @@ func checkCopyLocks(f *File, node ast.Node) { ...@@ -36,28 +38,28 @@ func checkCopyLocks(f *File, node ast.Node) {
// inadvertently copy a lock, by checking whether // inadvertently copy a lock, by checking whether
// its receiver, parameters, or return values // its receiver, parameters, or return values
// are locks. // are locks.
func checkCopyLocksFunc(f *File, d *ast.FuncDecl) { func checkCopyLocksFunc(f *File, name string, recv *ast.FieldList, typ *ast.FuncType) {
if d.Recv != nil && len(d.Recv.List) > 0 { if recv != nil && len(recv.List) > 0 {
expr := d.Recv.List[0].Type expr := recv.List[0].Type
if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); path != nil { if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); path != nil {
f.Badf(expr.Pos(), "%s passes Lock by value: %v", d.Name.Name, path) f.Badf(expr.Pos(), "%s passes Lock by value: %v", name, path)
} }
} }
if d.Type.Params != nil { if typ.Params != nil {
for _, field := range d.Type.Params.List { for _, field := range typ.Params.List {
expr := field.Type expr := field.Type
if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); path != nil { if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); path != nil {
f.Badf(expr.Pos(), "%s passes Lock by value: %v", d.Name.Name, path) f.Badf(expr.Pos(), "%s passes Lock by value: %v", name, path)
} }
} }
} }
if d.Type.Results != nil { if typ.Results != nil {
for _, field := range d.Type.Results.List { for _, field := range typ.Results.List {
expr := field.Type expr := field.Type
if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); path != nil { if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); path != nil {
f.Badf(expr.Pos(), "%s returns Lock by value: %v", d.Name.Name, path) f.Badf(expr.Pos(), "%s returns Lock by value: %v", name, path)
} }
} }
} }
......
...@@ -14,6 +14,11 @@ func BadFunc(sync.Mutex) {} // ERROR "BadFunc passes Lock by value: sync.Mutex" ...@@ -14,6 +14,11 @@ func BadFunc(sync.Mutex) {} // ERROR "BadFunc passes Lock by value: sync.Mutex"
func OkRet() *sync.Mutex {} func OkRet() *sync.Mutex {}
func BadRet() sync.Mutex {} // ERROR "BadRet returns Lock by value: sync.Mutex" func BadRet() sync.Mutex {} // ERROR "BadRet returns Lock by value: sync.Mutex"
var (
OkClosure = func(*sync.Mutex) {}
BadClosure = func(sync.Mutex) {} // ERROR "func passes Lock by value: sync.Mutex"
)
type EmbeddedRWMutex struct { type EmbeddedRWMutex struct {
sync.RWMutex sync.RWMutex
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment