Equality is incorrect when nil is used as the left argument of ==

[elee1766]

hi!

this issue is sorta blocking for me so i thought i would try to fix it.

im still learning the codebase and understanding how yaegi works, but I thought I would attempt to add a test in the style of other tests as a start.

please let me know if there is anything i need to change / run, or if anyone knows perhaps a good place to start for tackling this issue

Fixes #1496

[CLAassistant]

All committers have signed the CLA.

[elee1766]

adding
fmt.Printf("equals: <%s> <%s> %v \n", t.id(), o.id(), t.id() == o.id())
to line type.go:1504 seems to show

equals:  <[]uint8> <nil> false
equals:  <bool> <bool> true
equals:  <nil> <[]uint8> false
equals:  <bool> <bool> true

which are the same, which perhaps means something is happening later down the line? will keep digging

oh, it seems that's just called by typecheck.go, and yeah those things are matching, so it must be down the line. sorry for the red herring!

[elee1766]

ok, so these two statements []byte{} == nil and nil == []byte{} seem to be getting optimized somewhere before interpretation?

i say this because the "equals" in "op.go" is not called for those operations (they are for a==false, b==false, and a==b though), but maybe i misunderstand

[elee1766]

ok, maybe third time is the charm

i added fmt.Printf(`nodes <%s> <%s> %v\n`, c0.name(), n.child[1].name(), value(f).IsNil())

to the frame function near run.go:3880. it seems that the difference materializes here.

so this is consistent with the behavior. It just checks if the first? value is nil, and if it is, returns true? hence why (nil==[]byte{}) returns true

but that means that uh, nil equality is completely broken broken then right?

ok

i updated the test just now, it seems that if nil is the first element in a comparison, the expression always evaluates to true.

[elee1766]

questions:

1. i dont actually know if this check is 100% correct? i think it is more correct than the previous one. it matches go run now at least.

2. i dont know if "nil" is valid in any binary operator that isnt aEqual. if its just aEqual, this fix should be fine?

3. i couldn't run tests on my local computer, im not sure why, maybe i just cant read

[elee1766]

#1496 is this how github works trying to tag the issue to this pr

im so sorry if someone is subscribed to this issue and im spamming your in box happy new year :)

[mvertes]

Thank you for this nice first submission. The fix you propose is globally correct, just a few details to change before we can merge this.

Thanks again.

[elee1766]

awesome. i think i changed those things. let me know if there is anything else that needs to change.

[mvertes]

Looks good to me.