Left: | ||
Right: |
LEFT | RIGHT |
---|---|
1 // Copyright 2014 The Go Authors. All rights reserved. | 1 // Copyright 2014 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 for suspicious shifts. | 6 This file contains the code to check for suspicious shifts. |
7 */ | 7 */ |
8 | 8 |
9 package main | 9 package main |
10 | 10 |
11 import ( | 11 import ( |
12 "go/ast" | 12 "go/ast" |
13 "go/token" | 13 "go/token" |
14 "strconv" | |
15 | 14 |
15 "code.google.com/p/go.tools/go/exact" | |
16 "code.google.com/p/go.tools/go/types" | 16 "code.google.com/p/go.tools/go/types" |
17 ) | 17 ) |
18 | 18 |
19 func init() { | 19 func init() { |
20 register("shift", | 20 register("shift", |
21 » » "check for suspicious shifts", | 21 » » "check for useless shifts", |
22 checkShift, | 22 checkShift, |
23 » » binaryExpr, | 23 » » binaryExpr, assignStmt) |
dsymonds
2014/08/25 06:41:28
join the next line onto this one. (keep the list o
| |
24 » » assignStmt) | |
25 } | 24 } |
26 | 25 |
27 func checkShift(f *File, node ast.Node) { | 26 func checkShift(f *File, node ast.Node) { |
28 switch node := node.(type) { | 27 switch node := node.(type) { |
29 case *ast.BinaryExpr: | 28 case *ast.BinaryExpr: |
30 if node.Op == token.SHL || node.Op == token.SHR { | 29 if node.Op == token.SHL || node.Op == token.SHR { |
31 » » » checkSuspiciousShift(f, node, node.X, node.Y, node.Op) | 30 » » » checkLongShift(f, node, node.X, node.Y) |
32 } | 31 } |
33 case *ast.AssignStmt: | 32 case *ast.AssignStmt: |
34 » » for i, lhs := range node.Lhs { | 33 » » if len(node.Lhs) != 1 || len(node.Rhs) != 1 { |
josharian
2014/08/25 02:40:17
From the spec: "In assignment operations, both the
| |
35 » » » if len(node.Rhs) <= i { | 34 » » » return |
36 » » » » break | 35 » » } |
37 » » » } | 36 » » if node.Tok == token.SHL_ASSIGN || node.Tok == token.SHR_ASSIGN { |
38 » » » if node.Tok == token.SHL_ASSIGN || node.Tok == token.SHR _ASSIGN { | 37 » » » checkLongShift(f, node, node.Lhs[0], node.Rhs[0]) |
39 » » » » rhs := node.Rhs[i] | |
40 » » » » checkSuspiciousShift(f, node, lhs, rhs, node.Tok ) | |
41 » » » } | |
42 } | 38 } |
43 } | 39 } |
44 } | 40 } |
45 | 41 |
46 // checkSuspiciousShift checks if shift or shift-assign operations shift by more | 42 // checkLongShift checks if shift or shift-assign operations shift by more than |
47 // than the length of the underlying variable. | 43 // the length of the underlying variable. |
48 func checkSuspiciousShift(f *File, node ast.Node, x, y ast.Expr, op token.Token) { | 44 func checkLongShift(f *File, node ast.Node, x, y ast.Expr) { |
josharian
2014/08/25 02:40:17
op is unused.
| |
49 » basic, ok := y.(*ast.BasicLit) | 45 » v := f.pkg.types[y].Value |
josharian
2014/08/25 02:40:17
Use f.pkg.types[y].Value to get a constant value i
| |
50 » if !ok || basic.Kind != token.INT { | 46 » if v == nil { |
51 return | 47 return |
52 } | 48 } |
53 » amt, err := strconv.Atoi(basic.Value) | 49 » amt, ok := exact.Int64Val(v) |
54 » if err != nil { | 50 » if !ok { |
55 return | 51 return |
56 } | 52 } |
57 t := f.pkg.types[x].Type | 53 t := f.pkg.types[x].Type |
58 » if t != nil { | 54 » if t == nil { |
59 » » t = t.Underlying() | 55 » » return |
60 } | 56 } |
61 » b, ok := t.(*types.Basic) | 57 » b, ok := t.Underlying().(*types.Basic) |
62 if !ok { | 58 if !ok { |
63 return | 59 return |
64 } | 60 } |
65 » size := 0 | 61 » var size int64 |
62 » var msg string | |
66 switch b.Kind() { | 63 switch b.Kind() { |
67 case types.Uint8, types.Int8: | 64 case types.Uint8, types.Int8: |
68 size = 8 | 65 size = 8 |
josharian
2014/08/25 02:40:17
The max meaningful shift for signed ints is one sm
minux
2014/08/25 04:42:18
this is not right.
int8 >> 8 is still meaningful,
| |
69 case types.Uint16, types.Int16: | 66 case types.Uint16, types.Int16: |
70 size = 16 | 67 size = 16 |
71 case types.Uint32, types.Int32: | 68 case types.Uint32, types.Int32: |
72 size = 32 | 69 size = 32 |
73 case types.Uint64, types.Int64: | 70 case types.Uint64, types.Int64: |
74 size = 64 | 71 size = 64 |
72 case types.Int, types.Uint, types.Uintptr: | |
73 // These types may be as small as 32 bits, but no smaller. | |
74 size = 32 | |
75 msg = "might be " | |
75 default: | 76 default: |
76 return | 77 return |
77 } | 78 } |
78 if amt >= size { | 79 if amt >= size { |
79 ident := f.gofmt(x) | 80 ident := f.gofmt(x) |
80 » » f.Badf(node.Pos(), "suspicious shift of %s", ident) | 81 » » f.Badf(node.Pos(), "%s %stoo small for shift of %d", ident, msg, amt) |
dsymonds
2014/08/25 06:41:28
I'm not sure what one would do with such a message
| |
81 } | 82 } |
82 } | 83 } |
LEFT | RIGHT |