From c200229f9ee1c06bea5c0708d38a5d4807d5c34a Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 15 Aug 2022 22:22:18 -0700 Subject: [PATCH] util/deephash: simplify canMemHash (#5384) Put the t.Size() == 0 check first since this is applicable in all cases. Drop the last struct field conditional since this is covered by the sumFieldSize check at the end. Signed-off-by: Joe Tsai --- util/deephash/deephash.go | 17 +++++++++-------- util/deephash/deephash_test.go | 26 ++++++++++++-------------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/util/deephash/deephash.go b/util/deephash/deephash.go index 15a68ec67..27d3a6b77 100644 --- a/util/deephash/deephash.go +++ b/util/deephash/deephash.go @@ -722,10 +722,15 @@ func typeIsRecursive(t reflect.Type) bool { // canMemHash reports whether a slice of t can be hashed by looking at its // contiguous bytes in memory alone. (e.g. structs with gaps aren't memhashable) func canMemHash(t reflect.Type) bool { + if t.Size() == 0 { + return true + } switch t.Kind() { - case reflect.Bool, reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, - reflect.Uint, reflect.Uintptr, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, - reflect.Float64, reflect.Float32, reflect.Complex128, reflect.Complex64: + case reflect.Bool, + reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr, + reflect.Float32, reflect.Float64, + reflect.Complex64, reflect.Complex128: return true case reflect.Array: return canMemHash(t.Elem()) @@ -734,15 +739,11 @@ func canMemHash(t reflect.Type) bool { for i, numField := 0, t.NumField(); i < numField; i++ { sf := t.Field(i) if !canMemHash(sf.Type) { - // Special case for 0-width fields that aren't at the end. - if sf.Type.Size() == 0 && i < numField-1 { - continue - } return false } sumFieldSize += sf.Type.Size() } - return sumFieldSize == t.Size() // else there are gaps + return sumFieldSize == t.Size() // ensure no gaps } return false } diff --git a/util/deephash/deephash_test.go b/util/deephash/deephash_test.go index 983952b2d..7f0b5f419 100644 --- a/util/deephash/deephash_test.go +++ b/util/deephash/deephash_test.go @@ -379,20 +379,18 @@ func TestCanMemHash(t *testing.T) { _ uint8 _ int }{}, false}, // gap - { - struct { - _ structs.Incomparable // if not last, zero-width - x int - }{}, - true, - }, - { - struct { - x int - _ structs.Incomparable // zero-width last: has space, can't memhash - }{}, - false, - }} + {struct { + _ structs.Incomparable // if not last, zero-width + x int + }{}, true}, + {struct { + x int + _ structs.Incomparable // zero-width last: has space, can't memhash + }{}, + false}, + {[0]chan bool{}, true}, + {struct{ f [0]func() }{}, true}, + } for _, tt := range tests { got := canMemHash(reflect.TypeOf(tt.val)) if got != tt.want {