diff --git a/src/cmd/vet/copylock.go b/src/cmd/vet/copylock.go index 8d8399a851b..78632944ca8 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, funcLit, assignStmt) + funcDecl, rangeStmt, funcLit, assignStmt, genDecl, compositeLit) } // checkCopyLocks checks whether node might @@ -33,15 +33,47 @@ func checkCopyLocks(f *File, node ast.Node) { checkCopyLocksFunc(f, "func", nil, node.Type) case *ast.AssignStmt: checkCopyLocksAssign(f, node) + case *ast.GenDecl: + checkCopyLocksGenDecl(f, node) + case *ast.CompositeLit: + checkCopyCompositeLit(f, node) } } // checkCopyLocksAssign checks whether an assignment // copies a lock. func checkCopyLocksAssign(f *File, as *ast.AssignStmt) { - for _, x := range as.Lhs { - if path := lockPath(f.pkg.typesPkg, f.pkg.types[x].Type); path != nil { - f.Badf(x.Pos(), "assignment copies lock value to %v: %v", f.gofmt(x), path) + for i, x := range as.Rhs { + if path := lockPathRhs(f, x); path != nil { + f.Badf(x.Pos(), "assignment copies lock value to %v: %v", f.gofmt(as.Lhs[i]), path) + } + } +} + +// checkCopyLocksGenDecl checks whether lock is copied +// in variable declaration. +func checkCopyLocksGenDecl(f *File, gd *ast.GenDecl) { + if gd.Tok != token.VAR { + return + } + for _, spec := range gd.Specs { + valueSpec := spec.(*ast.ValueSpec) + for i, x := range valueSpec.Values { + if path := lockPathRhs(f, x); path != nil { + f.Badf(x.Pos(), "variable declaration copies lock value to %v: %v", valueSpec.Names[i].Name, path) + } + } + } +} + +// checkCopyCompositeLit detects lock copy inside a composite literal +func checkCopyCompositeLit(f *File, cl *ast.CompositeLit) { + for _, x := range cl.Elts { + if node, ok := x.(*ast.KeyValueExpr); ok { + x = node.Value + } + if path := lockPathRhs(f, x); path != nil { + f.Badf(x.Pos(), "literal copies lock value from %v: %v", f.gofmt(x), path) } } } @@ -132,6 +164,13 @@ func (path typePath) String() string { return buf.String() } +func lockPathRhs(f *File, x ast.Expr) typePath { + if _, ok := x.(*ast.CompositeLit); ok { + return nil + } + return lockPath(f.pkg.typesPkg, f.pkg.types[x].Type) +} + // lockPath returns a typePath describing the location of a lock value // contained in typ. If there is no contained lock, it returns nil. func lockPath(tpkg *types.Package, typ types.Type) typePath { diff --git a/src/cmd/vet/testdata/copylock.go b/src/cmd/vet/testdata/copylock.go index 03d0c33f364..2b8cec14202 100644 --- a/src/cmd/vet/testdata/copylock.go +++ b/src/cmd/vet/testdata/copylock.go @@ -7,6 +7,21 @@ func OkFunc() { p := x var y sync.Mutex p = &y + + var z = sync.Mutex{} + w := sync.Mutex{} + + w = sync.Mutex{} + q := struct{ L sync.Mutex }{ + L: sync.Mutex{}, + } + + yy := []Tlock{ + sync.Tlock{}, + sync.Tlock{ + once: sync.Once{}, + }, + } } type Tlock struct { @@ -25,4 +40,19 @@ func BadFunc() { tp = &t *tp = t // ERROR "assignment copies lock value to \*tp: testdata.Tlock contains sync.Once contains sync.Mutex" t = *tp // ERROR "assignment copies lock value to t: testdata.Tlock contains sync.Once contains sync.Mutex" + + y := *x // ERROR "assignment copies lock value to y: sync.Mutex" + var z = t // ERROR "variable declaration copies lock value to z: testdata.Tlock contains sync.Once contains sync.Mutex" + + w := struct{ L sync.Mutex }{ + L: *x, // ERROR "literal copies lock value from \*x: sync.Mutex" + } + var q = map[int]Tlock{ + 1: t, // ERROR "literal copies lock value from t: testdata.Tlock contains sync.Once contains sync.Mutex" + 2: *tp, // ERROR "literal copies lock value from \*tp: testdata.Tlock contains sync.Once contains sync.Mutex" + } + yy := []Tlock{ + t, // ERROR "literal copies lock value from t: testdata.Tlock contains sync.Once contains sync.Mutex" + *tp, // ERROR "literal copies lock value from \*tp: testdata.Tlock contains sync.Once contains sync.Mutex" + } }