mirror of
https://github.com/golang/go
synced 2024-11-18 06:14:46 -07:00
cmd/vet: assume that no builtin funcs are pure
That was the intention with the existing code, but it was buggy; builtin functions aren't treated as values by types.TypeAndVal. Thus, we should use the IsBuiltin method instead of IsValue. Teaching vet what builtin funcs are pure is already being tracked as a separate issue, #22851. While at it, also add a test with methods, just to be sure that the current logic doesn't break with that edge case either. Fixes #25303. Change-Id: Ic18402b22cceeabf76641c02f575b194b9a536cc Reviewed-on: https://go-review.googlesource.com/112177 Run-TryBot: Daniel Martí <mvdan@mvdan.cc> Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
This commit is contained in:
parent
23e9dc7994
commit
2486ebfb63
@ -147,12 +147,18 @@ func hasSideEffects(f *File, e ast.Expr) bool {
|
|||||||
// conversion as a *types.Named.
|
// conversion as a *types.Named.
|
||||||
typVal := f.pkg.types[n.Fun]
|
typVal := f.pkg.types[n.Fun]
|
||||||
_, isSig := typVal.Type.(*types.Signature)
|
_, isSig := typVal.Type.(*types.Signature)
|
||||||
if typVal.IsValue() && isSig {
|
switch {
|
||||||
|
case typVal.IsValue() && isSig:
|
||||||
// If we have a value of unnamed signature type,
|
// If we have a value of unnamed signature type,
|
||||||
// this CallExpr is a func call and not a type
|
// this CallExpr is a non-builtin func call and
|
||||||
// conversion. Conservatively assume that all
|
// not a type conversion. Conservatively assume
|
||||||
// function and method calls have side effects
|
// that all function and method calls have side
|
||||||
// for now.
|
// effects for now.
|
||||||
|
safe = false
|
||||||
|
return false
|
||||||
|
case typVal.IsBuiltin():
|
||||||
|
// For now, conservatively assume that all
|
||||||
|
// built-in functions have side effects.
|
||||||
safe = false
|
safe = false
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
9
src/cmd/vet/testdata/bool.go
vendored
9
src/cmd/vet/testdata/bool.go
vendored
@ -10,12 +10,18 @@ import "io"
|
|||||||
|
|
||||||
type T int
|
type T int
|
||||||
|
|
||||||
|
func (t T) Foo() int { return int(t) }
|
||||||
|
|
||||||
type FT func() int
|
type FT func() int
|
||||||
|
|
||||||
|
var S []int
|
||||||
|
|
||||||
func RatherStupidConditions() {
|
func RatherStupidConditions() {
|
||||||
var f, g func() int
|
var f, g func() int
|
||||||
if f() == 0 || f() == 0 { // OK f might have side effects
|
if f() == 0 || f() == 0 { // OK f might have side effects
|
||||||
}
|
}
|
||||||
|
var t T
|
||||||
|
_ = t.Foo() == 2 || t.Foo() == 2 // OK Foo might have side effects
|
||||||
if v, w := f(), g(); v == w || v == w { // ERROR "redundant or: v == w || v == w"
|
if v, w := f(), g(); v == w || v == w { // ERROR "redundant or: v == w || v == w"
|
||||||
}
|
}
|
||||||
_ = f == nil || f == nil // ERROR "redundant or: f == nil || f == nil"
|
_ = f == nil || f == nil // ERROR "redundant or: f == nil || f == nil"
|
||||||
@ -25,9 +31,10 @@ func RatherStupidConditions() {
|
|||||||
_ = FT(f) == nil || FT(f) == nil // ERROR "redundant or: FT(f) == nil || FT(f) == nil"
|
_ = FT(f) == nil || FT(f) == nil // ERROR "redundant or: FT(f) == nil || FT(f) == nil"
|
||||||
|
|
||||||
_ = (func() int)(f) == nil || (func() int)(f) == nil // ERROR "redundant or: (func() int)(f) == nil || (func() int)(f) == nil"
|
_ = (func() int)(f) == nil || (func() int)(f) == nil // ERROR "redundant or: (func() int)(f) == nil || (func() int)(f) == nil"
|
||||||
|
_ = append(S, 3) == nil || append(S, 3) == nil // OK append has side effects
|
||||||
|
|
||||||
var namedFuncVar FT
|
var namedFuncVar FT
|
||||||
_ = namedFuncVar() == namedFuncVar() // OK; still func calls
|
_ = namedFuncVar() == namedFuncVar() // OK still func calls
|
||||||
|
|
||||||
var c chan int
|
var c chan int
|
||||||
_ = 0 == <-c || 0 == <-c // OK subsequent receives may yield different values
|
_ = 0 == <-c || 0 == <-c // OK subsequent receives may yield different values
|
||||||
|
Loading…
Reference in New Issue
Block a user