Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(17)

Delta Between Two Patch Sets: cmd/vet/copylock.go

Issue 145360043: code review 145360043: go.tools/cmd/vet: warn about copying locks in range sta... (Closed)
Left Patch Set: Created 9 years, 6 months ago
Right Patch Set: diff -r 9c7110ab58402df527dbf8373d0fa888000779d4 https://code.google.com/p/go.tools Created 9 years, 6 months ago
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments. Please Sign in to add in-line comments.
Jump to:
Right: Side by side diff | Download
« no previous file with change/comment | « no previous file | cmd/vet/testdata/copylock_func.go » ('j') | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
(no file at all)
1 // Copyright 2013 The Go Authors. All rights reserved. 1 // Copyright 2013 The Go Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style 2 // Use of this source code is governed by a BSD-style
3 // license that can be found in the LICENSE file. 3 // license that can be found in the LICENSE file.
4 4
5 // This file contains the code to check that locks are not passed by value. 5 // This file contains the code to check that locks are not passed by value.
6 6
7 package main 7 package main
8 8
9 import ( 9 import (
10 "bytes" 10 "bytes"
11 "fmt" 11 "fmt"
12 "go/ast" 12 "go/ast"
13 "go/token"
13 14
14 "code.google.com/p/go.tools/go/types" 15 "code.google.com/p/go.tools/go/types"
15 ) 16 )
16 17
17 func init() { 18 func init() {
18 register("copylocks", 19 register("copylocks",
19 "check that locks are not passed by value", 20 "check that locks are not passed by value",
20 checkCopyLocks, 21 checkCopyLocks,
21 » » funcDecl) 22 » » funcDecl, rangeStmt)
22 } 23 }
23 24
24 // checkCopyLocks checks whether a function might 25 // checkCopyLocks checks whether node might
26 // inadvertently copy a lock.
27 func checkCopyLocks(f *File, node ast.Node) {
28 » switch node := node.(type) {
29 » case *ast.RangeStmt:
30 » » checkCopyLocksRange(f, node)
31 » case *ast.FuncDecl:
32 » » checkCopyLocksFunc(f, node)
33 » }
34 }
35
36 // checkCopyLocksFunc checks whether a function might
25 // inadvertently copy a lock, by checking whether 37 // inadvertently copy a lock, by checking whether
26 // its receiver, parameters, or return values 38 // its receiver, parameters, or return values
27 // are locks. 39 // are locks.
28 func checkCopyLocks(f *File, node ast.Node) { 40 func checkCopyLocksFunc(f *File, d *ast.FuncDecl) {
29 » d := node.(*ast.FuncDecl)
30
31 if d.Recv != nil && len(d.Recv.List) > 0 { 41 if d.Recv != nil && len(d.Recv.List) > 0 {
32 expr := d.Recv.List[0].Type 42 expr := d.Recv.List[0].Type
33 if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); pat h != nil { 43 if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); pat h != nil {
34 f.Badf(expr.Pos(), "%s passes Lock by value: %v", d.Name .Name, path) 44 f.Badf(expr.Pos(), "%s passes Lock by value: %v", d.Name .Name, path)
35 } 45 }
36 } 46 }
37 47
38 if d.Type.Params != nil { 48 if d.Type.Params != nil {
39 for _, field := range d.Type.Params.List { 49 for _, field := range d.Type.Params.List {
40 expr := field.Type 50 expr := field.Type
41 if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Ty pe); path != nil { 51 if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Ty pe); path != nil {
42 f.Badf(expr.Pos(), "%s passes Lock by value: %v" , d.Name.Name, path) 52 f.Badf(expr.Pos(), "%s passes Lock by value: %v" , d.Name.Name, path)
43 } 53 }
44 } 54 }
45 } 55 }
46 56
47 if d.Type.Results != nil { 57 if d.Type.Results != nil {
48 for _, field := range d.Type.Results.List { 58 for _, field := range d.Type.Results.List {
49 expr := field.Type 59 expr := field.Type
50 if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Ty pe); path != nil { 60 if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Ty pe); path != nil {
51 f.Badf(expr.Pos(), "%s returns Lock by value: %v ", d.Name.Name, path) 61 f.Badf(expr.Pos(), "%s returns Lock by value: %v ", d.Name.Name, path)
52 } 62 }
53 } 63 }
54 } 64 }
55 } 65 }
56 66
67 // checkCopyLocksRange checks whether a range statement
68 // might inadvertently copy a lock by checking whether
69 // any of the range variables are locks.
70 func checkCopyLocksRange(f *File, r *ast.RangeStmt) {
71 checkCopyLocksRangeVar(f, r.Tok, r.Key)
72 checkCopyLocksRangeVar(f, r.Tok, r.Value)
73 }
74
75 func checkCopyLocksRangeVar(f *File, rtok token.Token, e ast.Expr) {
76 if e == nil {
77 return
78 }
79 id, isId := e.(*ast.Ident)
80 if isId && id.Name == "_" {
81 return
82 }
83
84 var typ types.Type
85 if rtok == token.DEFINE {
86 if !isId {
87 return
88 }
89 obj := f.pkg.defs[id]
90 if obj == nil {
91 return
92 }
93 typ = obj.Type()
94 } else {
95 typ = f.pkg.types[e].Type
96 }
97
98 if typ == nil {
99 return
100 }
101 if path := lockPath(f.pkg.typesPkg, typ); path != nil {
102 f.Badf(e.Pos(), "range var %s copies Lock: %v", f.gofmt(e), path )
103 }
104 }
105
57 type typePath []types.Type 106 type typePath []types.Type
58 107
59 // pathString pretty-prints a typePath. 108 // String pretty-prints a typePath.
60 func (path typePath) String() string { 109 func (path typePath) String() string {
61 n := len(path) 110 n := len(path)
62 var buf bytes.Buffer 111 var buf bytes.Buffer
63 for i := range path { 112 for i := range path {
64 if i > 0 { 113 if i > 0 {
65 fmt.Fprint(&buf, " contains ") 114 fmt.Fprint(&buf, " contains ")
66 } 115 }
67 // The human-readable path is in reverse order, outermost to inn ermost. 116 // The human-readable path is in reverse order, outermost to inn ermost.
68 fmt.Fprint(&buf, path[n-i-1].String()) 117 fmt.Fprint(&buf, path[n-i-1].String())
69 } 118 }
(...skipping 27 matching lines...) Expand all
97 for i := 0; i < nfields; i++ { 146 for i := 0; i < nfields; i++ {
98 ftyp := styp.Field(i).Type() 147 ftyp := styp.Field(i).Type()
99 subpath := lockPath(tpkg, ftyp) 148 subpath := lockPath(tpkg, ftyp)
100 if subpath != nil { 149 if subpath != nil {
101 return append(subpath, typ) 150 return append(subpath, typ)
102 } 151 }
103 } 152 }
104 153
105 return nil 154 return nil
106 } 155 }
LEFTRIGHT

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b