reflect: DeepEqual should ignore non nil functions

[gopherbot]

by machielg:

When comparing two structs with non-nil function pointer, reflect.DeepEqual will return
false. This is of course incorrect because the structs might be equal. In the ideal case
it would actually compare the functions, but that's not feasible. 

Returning 'false' is impractical because DeepEqual will now always give me false for my
structs. It would be more practical to just ignore the functions in the comparison. That
way DeepEqual will be informative (everything except functions were equal or not) and
the user can decide how to deal with function equality.

[dsymonds]

Comment 1:

I doubt it'll change, since existing code might be relying on this. The docs for
DeepEqual explicitly say "Functions are equal only if they are both nil.".

Labels changed: added repo-main.

[gopherbot]

Comment 2 by machielg:

Of course changing existing behaviour could be a problem for existing users. There are
always few options though:
1. Change the behaviour because the the existing behaviour is not correct/wanted.
2. Introduce a new function that has deals differently with structs with function fields
(e.g. my proposal to ignore the functions)
3. Warn users that behaviour will change in the future and do 1. at a later stage.
My argument is that when dealing with function pointers the DeepEqual is useful is so
few cases a change is warranted. Anyone relying on current behaviour will have some
fuzziness to deal with, since 'false' can mean either actually not equal or some
function was not nil. My guess is that anyone who uses functions as struct properties
avoids DeepEqual.
I currently have to copy over the DeepEqual source code including all the private
functions in order to get useful behaviour.

[ianlancetaylor]

Comment 3:

Since the language doesn't permit comparing function values other than nil, DeepEqual
has to make a choice.  There is no choice that will be correct for all use cases.  I'm
sorry that the current choice doesn't work for you, but changing it will give us
something that won't work for other people.  Since the current DeepEqual code is not
wrong, I can't see a reason to change it.

Status changed to WorkingAsIntended.

[gopherbot]

Comment 4 by machielg:

My option for adding a function that leaves the functions alone seems to be ignored. The
current DeepEqual is essentially wrong, it gives an answer when in fact it doesn't know.
Someone decided that returning 'false' was a practical solution, but it's not practical,
it just makes the function useless when dealing with function pointers. There is no
thorough reasoning behind the 'false' return value that I can find, in the original
issue it was just proposed by someone and then implemented without any comments.
I don't see where the resistance is coming from not to think about a solution for this
problem. Basically writing unit tests for Structs with functions is seriously
impractical in Go, requiring me to copy large parts of the runtime. Doesn't that
indicate a missed feature? The solution(s) I proposed are convenient and won't hinder
any existing implementations.

[ianlancetaylor]

Comment 5:

I don't understand what it means to "leave the functions alone."  If I write
    func f() bool { return reflect.DeepEqual(f, f) }
the function has to return something.
There is nothing magic about reflect.DeepEqual.  It's just a library function.  It has
to make some specific choices, and those choices have been made.  If you don't like the
choices that it makes, then write your own version.  You describe that as copying large
parts of the runtime, but it's not.  It's about 140 lines of Go code, including
comments.  Writing your own version means writing an equality function that serves your
specific needs, something that is done routinely for cases where reflect.DeepEqual is
insufficient or incorrect.
You describe not supporting your particular use case as a "missed feature."  You're
right: it is a missed feature.  There are, in that sense, many missing features in Go. 
What's important for Go libraries is not providing every feature, but providing all the
tools you need to make your program work.

[gopherbot]

Comment 6 by machielg:

Thanks for the extra comment, I wasn't talking about DeepEqual for comparing two
functions, just comparing structs that contain functions. 
I understand your idea of not changing the language and/or not adding every feature
imaginable. I can indeed implement my own function and I will.
Thanks again.