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

Comparison operators have higher precedence than range operator `..` #22877

Open
insaneinside opened this Issue Feb 27, 2015 · 8 comments

Comments

Projects
None yet
10 participants
@insaneinside
Copy link
Contributor

insaneinside commented Feb 27, 2015

This has the effect of causing parse errors and other weirdness when trying to use a range literal in a comparison.

1..2 == 1..2

gives us

<input>:1:10: 1:12 error: expected one of `.`, `;`, `<eof>`, or an operator, found `..`
<input>:1 1..2 == 1..2

Huh? Okay, what about this?

1..2 == 1
<anon>:13:20: 13:31 error: start and end of range have incompatible types: expected `_`, found `bool` (expected integral variable, found bool) [E0308]
<anon>:13 println!("{:?}", { 1..2 == 1 });
                             ^~~~~~~~~~~

So it looks as though the comparison is being reduced before the range.

More fun:

1..2 < (3..4)
<anon>:13:27: 13:35 error: mismatched types:
expected `_`,
   found `core::ops::Range<_>`
(expected integral variable,
   found struct `core::ops::Range`) [E0308]
<anon>:13 println!("{:?}", { 1..2 < (3..4) });
                                   ^~~~~~~~

Never mind the fact that ranges don't implement (Partial)Ord -- the precedence is still wrong here, too. Placing the parentheses on the first range instead of the second properly tells us

<anon>:13:20: 13:30 error: binary operation `<` cannot be applied to type `core::ops::Range<_>`
<anon>:13 println!("{:?}", { (1..2) < 3..4 });
                            ^~~~~~~~~~

(although i'd argue that it should also complain about mismatched types here, as well. ;)

@insaneinside

This comment has been minimized.

Copy link
Contributor Author

insaneinside commented Feb 27, 2015

Pull request #21374 (merged) includes several other relevant xrefs.

@kmcallister kmcallister added the A-parser label Mar 1, 2015

@huonw huonw added I-nominated T-libs T-lang and removed T-libs labels Jan 8, 2016

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jan 14, 2016

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jan 14, 2016

If someone shows an example of something that (1.) currently parses but also that (2.) intuition dictates shouldn't parse, then the lang team would be more likely to be inspired to assign this a higher priority, due to backwards compatibility concerns.

As it is right now, if the main problems are cases where things do not parse that one might like to parse, then that is a potentially interesting problem to tackle, but it is not quite as high priority as resolving one of the many potentially backwards-incompatible issues that we have in front of us.

@pnkfelix pnkfelix removed the I-nominated label Jan 14, 2016

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jan 14, 2016

triage: P-medium

@insaneinside

This comment has been minimized.

Copy link
Contributor Author

insaneinside commented Jan 14, 2016

What do we define as "parses" here? The parser "parses" all of the given examples --- but each one as a different operation!

@dgrunwald

This comment has been minimized.

Copy link
Contributor

dgrunwald commented Jan 14, 2016

I originally tried to give .. a higher precedence than the comparison operators, but that made it hard to have the same precedence level for both the prefix- and postfix- forms of unary ...

Giving .. a higher precedence than the comparison operators also causes a grammar ambiguity for r == 1.. && true. This currently parses as (r == 1) .. (&&true). Just changing the precedence of .. would cause it to parse as r == (1..(&&true)), but it would be more useful as r == (1..) && true (and more consistent with true && r == 1..).

See #20811 for the original issue about the precedence of ... #20958 was the first (not accepted) PR where I gave .. precedence above the comparison operators; #21374 is the accepted PR where I kept the low precedence.

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jan 14, 2016

@insaneinside from a backward compatibility standpoint, the issue is what might accepted by the end-to-end compile.

But you make a good point: it might be possible that by implementing the overloaded operators in a particular way, one might be able to construct an example that gets through the compiler with an unexpected parse today.


The parser "parses" all of the given examples --- but each one as a different operation!

Update: also, to be pedantic, the first example didn't parse, right? But this others certainly did.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Sep 24, 2018

Triage: no changes. Is this even fixable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.