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

Delta Between Two Patch Sets: src/cmd/vet/rangeloop.go

Issue 6494075: code review 6494075: vet: add range variable misuse detection (Closed)
Left Patch Set: diff -r f48f1de09de5 https://go.googlecode.com/hg Created 11 years, 6 months ago
Right Patch Set: diff -r 42b884930d7c https://go.googlecode.com/hg Created 11 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:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « src/cmd/vet/main.go ('k') | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 // Copyright 2012 The Go Authors. All rights reserved. 1 // Copyright 2012 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 /* 5 /*
6 This file contains the code to check range loop variables bound inside function 6 This file contains the code to check range loop variables bound inside function
7 literals that are deferred or launched in new goroutines. We only check 7 literals that are deferred or launched in new goroutines. We only check
8 instances where the defer or go statement is the last statement in the loop 8 instances where the defer or go statement is the last statement in the loop
9 body, as otherwise we would need whole program analysis. 9 body, as otherwise we would need whole program analysis.
10 10
(...skipping 12 matching lines...) Expand all
23 23
24 import "go/ast" 24 import "go/ast"
25 25
26 // checkRangeLoop walks the body of the provided range statement, checking if 26 // checkRangeLoop walks the body of the provided range statement, checking if
27 // its index or value variables are used unsafely inside goroutines or deferred 27 // its index or value variables are used unsafely inside goroutines or deferred
28 // function literals. 28 // function literals.
29 func checkRangeLoop(f *File, n *ast.RangeStmt) { 29 func checkRangeLoop(f *File, n *ast.RangeStmt) {
30 if !*vetRangeLoops && !*vetAll { 30 if !*vetRangeLoops && !*vetAll {
31 return 31 return
32 } 32 }
33 » walkRangeFor(f, n.Key, n) 33 » key, _ := n.Key.(*ast.Ident)
34 » walkRangeFor(f, n.Value, n) 34 » val, _ := n.Value.(*ast.Ident)
35 } 35 » if key == nil && val == nil {
36
37 func walkRangeFor(f *File, e ast.Expr, n *ast.RangeStmt) {
38 » id, ok := e.(*ast.Ident)
39 » if !ok {
40 return 36 return
41 } 37 }
42 » last := &lastEscapingCallVisitor{} 38 » sl := n.Body.List
43 » ast.Walk(last, n.Body) 39 » if len(sl) == 0 {
44 » if last.call == nil {
45 return 40 return
46 } 41 }
47 » lit, ok := last.call.Fun.(*ast.FuncLit) 42 » var last *ast.CallExpr
43 » switch s := sl[len(sl)-1].(type) {
44 » case *ast.GoStmt:
45 » » last = s.Call
46 » case *ast.DeferStmt:
47 » » last = s.Call
48 » default:
49 » » return
50 » }
51 » lit, ok := last.Fun.(*ast.FuncLit)
48 if !ok { 52 if !ok {
49 return 53 return
50 } 54 }
51 ast.Inspect(lit.Body, func(n ast.Node) bool { 55 ast.Inspect(lit.Body, func(n ast.Node) bool {
52 » » if n, ok := n.(*ast.Ident); ok && n.Obj != nil && n.Obj == id.Ob j { 56 » » if n, ok := n.(*ast.Ident); ok && n.Obj != nil && (n.Obj == key. Obj || n.Obj == val.Obj) {
53 f.Warn(n.Pos(), "range variable", n.Name, "enclosed by f unction") 57 f.Warn(n.Pos(), "range variable", n.Name, "enclosed by f unction")
54 } 58 }
55 return true 59 return true
56 }) 60 })
57 }
58
59 // lastEscapingCallVisitor implements ast.Visitor.
60 // It walks the tree and populates its call field with the last go or defer
61 // call to appear in the tree. If any other construct appears after that call,
62 // the call field will be nil.
63 type lastEscapingCallVisitor struct {
64 call *ast.CallExpr
65 }
66
67 func (v *lastEscapingCallVisitor) Visit(node ast.Node) ast.Visitor {
68 switch s := node.(type) {
69 case *ast.GoStmt:
70 v.call = s.Call
71 return nil
72 case *ast.DeferStmt:
73 v.call = s.Call
74 return nil
75 case nil:
76 default:
77 v.call = nil
78 }
79 return v
80 } 61 }
81 62
82 func BadRangeLoopsUsedInTests() { 63 func BadRangeLoopsUsedInTests() {
83 var s []int 64 var s []int
84 for i, v := range s { 65 for i, v := range s {
85 go func() { 66 go func() {
86 println(i) // ERROR "range variable i enclosed by functi on" 67 println(i) // ERROR "range variable i enclosed by functi on"
87 println(v) // ERROR "range variable v enclosed by functi on" 68 println(v) // ERROR "range variable v enclosed by functi on"
88 }() 69 }()
89 } 70 }
90 for i, v := range s { 71 for i, v := range s {
91 defer func() { 72 defer func() {
92 println(i) // ERROR "range variable i enclosed by functi on" 73 println(i) // ERROR "range variable i enclosed by functi on"
93 println(v) // ERROR "range variable v enclosed by functi on" 74 println(v) // ERROR "range variable v enclosed by functi on"
94 }() 75 }()
95 } 76 }
96 » for i, v := range s { 77 » for i := range s {
97 » » { 78 » » go func() {
98 » » » go func() { 79 » » » println(i) // ERROR "range variable i enclosed by functi on"
99 » » » » println(i) // ERROR "range variable i enclosed b y function" 80 » » }()
100 » » » » println(v) // ERROR "range variable v enclosed b y function" 81 » }
101 » » » }() 82 » for _, v := range s {
102 » » } 83 » » go func() {
84 » » » println(v) // ERROR "range variable v enclosed by functi on"
85 » » }()
103 } 86 }
104 for i, v := range s { 87 for i, v := range s {
105 go func() { 88 go func() {
106 println(i, v) 89 println(i, v)
107 }() 90 }()
108 println("unfortunately, we don't catch the error above because o f this statement") 91 println("unfortunately, we don't catch the error above because o f this statement")
109 } 92 }
110 for i, v := range s { 93 for i, v := range s {
111 go func(i, v int) { 94 go func(i, v int) {
112 println(i, v) 95 println(i, v)
113 }(i, v) 96 }(i, v)
114 } 97 }
115 for i, v := range s { 98 for i, v := range s {
116 i, v := i, v 99 i, v := i, v
117 go func() { 100 go func() {
118 println(i, v) 101 println(i, v)
119 }() 102 }()
120 } 103 }
121 } 104 }
LEFTRIGHT

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