Go 1.16 will change behavior of what appears to be a buggy conditional

[ianlancetaylor]

This line of code looks buggy to me:

https://github.com/go-openapi/validate/blob/master/type.go#L140

The troublesome code is:

reflect.DeepEqual(reflect.Zero(reflect.TypeOf(data)), reflect.ValueOf(data))

The function reflect.DeepEqual takes two different values of any type and returns whether they are "deep equal". It does not ordinarily take two different values of the type reflect.Value, because it is not normally interesting whether two different reflect.Value values are equal. What is interesting is whether the values stored in the reflect.Value values are equal.

Before Go 1.16, a call to reflect.Zero would internally store a pointer to a newly allocated zero value. Thus before Go 1.16 this expression would always return false (at least, I can't think of any cases where it would return true). In Go 1.16 that was changed by https://golang.org/cl/192331: as an optimization, reflect.Zero will now use the same pointer for all types smaller than 1024 bytes. The effect is that now if data was itself obtained by a call to reflect.Zero, the expression will now return true.

It appears that the code is trying to do something like

reflect.DeepEqual(reflect.Zero(reflect.TypeOf(data)).Interface(), data)

which as of Go 1.13 is more efficiently written as

reflect.ValueOf(data).IsZero()

I don't know what the right fix is here. Changing this code has various knock-on effects on other packages that use this one. Is an empty string really the same as a null value? I don't know.