From ff8f3f0fa126c71059f53f81be3b87237fb55546 Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Fri, 12 Jun 2015 19:49:45 -0700 Subject: [PATCH] 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/11051 Reviewed-by: Rob Pike --- src/cmd/vet/copylock.go | 26 ++++++++++++++------------ src/cmd/vet/testdata/copylock_func.go | 5 +++++ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/cmd/vet/copylock.go b/src/cmd/vet/copylock.go index 95cecc799c..6c71061b3e 100644 --- a/src/cmd/vet/copylock.go +++ b/src/cmd/vet/copylock.go @@ -18,7 +18,7 @@ func init() { register("copylocks", "check that locks are not passed by value", checkCopyLocks, - funcDecl, rangeStmt) + funcDecl, rangeStmt, funcLit) } // checkCopyLocks checks whether node might @@ -28,7 +28,9 @@ func checkCopyLocks(f *File, node ast.Node) { case *ast.RangeStmt: checkCopyLocksRange(f, node) 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) { // inadvertently copy a lock, by checking whether // its receiver, parameters, or return values // are locks. -func checkCopyLocksFunc(f *File, d *ast.FuncDecl) { - if d.Recv != nil && len(d.Recv.List) > 0 { - expr := d.Recv.List[0].Type +func checkCopyLocksFunc(f *File, name string, recv *ast.FieldList, typ *ast.FuncType) { + if recv != nil && len(recv.List) > 0 { + expr := recv.List[0].Type 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 { - for _, field := range d.Type.Params.List { + if typ.Params != nil { + for _, field := range typ.Params.List { expr := field.Type 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 { - for _, field := range d.Type.Results.List { + if typ.Results != nil { + for _, field := range typ.Results.List { expr := field.Type 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) } } } diff --git a/src/cmd/vet/testdata/copylock_func.go b/src/cmd/vet/testdata/copylock_func.go index 108c044209..d83957fe18 100644 --- a/src/cmd/vet/testdata/copylock_func.go +++ b/src/cmd/vet/testdata/copylock_func.go @@ -14,6 +14,11 @@ func BadFunc(sync.Mutex) {} // ERROR "BadFunc passes Lock by value: sync.Mutex" func OkRet() *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 { sync.RWMutex }