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

[WIP] Suppress the strict comparison warning when comparing .. with a literal #1037

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

pepkin88
Copy link
Contributor

@pepkin88 pepkin88 commented Mar 29, 2018

Currently, a code like

1
  .. == 1
  1 == ..

will produce misleading warnings upon compilation:

WARNING: strict comparison of two different types will always be false: .. == 1
WARNING: strict comparison of two different types will always be false: 1 == ..

It does that because .. is interpreted as a callable literal, probably for compiler's simplicity. That's why I added a simple exception to the conditions when to show the warning. I also have checked for other symbols requiring attention, but I think it's only .. (I checked Literal::compile's switch cases and the Literal class in general).

I don't know how to test for warnings, to I left the commit without tests.

@vendethiel
Copy link
Contributor

Most probably .. shouldn’t be a literal...

@vendethiel
Copy link
Contributor

vendethiel commented Mar 29, 2018

Would you mind going the extra mile, and instead not represent .. as a Literal?
There shouldn't be that much code to change (those are 3 links)

(I'm lying... There's more than this that relies on it being a Literal..)

@pepkin88
Copy link
Contributor Author

I'll look into that.
For now, I'll leave it there. There's no rush for that fix :)

@rhendric rhendric changed the title Suppress the strict comparison warning when comparing .. with a literal [WIP] Suppress the strict comparison warning when comparing .. with a literal Apr 5, 2018
@vendethiel
Copy link
Contributor

FWIW the issue is also visible with 1 == this.

@rhendric
Copy link
Collaborator

I suppose this ought not to be a Literal either?

@vendethiel
Copy link
Contributor

Would fix a few other issues, yes.

@rhendric
Copy link
Collaborator

Oh, neat. Which ones did you have in mind?

@vendethiel
Copy link
Contributor

~> `this`; too I think.

@rhendric
Copy link
Collaborator

(Assuming you meant double-backticks there, and GitHub ate them; if not, I'm very confused!)

Hmm, I don't see it. That this will end up inside a JS node, regardless of what kind of node a bare this is. To make that behave the same as ~> this, I think you'd have to change Fun::uses-this to parse the contents of JS children to see if they contain this. (Which doesn't sound worth doing to me, TBH.)

@vendethiel
Copy link
Contributor

Oops, that's on me, I totally forgot JS solved those issues already. (and yes, I did not miss infix this)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants