Skip to content

Commit 01dccea

Browse files
hopehookromaindoumenc
authored andcommitted
reflect: panic when Value.Equal using two non-comparable values
Assuming the two values are valid and non-comparable, Equal should panic. x := reflect.ValueOf([]int{1, 2, 3}) x.Equal(x) // can not report false, should panic Assuming one of them is non-comparable and the other is invalid, it should always report false. x := reflect.ValueOf([]int{1, 2, 3}) y := reflect.ValueOf(nil) x.Equal(y) // should report false For golang#46746. Change-Id: Ifecd77ca0b3de3019fae2be39048f9277831676c Reviewed-on: https://go-review.googlesource.com/c/go/+/440037 Reviewed-by: Ian Lance Taylor <[email protected]> Reviewed-by: David Chase <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]> Auto-Submit: Ian Lance Taylor <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent a6b1217 commit 01dccea

File tree

2 files changed

+38
-17
lines changed

2 files changed

+38
-17
lines changed

src/reflect/all_test.go

+35-15
Original file line numberDiff line numberDiff line change
@@ -8244,16 +8244,6 @@ var valueEqualTests = []ValueEqualTest{
82448244
true,
82458245
true, false,
82468246
},
8247-
{
8248-
&equalSlice, []int{1},
8249-
false,
8250-
true, false,
8251-
},
8252-
{
8253-
map[int]int{}, map[int]int{},
8254-
false,
8255-
false, false,
8256-
},
82578247
{
82588248
(chan int)(nil), nil,
82598249
false,
@@ -8289,11 +8279,6 @@ var valueEqualTests = []ValueEqualTest{
82898279
true,
82908280
false, false,
82918281
},
8292-
{
8293-
&mapInterface, &mapInterface,
8294-
false,
8295-
true, true,
8296-
},
82978282
}
82988283

82998284
func TestValue_Equal(t *testing.T) {
@@ -8324,6 +8309,41 @@ func TestValue_Equal(t *testing.T) {
83248309
}
83258310
}
83268311

8312+
func TestValue_EqualNonComparable(t *testing.T) {
8313+
var invalid = Value{} // ValueOf(nil)
8314+
var values = []Value{
8315+
// Value of slice is non-comparable.
8316+
ValueOf([]int(nil)),
8317+
ValueOf(([]int{})),
8318+
8319+
// Value of map is non-comparable.
8320+
ValueOf(map[int]int(nil)),
8321+
ValueOf((map[int]int{})),
8322+
8323+
// Value of func is non-comparable.
8324+
ValueOf(((func())(nil))),
8325+
ValueOf(func() {}),
8326+
8327+
// Value of struct is non-comparable because of non-comparable elements.
8328+
ValueOf((NonComparableStruct{})),
8329+
8330+
// Value of array is non-comparable because of non-comparable elements.
8331+
ValueOf([0]map[int]int{}),
8332+
ValueOf([0]func(){}),
8333+
ValueOf(([1]struct{ I interface{} }{{[]int{}}})),
8334+
ValueOf(([1]interface{}{[1]interface{}{map[int]int{}}})),
8335+
}
8336+
for _, value := range values {
8337+
// Panic when reflect.Value.Equal using two valid non-comparable values.
8338+
shouldPanic("reflect.Value.Equal using two non-comparable values", func() { value.Equal(value) })
8339+
8340+
// If one is non-comparable and the other is invalid, the expected result is always false.
8341+
if r := value.Equal(invalid); r != false {
8342+
t.Errorf("%s == invalid got %t, want false", value.Type(), r)
8343+
}
8344+
}
8345+
}
8346+
83278347
func TestInitFuncTypes(t *testing.T) {
83288348
n := 100
83298349
var wg sync.WaitGroup

src/reflect/value.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -3320,7 +3320,8 @@ func (v Value) Comparable() bool {
33203320
}
33213321

33223322
// Equal reports true if v is equal to u.
3323-
// For valid values, if either v or u is non-comparable, Equal returns false.
3323+
// For valid values, if one of them is non-comparable, and the other is comparable,
3324+
// Equal reports false; if v and u are both non-comparable, Equal will panic.
33243325
func (v Value) Equal(u Value) bool {
33253326
if !v.IsValid() || !u.IsValid() {
33263327
return v.IsValid() == u.IsValid()
@@ -3334,7 +3335,7 @@ func (v Value) Equal(u Value) bool {
33343335
return v.Elem().Equal(u.Elem())
33353336
}
33363337

3337-
return false
3338+
panic("reflect.Value.Equal using two non-comparable values")
33383339
}
33393340

33403341
// convertOp returns the function to convert a value of type src

0 commit comments

Comments
 (0)