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

Returning a comparison expression with a match fails depending on the operand order #47287

Closed
NPN opened this issue Jan 9, 2018 · 4 comments · Fixed by #60188
Closed

Returning a comparison expression with a match fails depending on the operand order #47287

NPN opened this issue Jan 9, 2018 · 4 comments · Fixed by #60188
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@NPN
Copy link

NPN commented Jan 9, 2018

Suppose the return expression of a function is a comparison between a match expression and some other operand. If the operand and comparison operator come before the match expression, the code compiles. However, if the operand and comparison operator come after the match expression, the code fails to compile.

fn works(x: u32) -> bool {
    0 < match x {
        _ => 1,
    }
}

fn fails(x: u32) -> bool {
    match x {
        _ => 1,
    } > 0
}

The error is thus:

error: expected expression, found `>`
  --> src/main.rs:10:7
   |
10 |     } > 0
   |       ^

error[E0308]: match arms have incompatible types
  --> src/main.rs:8:5
   |
8  | /     match x {
9  | |         _ => 1,
10 | |     } > 0
   | |_____^ expected bool, found integral variable
   |
   = note: expected type `bool`
              found type `{integer}`
note: match arm with an incompatible type
  --> src/main.rs:9:14
   |
9  |         _ => 1,
   |              ^

Strangely, the code does work if the comparison is a statement:

fn wontfail(x: u32) -> bool {
    let y = match x {
        _ => 1,
    } > 0;
    y
}

Tested using rustc 1.23.0 on Playground.

@hanna-kruppe
Copy link
Contributor

This is known and intentional IIRC, but I can't remember what it's called or where it's documented. It affects every expression that ends in braces, e.g., { 1 } > 0 and if ... { 1 } else { 1 } > 0.

@petrochenkov
Copy link
Contributor

It affects every expression that ends in braces

Every expression statement that ends in braces

    true && { 1 } > 0; // OK, { 1 } is an intermediate expression somewhere
    { 1 } > 0; // FAIL, expression { 1 } is a full statement, we expect a next statement after it

@NPN
Copy link
Author

NPN commented Jan 10, 2018

So this is not a bug, but an intentional limitation made for better/easier parsing or semantics? I suppose that makes sense, even if it wasn't what I expected. I had assumed that since if and match statements can be assigned to variables (i.e. they are rvalues), that meant that statements could be used anywhere a "normal" value (e.g. u32, bool) could be used.

I wish there was an explanation of why this is the case, though. The reference section on expression statements doesn't say anything about operators not being allowed to appear after a full statement (though maybe that's because the name implies that it is a complete statement, and thus nothing can follow it).

I would close this issue, but before I do that, perhaps the error message could be improved? I have a few ideas, though take them with a grain of salt, because I don't know what the compiler/parser actually can and can't do.

  1. The first error expected expression, found `>` correctly explains the problem, but the following errors give the impression that something is wrong with the match statement, and not the comparison expression. If the compiler just stopped at the first error, it would be less confusing.
  2. The compiler could give a suggestion to reorder the expression if it detects such a syntax error. (This is probably impossible, though.)
  3. The error could be given a code like [EXXXX]. The error section of the Rust documentation would then explain that if you get this error, you should try switching the expression and operand. That would make the error less confusing, as the current error message doesn't explain how to fix the problem.

@petrochenkov
Copy link
Contributor

@NPN
The explanation is whether we continue parsing operators after an expression statements or not, we'll get wrong results sometimes. This code, for example:

if cond {
    stmts;
}

*my_ref = value;

or this

if cond {
    stmts;
}

(a + b).method();

You don't want these example to be parsed as multiplication and function call, respectively - (if cond {}) * my_ref and (if cond {})(a + b).
So, have to make a choice - to continue parsing and get errors sometimes and to stop parsing and get errors sometimes. I don't know when this choice was made exactly, but the choice was "stop parsing".

It's certainly possible to improve diagnostics and this is probably the first thing that needs to be done.

I think it's also possible to "cherry-pick" some operators that can continue an expression, but can't start an expression and continue parsing only when encountering them, but that's somewhat risky and needs to be though out carefully - what if we'll need to extend the grammar and add unary + operator, for example.

@gsollazzo gsollazzo added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 1, 2018
estebank added a commit to estebank/rust that referenced this issue Apr 29, 2019
Centril added a commit to Centril/rust that referenced this issue May 9, 2019
Identify when a stmt could have been parsed as an expr

There are some expressions that can be parsed as a statement without
a trailing semicolon depending on the context, which can lead to
confusing errors due to the same looking code being accepted in some
places and not others. Identify these cases and suggest enclosing in
parenthesis making the parse non-ambiguous without changing the
accepted grammar.

Fix rust-lang#54186, cc rust-lang#54482, fix rust-lang#59975, fix rust-lang#47287.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants