new reflect.Value method: SetMem

[Russ Cox]

I propose to add a SetMem method to reflect.Value.
dst.SetMem(src) is like, in C, memmove(&dst, &src, sizeof dst),
restricted to the cases where the types involved are the same except
in name and possibly signedness.

// SetMem copies the memory representation of x into v.
// It panics if v.CanSet() is false or if v's type is not memory
assignable to x's type.
//
// A value of type T1 is memory assignable to type T2 or vice versa if
any of these are true:
//
// - T1 and T2 are the same underlying type, regardless of name
// - T1 and T2 are 8-bit integer types (int8, uint8)
// - T1 and T2 are 16-bit integer types (int16, uint16)
// - T1 and T2 are 32-bit integer types (int32, uint32; int, uint if
they are 32 bits)
// - T1 and T2 are 64-bit integer types (int64, uint64; int, uint if
they are 64 bits)
// - T1 and T2 are arrays of the same length
// with memory assignable element types
// - T1 and T2 are pointers with memory assignable element types.
// - T1 and T2 are slices with memory assignable element types
// - T1 and T2 are structs containing only exported fields, and
// corresponding field types in the two structs are memory assignable.
//
// A caller can check whether two types are memory assignable by using
// a Type's MemAssignableTo method.
//
func (v Value) SetMem(x Value)

You could use this, for example, to assign a struct { X []uint32; Y
[]uint64 } to a struct { A []int32; B []int64 }. Note that both field
names and element types changed.

One possibility would be to expand 'integer types' to 'numeric types',
which would add float32 to the 32-bit list and float64 and complex64
to the 64-bit list. That would allow some somewhat nonsensical results
like copying a complex64 into a float64, but it would also allow
things like converting a []float32 into a []uint32 where each uint32
was the corresponding float32 bits (what math.Float32bits returns).
The current definition has the nice property that the conversions
involved are only from signed to unsigned and back, so they are only
"conversions" that already happen in Go programs.

Russ

[Russ Cox]

I believe this proposal has fairly general uses, but here is one:
making the protocol buffer library compile for App Engine using
package reflect instead of package unsafe:
http://codereview.appspot.com/6334045

Russ

[i3dmaster]

Can I think of it as a more generic version of builtin copy or
reflect.Copy? Another question, does it really need panic if failed?
It sounds like you will check CanSet first, so assume no real harm has
done yet if the check is failed?

Thanks,

[Ian Lance Taylor]

Russ Cox <r...@golang.org> writes:

> http://codereview.appspot.com/6334045

I can't see this for some reason. I can see the description but the
patch is empty.

Ian

[roger peppe]

This seems like a nice idea, though I'm slightly concerned that it
might become over-used to do conversions that
should really be statically checked by the compiler.

For instance the fact that it allows conversion between int and int32
means that when we move to 64 bit ints, we could get
avoidable runtime code breakage. (One scenario: I use
encoding/binary to read into []int32, then use reflect
to convert to []int).

[Rob 'Commander' Pike]

I'd prefer if int and uintptr were not on the list, since their copying by this function will render the program non-portable. Only explicitly-sized numeric base types should be allowed.

Also I'd like to see a more concise definition, one that better captures the spirit of the conversion. Something like:

- Both base types' Kind must be a sized integer Kind: Int8, Uint64 etc. but not Int or Uintptr.
- The two base types must be of the same size but may be of different signedness.
- Given two such base types, one may:
- copy two values of those types (that is, copy a value of one type to a value of the other type)
- copy two arrays of those element types of the same length elementwise
- copy two slices of those element types of the same length elementwise
- copy two pointers of those element types
- copy two structs of exported-only fields that each satisfy the conditions above, in order, field by field. The copy is recursive for structs and arrays (only). The elements of fields of slice and pointer type are not copied; only the field value is copied.

Note that these rules implement only operations already possible in Go using a subset of the legal conversions.

-rob

[Kyle Lemons]

On Fri, Jun 22, 2012 at 8:41 AM, Rob 'Commander' Pike <r...@google.com> wrote:

I'd prefer if int and uintptr were not on the list, since their copying by this function will render the program non-portable. Only explicitly-sized numeric base types should be allowed.

What about allowing conversion from int to uint and back (but not to their explicit counterparts)?

Also I'd like to see a more concise definition, one that better captures the spirit of the conversion.  Something like:

