Skip to content

Commit

Permalink
Explicitly check each interface type in core.IsAnyNilOrZero (#7168)
Browse files Browse the repository at this point in the history 
I introduced a bug in #7162
which lead to incorrect type comparisons.

`IsAnyNilOrZero` takes a variadic amount of interfaces with each
interface containing a value with underlying type `string`, `int32`,
`float64`, etc and checks for that type's zero value in a switch
statement. When checking for multiple numeric types on the same case
statement line, the compiler will default to `int` rather than implying
the type of the untyped constant `0` leading to an incorrect result at
runtime. Contrast that to when each numeric type is on its own case
statement line where a correct comparison can be made.

Per [go.dev/blog/constants](https://go.dev/blog/constants):
> The default type of an untyped constant is determined by its syntax.
For string constants, the only possible implicit type is string. For
[numeric constants](https://go.dev/ref/spec#Numeric_types), the implicit
type has more variety. Integer constants default to int, floating-point
constants float64, rune constants to rune (an alias for int32), and
imaginary constants to complex128.

Per
[go.dev/doc/effective_go](https://go.dev/doc/effective_go#interfaces_and_types):
> Constants in Go are just that—constant. They are created at compile
time, even when defined as locals in functions, and can only be numbers,
characters (runes), strings or booleans. Because of the compile-time
restriction, the expressions that define them must be constant
expressions, evaluatable by the compiler.

See the following code for an explicit example.
```
package main
import "fmt"
func main() {
	for _, val := range []interface{}{1, int8(1), int16(1), int32(1), int64(1)} {
		switch v := val.(type) {
		case int, int8, int16, int32, int64:
			fmt.Printf("Type %T:\t%v\n", v, v == 1)
		}
	}
        fmt.Println("-----------------------------")
	for _, val := range []interface{}{1, int8(1), int16(1), int32(1), int64(1)} {
		switch v := val.(type) {
		case int:
			fmt.Printf("Type %T:\t%v\n", v, v == 1)
		case int8:
			fmt.Printf("Type %T:\t%v\n", v, v == 1)
		case int16:
			fmt.Printf("Type %T:\t%v\n", v, v == 1)
		case int32:
			fmt.Printf("Type %T:\t%v\n", v, v == 1)
		case int64:
			fmt.Printf("Type %T:\t%v\n", v, v == 1)
		}
	}
}

-------
Type int:	true
Type int8:	false
Type int16:	false
Type int32:	false
Type int64:	false
-----------------------------
Type int:	true
Type int8:	true
Type int16:	true
Type int32:	true
Type int64:	true
```
  • Loading branch information
@pgporada
pgporada committed Nov 20, 2023
1 parent 01f2336 commit caf73f2
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 6 deletions.
41 changes: 39 additions & 2 deletions core/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,18 +222,55 @@ func IsAnyNilOrZero(vals ...interface{}) bool {
return true
}
case byte:
// Byte is an alias for uint8 and will cover that case.
if v == 0 {
return true
}
case []byte:
if len(v) == 0 {
return true
}
case int, int8, int16, int32, int64, uint, uint16, uint32, uint64:
case int:
if v == 0 {
return true
}
case float32, float64:
case int8:
if v == 0 {
return true
}
case int16:
if v == 0 {
return true
}
case int32:
if v == 0 {
return true
}
case int64:
if v == 0 {
return true
}
case uint:
if v == 0 {
return true
}
case uint16:
if v == 0 {
return true
}
case uint32:
if v == 0 {
return true
}
case uint64:
if v == 0 {
return true
}
case float32:
if v == 0 {
return true
}
case float64:
if v == 0 {
return true
}
Expand Down
53 changes: 49 additions & 4 deletions core/util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,36 @@ func TestIsAnyNilOrZero(t *testing.T) {
test.Assert(t, IsAnyNilOrZero(false), "False bool seen as non-zero")
test.Assert(t, !IsAnyNilOrZero(true), "True bool seen as zero")

test.Assert(t, IsAnyNilOrZero(0), "Zero num seen as non-zero")
test.Assert(t, !IsAnyNilOrZero(uint32(5)), "Non-zero num seen as zero")
test.Assert(t, !IsAnyNilOrZero(-12.345), "Non-zero num seen as zero")
test.Assert(t, IsAnyNilOrZero(0), "Untyped constant zero seen as non-zero")
test.Assert(t, !IsAnyNilOrZero(1), "Untyped constant 1 seen as zero")
test.Assert(t, IsAnyNilOrZero(int(0)), "int(0) seen as non-zero")
test.Assert(t, !IsAnyNilOrZero(int(1)), "int(1) seen as zero")
test.Assert(t, IsAnyNilOrZero(int8(0)), "int8(0) seen as non-zero")
test.Assert(t, !IsAnyNilOrZero(int8(1)), "int8(1) seen as zero")
test.Assert(t, IsAnyNilOrZero(int16(0)), "int16(0) seen as non-zero")
test.Assert(t, !IsAnyNilOrZero(int16(1)), "int16(1) seen as zero")
test.Assert(t, IsAnyNilOrZero(int32(0)), "int32(0) seen as non-zero")
test.Assert(t, !IsAnyNilOrZero(int32(1)), "int32(1) seen as zero")
test.Assert(t, IsAnyNilOrZero(int64(0)), "int64(0) seen as non-zero")
test.Assert(t, !IsAnyNilOrZero(int64(1)), "int64(1) seen as zero")

test.Assert(t, IsAnyNilOrZero(uint(0)), "uint(0) seen as non-zero")
test.Assert(t, !IsAnyNilOrZero(uint(1)), "uint(1) seen as zero")
test.Assert(t, IsAnyNilOrZero(uint8(0)), "uint8(0) seen as non-zero")
test.Assert(t, !IsAnyNilOrZero(uint8(1)), "uint8(1) seen as zero")
test.Assert(t, IsAnyNilOrZero(uint16(0)), "uint16(0) seen as non-zero")
test.Assert(t, !IsAnyNilOrZero(uint16(1)), "uint16(1) seen as zero")
test.Assert(t, IsAnyNilOrZero(uint32(0)), "uint32(0) seen as non-zero")
test.Assert(t, !IsAnyNilOrZero(uint32(1)), "uint32(1) seen as zero")
test.Assert(t, IsAnyNilOrZero(uint64(0)), "uint64(0) seen as non-zero")
test.Assert(t, !IsAnyNilOrZero(uint64(1)), "uint64(1) seen as zero")

test.Assert(t, !IsAnyNilOrZero(-12.345), "Untyped float32 seen as zero")
test.Assert(t, !IsAnyNilOrZero(float32(6.66)), "Non-empty float32 seen as zero")
test.Assert(t, IsAnyNilOrZero(float32(0)), "Empty float32 seen as non-zero")

test.Assert(t, !IsAnyNilOrZero(float64(7.77)), "Non-empty float64 seen as zero")
test.Assert(t, IsAnyNilOrZero(float64(0)), "Empty float64 seen as non-zero")

test.Assert(t, IsAnyNilOrZero(""), "Empty string seen as non-zero")
test.Assert(t, !IsAnyNilOrZero("string"), "Non-empty string seen as zero")
Expand Down Expand Up @@ -158,10 +185,28 @@ func BenchmarkIsAnyNilOrZero(b *testing.B) {
}{
{input: int(0)},
{input: int(1)},
{input: int8(0)},
{input: int8(1)},
{input: int16(0)},
{input: int16(1)},
{input: int32(0)},
{input: int32(1)},
{input: int64(0)},
{input: int64(1)},
{input: uint(0)},
{input: uint(1)},
{input: uint8(0)},
{input: uint8(1)},
{input: uint16(0)},
{input: uint16(1)},
{input: uint32(0)},
{input: uint32(1)},
{input: uint64(0)},
{input: uint64(1)},
{input: float32(0)},
{input: float32(0.1)},
{input: float64(0)},
{input: float64(0.1)},
{input: ""},
{input: "ahoyhoy"},
{input: []string{}},
Expand All @@ -185,7 +230,7 @@ func BenchmarkIsAnyNilOrZero(b *testing.B) {
}

for _, v := range table {
b.Run(fmt.Sprintf("input_%v", v.input), func(b *testing.B) {
b.Run(fmt.Sprintf("input_%T_%v", v.input, v.input), func(b *testing.B) {
for i := 0; i < b.N; i++ {
_ = IsAnyNilOrZero(v.input)
}
Expand Down

0 comments on commit caf73f2

Please sign in to comment.