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

Improve error message for == of two different types #27227

Closed
wants to merge 4 commits into from
Closed

Conversation

ghost
Copy link

@ghost ghost commented Sep 19, 2018

Fixes #26592

@iliashkolyar
Copy link
Contributor

@andy-ms
If i'm not mistaken, this fix will not handle literal type comparisons such as:

var a = (1 == "1");
var b = ("" == 0);
var c = (false == "");
var d = (false == 0);

All of the above are also runtime comparisons that will yield "This condition will always return 'false'" mistakenly.

@ghost
Copy link
Author

ghost commented Oct 22, 2018

@DanielRosenwasser Please review

@sandersn sandersn added this to Curio in Pall Mall Jan 28, 2020
@sandersn sandersn added the For Backlog Bug PRs that fix a backlog bug label Feb 1, 2020
@sandersn sandersn added this to Needs review in PR Backlog Mar 5, 2020
@sandersn sandersn removed this from Curio in Pall Mall Mar 5, 2020
@elibarzilay
Copy link
Contributor

elibarzilay commented Jul 21, 2020

I rebased it up the history in small steps, and it worked fine with obvious fixes, until 48fc6b8 (from #32239) which edits the same code to something different, so making this work will require actually redoing it. (It looks like the main issue is that 48fc6b8 extends tryGiveBetterPrimaryError whereas this one removes it.)

@elibarzilay
Copy link
Contributor

Talked to @RyanCavanaugh, seems not worth the effort of reviving.

PR Backlog automation moved this from Needs review to Done Jul 21, 2020
@RyanCavanaugh RyanCavanaugh deleted the equals_error branch October 25, 2022 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
PR Backlog
  
Done
Development

Successfully merging this pull request may close these issues.

"This condition will always return 'false'" but returns true at runtime
4 participants