- Both base types' Kind must be a sized integer Kind: Int8, Uint64 etc. but not Int or Uintptr.
- The two base types must be of the same size but may be of different signedness.
- Given two such base types, one may:
 - copy two values of those types (that is, copy a value of one type to a value of the other type)
 - copy two arrays of those element types of the same length elementwise
 - copy two slices of those element types of the same length elementwise
 - copy two pointers of those element types
 - copy two structs of exported-only fields that each satisfy the conditions above, in order, field by field. The copy is recursive for structs and arrays (only).  The elements of fields of slice and pointer type are not copied; only the field value is copied.

+1 for itemizing the allowed cases

[roger peppe]

On Jun 23, 2012 1:16 AM, "Kyle Lemons" <kev...@google.com> wrote:
>
> On Fri, Jun 22, 2012 at 8:41 AM, Rob 'Commander' Pike <r...@google.com> wrote:
>>
>> I'd prefer if int and uintptr were not on the list, since their copying by this function will render the program non-portable. Only explicitly-sized numeric base types should be allowed.
>
> What about allowing conversion from int to uint and back (but not to their explicit counterparts)?

+1

[Rob 'Commander' Pike]

On Jun 23, 2012, at 1:38 AM, roger peppe wrote:

On Jun 23, 2012 1:16 AM, "Kyle Lemons" <kev...@google.com> wrote:
>
> On Fri, Jun 22, 2012 at 8:41 AM, Rob 'Commander' Pike <r...@google.com> wrote:
>>
>> I'd prefer if int and uintptr were not on the list, since their copying by this function will render the program non-portable. Only explicitly-sized numeric base types should be allowed.
>
> What about allowing conversion from int to uint and back (but not to their explicit counterparts)?

That wouldn't introduce non-portabilities.

I must say I find this all a bit distasteful even though I understand its purpose. It doesn't smell quite right to have what is in essence a speed hack be part of reflect. Still thinking...

-rob

[Ian Lance Taylor]

Rob 'Commander' Pike <r...@google.com> writes:

> I must say I find this all a bit distasteful even though I understand
> its purpose. It doesn't smell quite right to have what is in essence a
> speed hack be part of reflect. Still thinking...

I still can't see anything in http://codereview.appspot.com/6334045 , so
I don't know how this will be used. But perhaps a more palatable way to
approach this would be to add ConvertAndSet to the reflect package,
which would implement the same conversions the language permits. Right
now reflect.Set implements the rules for assignment, so v.Set(x) is the
reflect version as v = x. Assuming v has type T, we could say that
v.ConvertAndSet(x) is the reflect version of v = T(x). This would be
more complicated than SetMem, because it would have to handle numeric
conversions, but the cases that the SetMem proposal handles could be
equally fast. And the meaning would be well-defined and wouldn't add
anything new or unsafe to the language.

Ian

[Russ Cox]

I don't think this is a speed hack. I think of it as a memory-safe
cast or memmove, to allow (safe) reinterpretation of bits as other
types. It would not work to allow just the Go conversions, because
even in Go you cannot turn a []uint32 into a []int32, or given type T
int32, a []T into a []int32. This arises in reflect because you
sometimes want to operate on the underlying type definition regardless
of the named type. Being able to switch signedness is a bonus for a
similar reason: it narrows the number of cases you really need to
handle. The same argument applies for turning []float32 into []uint32,
although I know that one is contentious, and it's not part of the
current proposal.

Russ

[Russ Cox]

> I still can't see anything in http://codereview.appspot.com/6334045 , so
> I don't know how this will be used.  But perhaps a more palatable way to
> approach this would be to add ConvertAndSet to the reflect package,
> which would implement the same conversions the language permits.

I finally got that CL to upload, so now there is code there.
Here is a representative example from pointer_reflect.go:

83 func (p pointer) Int32s(s *scratch) *[]int32 {
84 reflect.ValueOf(&s.int32sp).Elem().SetMem(p.v)
85 return s.int32sp
86 }

This takes a reflect.Value p.v and assigns it to a *[]int32 value
s.int32sp. The reflect.Value is often a *[]int32 but sometimes a
*[]uint32 or *[]T where type T int32.

We added the Value.Bytes and Value.SetBytes methods as a special case
to reflect for extracting and setting the underlying []byte, so that
if you had type T byte and a []T (and had not planned for this unknown
type T), you didn't need to use reflect to get at each byte
individually. This is the more general case of that.

Russ