reflect: rename Mapiter.SetKey to Value.SetIterKey

Same for Value. Add a bigger test. Include some shouldPanic checks. Fix a bug in assignment conversion. Fixes #48294 Change-Id: Id863ee5122a5787a7b35574b18586fd24d118788 Reviewed-on: https://go-review.googlesource.com/c/go/+/356049 Trust: Keith Randall <[email protected]> Trust: Josh Bleecher Snyder <[email protected]> Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Josh Bleecher Snyder <[email protected]>
golang · Oct 15, 2021 · 8dab959 · 8dab959
1 parent 3da0ff8
commit 8dab959
Show file tree

Hide file tree

Showing 2 changed files with 108 additions and 30 deletions.
diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
@@ -348,8 +348,8 @@ func TestMapIterSet(t *testing.T) {
 
 	iter := v.MapRange()
 	for iter.Next() {
-		iter.SetKey(k)
-		iter.SetValue(e)
+		k.SetIterKey(iter)
+		e.SetIterValue(iter)
 		want := m[k.String()]
 		got := e.Interface()
 		if got != want {
@@ -366,8 +366,8 @@ func TestMapIterSet(t *testing.T) {
 	got := int(testing.AllocsPerRun(10, func() {
 		iter := v.MapRange()
 		for iter.Next() {
-			iter.SetKey(k)
-			iter.SetValue(e)
+			k.SetIterKey(iter)
+			e.SetIterValue(iter)
 		}
 	}))
 	// Making a *MapIter allocates. This should be the only allocation.
@@ -7475,9 +7475,9 @@ func TestMapIterReset(t *testing.T) {
 		var seenk, seenv uint64
 		iter.Reset(ValueOf(m3))
 		for iter.Next() {
-			iter.SetKey(kv)
+			kv.SetIterKey(iter)
 			seenk ^= kv.Uint()
-			iter.SetValue(kv)
+			kv.SetIterValue(iter)
 			seenv ^= kv.Uint()
 		}
 		if seenk != 0b111 {
@@ -7619,3 +7619,81 @@ func TestConvertibleTo(t *testing.T) {
 		t.Fatalf("(%s).ConvertibleTo(%s) = true, want false", t3, t4)
 	}
 }
+
+func TestSetIter(t *testing.T) {
+	data := map[string]int{
+		"foo": 1,
+		"bar": 2,
+		"baz": 3,
+	}
+
+	m := ValueOf(data)
+	i := m.MapRange()
+	k := New(TypeOf("")).Elem()
+	v := New(TypeOf(0)).Elem()
+	shouldPanic("Value.SetIterKey called before Next", func() {
+		k.SetIterKey(i)
+	})
+	shouldPanic("Value.SetIterValue called before Next", func() {
+		k.SetIterValue(i)
+	})
+	data2 := map[string]int{}
+	for i.Next() {
+		k.SetIterKey(i)
+		v.SetIterValue(i)
+		data2[k.Interface().(string)] = v.Interface().(int)
+	}
+	if !DeepEqual(data, data2) {
+		t.Errorf("maps not equal, got %v want %v", data2, data)
+	}
+	shouldPanic("Value.SetIterKey called on exhausted iterator", func() {
+		k.SetIterKey(i)
+	})
+	shouldPanic("Value.SetIterValue called on exhausted iterator", func() {
+		k.SetIterValue(i)
+	})
+
+	i.Reset(m)
+	i.Next()
+	shouldPanic("Value.SetIterKey using unaddressable value", func() {
+		ValueOf("").SetIterKey(i)
+	})
+	shouldPanic("Value.SetIterValue using unaddressable value", func() {
+		ValueOf(0).SetIterValue(i)
+	})
+	shouldPanic("value of type string is not assignable to type int", func() {
+		New(TypeOf(0)).Elem().SetIterKey(i)
+	})
+	shouldPanic("value of type int is not assignable to type string", func() {
+		New(TypeOf("")).Elem().SetIterValue(i)
+	})
+
+	// Make sure assignment conversion works.
+	var x interface{}
+	y := ValueOf(&x).Elem()
+	y.SetIterKey(i)
+	if _, ok := data[x.(string)]; !ok {
+		t.Errorf("got key %s which is not in map", x)
+	}
+	y.SetIterValue(i)
+	if x.(int) < 1 || x.(int) > 3 {
+		t.Errorf("got value %d which is not in map", x)
+	}
+
+	// Try some key/value types which are direct interfaces.
+	a := 88
+	b := 99
+	pp := map[*int]*int{
+		&a: &b,
+	}
+	i = ValueOf(pp).MapRange()
+	i.Next()
+	y.SetIterKey(i)
+	if got := *y.Interface().(*int); got != a {
+		t.Errorf("pointer incorrect: got %d want %d", got, a)
+	}
+	y.SetIterValue(i)
+	if got := *y.Interface().(*int); got != b {
+		t.Errorf("pointer incorrect: got %d want %d", got, b)
+	}
+}
diff --git a/src/reflect/value.go b/src/reflect/value.go
@@ -1651,30 +1651,30 @@ func (iter *MapIter) Key() Value {
 	return copyVal(ktype, iter.m.flag.ro()|flag(ktype.Kind()), iterkey)
 }
 
-// SetKey assigns dst to the key of iter's current map entry.
-// It is equivalent to dst.Set(i.Key()), but it avoids allocating a new Value.
-// As in Go, the key must be assignable to dst's type.
-func (iter *MapIter) SetKey(dst Value) {
+// SetIterKey assigns to v the key of iter's current map entry.
+// It is equivalent to v.Set(iter.Key()), but it avoids allocating a new Value.
+// As in Go, the key must be assignable to v's type.
+func (v Value) SetIterKey(iter *MapIter) {
 	if !iter.hiter.initialized() {
-		panic("MapIter.SetKey called before Next")
+		panic("reflect: Value.SetIterKey called before Next")
 	}
 	iterkey := mapiterkey(&iter.hiter)
 	if iterkey == nil {
-		panic("MapIter.SetKey called on exhausted iterator")
+		panic("reflect: Value.SetIterKey called on exhausted iterator")
 	}
 
-	dst.mustBeAssignable()
+	v.mustBeAssignable()
 	var target unsafe.Pointer
-	if dst.kind() == Interface {
-		target = dst.ptr
+	if v.kind() == Interface {
+		target = v.ptr
 	}
 
 	t := (*mapType)(unsafe.Pointer(iter.m.typ))
 	ktype := t.key
 
-	key := Value{ktype, iterkey, iter.m.flag | flag(ktype.Kind())}
-	key = key.assignTo("reflect.MapIter.SetKey", dst.typ, target)
-	typedmemmove(dst.typ, dst.ptr, key.ptr)
+	key := Value{ktype, iterkey, iter.m.flag | flag(ktype.Kind()) | flagIndir}
+	key = key.assignTo("reflect.MapIter.SetKey", v.typ, target)
+	typedmemmove(v.typ, v.ptr, key.ptr)
 }
 
 // Value returns the value of iter's current map entry.
@@ -1692,30 +1692,30 @@ func (iter *MapIter) Value() Value {
 	return copyVal(vtype, iter.m.flag.ro()|flag(vtype.Kind()), iterelem)
 }
 
-// SetValue assigns dst to the value of iter's current map entry.
-// It is equivalent to dst.Set(i.Value()), but it avoids allocating a new Value.
-// As in Go, the value must be assignable to dst's type.
-func (iter *MapIter) SetValue(dst Value) {
+// SetIterValue assigns to v the value of iter's current map entry.
+// It is equivalent to v.Set(iter.Value()), but it avoids allocating a new Value.
+// As in Go, the value must be assignable to v's type.
+func (v Value) SetIterValue(iter *MapIter) {
 	if !iter.hiter.initialized() {
-		panic("MapIter.SetValue called before Next")
+		panic("reflect: Value.SetIterValue called before Next")
 	}
 	iterelem := mapiterelem(&iter.hiter)
 	if iterelem == nil {
-		panic("MapIter.SetValue called on exhausted iterator")
+		panic("reflect: Value.SetIterValue called on exhausted iterator")
 	}
 
-	dst.mustBeAssignable()
+	v.mustBeAssignable()
 	var target unsafe.Pointer
-	if dst.kind() == Interface {
-		target = dst.ptr
+	if v.kind() == Interface {
+		target = v.ptr
 	}
 
 	t := (*mapType)(unsafe.Pointer(iter.m.typ))
 	vtype := t.elem
 
-	elem := Value{vtype, iterelem, iter.m.flag | flag(vtype.Kind())}
-	elem = elem.assignTo("reflect.MapIter.SetValue", dst.typ, target)
-	typedmemmove(dst.typ, dst.ptr, elem.ptr)
+	elem := Value{vtype, iterelem, iter.m.flag | flag(vtype.Kind()) | flagIndir}
+	elem = elem.assignTo("reflect.MapIter.SetValue", v.typ, target)
+	typedmemmove(v.typ, v.ptr, elem.ptr)
 }
 
 // Next advances the map iterator and reports whether there is another