Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slice element types are 'masked' when reflect.Value.Index is called on slices of interfaces #39096

Closed
dany74q opened this issue May 15, 2020 · 5 comments

Comments

@dany74q
Copy link

@dany74q dany74q commented May 15, 2020

What version of Go are you using (go version)?

$ go version
go version go1.14.2 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/dany74q/Library/Caches/go-build"
GOENV="/Users/dany74q/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOOS="darwin"
GOPATH="/Users/dany74q/go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.14.2_1/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.14.2_1/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/vz/n93rj9qs3mlb902f5z9b99ph0000gn/T/go-build920636391=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

The following code blocks yield different results, and I believe that this inconsistency might not have been intentional (and could be a potential source for bugs).

When iterating on a slice of interfaces (say, the empty interface - this applies to every interface though), passing each value to reflect.ValueOf,
and getting the type / kind of the returned reflect.Value - each value conserves the concrete type, for instance:

// https://play.golang.org/p/InkfIzrApBq

func main() {
	values := []interface{}{1, "str", 3.0, &struct{}{}, struct{}{}, nil}

	for i, value := range values {
		valueOf := reflect.ValueOf(value)
		kind := valueOf.Kind()

		fmt.Printf("Idx: %d Value: %s kind: %s\n", i, valueOf.String(), kind)
	}
}

Idx: 0 Value: <int Value> kind: int
Idx: 1 Value: str kind: string
Idx: 2 Value: <float64 Value> kind: float64
Idx: 3 Value: <*struct {} Value> kind: ptr
Idx: 4 Value: <struct {} Value> kind: struct
Idx: 5 Value: <invalid Value> kind: invalid

So far - this is aligned with my expectations.

However, if I would have been given a reflect.Value of that same slice, and then have iterated it via reflect.Value.Index,
the type of each returned element is set to the element type from the slice header - (e.g. interface{}), and its concrete type and kind are 'masked'.

// https://play.golang.org/p/jeWXy7P2t6c

func main() {
	values := []interface{}{1, "str", 3.0, &struct{}{}, struct{}{}, nil}

	reflectedValues := reflect.ValueOf(values)
	for i := 0; i < reflectedValues.Len(); i++ {
		value := reflectedValues.Index(i)
		kind := value.Kind()

		fmt.Printf("Idx: %d Value: %s kind: %s\n", i, value.String(), kind)
	}
}

Idx: 0 Value: <interface {} Value> kind: interface
Idx: 1 Value: <interface {} Value> kind: interface
Idx: 2 Value: <interface {} Value> kind: interface
Idx: 3 Value: <interface {} Value> kind: interface
Idx: 4 Value: <interface {} Value> kind: interface
Idx: 5 Value: <interface {} Value> kind: interface

The above is solved by a simple .Elem() on the returned reflect.Value.

This 'dereferncing', however, is not required when operating in the same manner, but on a slice of concrete types:

// https://play.golang.org/p/eqpqpvdorqn

func main() {
	values := []int{1, 2, 3}

	reflectedValues := reflect.ValueOf(values)
	for i := 0; i < reflectedValues.Len(); i++ {
		value := reflectedValues.Index(i)
		kind := value.Kind()

		fmt.Printf("Idx: %d Value: %s kind: %s\n", i, value.String(), kind)
	}
}

Idx: 0 Value: <int Value> kind: int
Idx: 1 Value: <int Value> kind: int
Idx: 2 Value: <int Value> kind: int

What did you expect to see?

Given that:

a. Value.Index(...) says that Index returns the i'th element (and not a pointer / wrapper to it),
b. There's an inconsistent behavior between using reflect.ValueOf on each element, and using reflect.Value.Index on the []interface{}, and
c. The type of reflect.Value.Index(..), when ran on, say, []int - returns a Value with Kind == Int, and does not require the .Elem() 'dereferencing'

I think that the above is proving to be quite confusing for the user, and that it might have been better to:

  1. Have Value.Index document this behavior, or
  2. When calling Value.Index on a slice of interfaces, return a Value with the actual type of the element, instead of the type inferred by the slice-header.

To put it in different words, I expect that iterating with reflect.Value.Index, and iterating with reflect.ValueOf on each element, would prove to have a consistent behavior, no matter the type passed in.

What did you see instead?

The resulted types are instead 'masked', and need to be unwrapped via .Elem(), but only when operating on slices of interfaces.

@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented May 15, 2020

you are doing 2 different things

  1. ranging over a slice, you get the individual element which is then passed to reflect, it does not retain any info that it was part of a []interface{} so you get the value directly
  2. indexing the slice, the slice element is a known to be a interface{} (because you passed reflect a []interface{}) and you have to work through the type system and dereference it

this is not limited to reflect

@dany74q
Copy link
Author

@dany74q dany74q commented May 15, 2020

Hm - very interesting, thanks for the prompt response !

Would you not agree, though, that when the documented behavior is: "Index returns the i'th element of v",
One would expect that the resulted value would be equivalent to reflect.ValueOf(v[i]) ?

If not, could you point to a use case that breaks, if Index had returned such equivalence ?

Thanks again for your time - much appreciated.

@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented May 15, 2020

i expect the result to be the same as :

values := []interface{}{1, "str", 3.0, &struct{}{}, struct{}{}, nil}
values[i]

which is an interface{}

example that breaks: you want to set the i'th element to something else, if the type didn't match, with your proposal it wouldn't work

@dany74q
Copy link
Author

@dany74q dany74q commented May 15, 2020

Yup, that did it.

Thanks again !

@dany74q dany74q closed this May 15, 2020
@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented May 15, 2020

for the record, please refer to the forums for Questions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.