From 3fbfc83db298c55669b6165eca1b8a56b04c895c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Fri, 4 May 2018 14:39:00 +0700 Subject: [PATCH] cmd/vet: recognise func type conversions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In hasSideEffects, vet has to be taught whether or not a CallExpr is an actual function call, or just a type conversion. The previous code knew how to differentiate fn(arg) from int(arg), but it incorrectly saw (func(T))(fn) as a func call. This edge case is slightly tricky, since the CallExpr.Fun has a func signature type, just like in func calls. However, the difference is that in this case the Fun is a type, not a value. This information is in types.TypeAndValue, so use it. Change-Id: I18bb8b23abbe7decc558b726ff2dc31fae2f13d6 Reviewed-on: https://go-review.googlesource.com/111416 Run-TryBot: Daniel Martí TryBot-Result: Gobot Gobot Reviewed-by: Robert Griesemer --- src/cmd/vet/bool.go | 15 ++++++++------- src/cmd/vet/testdata/bool.go | 3 +-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/cmd/vet/bool.go b/src/cmd/vet/bool.go index 31e81ec4bf..67321c3df4 100644 --- a/src/cmd/vet/bool.go +++ b/src/cmd/vet/bool.go @@ -145,13 +145,14 @@ func hasSideEffects(f *File, e ast.Expr) bool { // Don't call Type.Underlying(), since its lack // lets us see the NamedFuncType(x) type // conversion as a *types.Named. - _, ok := f.pkg.types[n.Fun].Type.(*types.Signature) - if ok { - // Conservatively assume that all function and - // method calls have side effects for - // now. This will include func type - // conversions, but it's ok given that - // this is the conservative side. + typVal := f.pkg.types[n.Fun] + _, isSig := typVal.Type.(*types.Signature) + if typVal.IsValue() && isSig { + // If we have a value of unnamed signature type, + // this CallExpr is a func call and not a type + // conversion. Conservatively assume that all + // function and method calls have side effects + // for now. safe = false return false } diff --git a/src/cmd/vet/testdata/bool.go b/src/cmd/vet/testdata/bool.go index bada13ae0d..be78caac18 100644 --- a/src/cmd/vet/testdata/bool.go +++ b/src/cmd/vet/testdata/bool.go @@ -24,8 +24,7 @@ func RatherStupidConditions() { _ = i == T(2) || i == T(2) // ERROR "redundant or: i == T(2) || i == T(2)" _ = FT(f) == nil || FT(f) == nil // ERROR "redundant or: FT(f) == nil || FT(f) == nil" - // TODO: distinguish from an actual func call - _ = (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" var namedFuncVar FT _ = namedFuncVar() == namedFuncVar() // OK; still func calls