-
Notifications
You must be signed in to change notification settings - Fork 364
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
Equality is incorrect when nil
is used as the left argument of ==
#1500
Conversation
adding
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! |
ok, so these two statements 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 |
ok, maybe third time is the charm i added 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. |
questions:
|
nil
is used as the left argument of ==
nil
is used as the left argument of ==
nil
is used as the left argument of ==
#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 :) |
nil
is used as the left argument of ==
nil
is used as the left argument of ==
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
awesome. i think i changed those things. let me know if there is anything else that needs to change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
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