Unintuitive error message for missing conditional in if-else expression

[LemmingAvalanche]

I think that this error message is unintuitive. I would have expected something more along the lines of " line 38 error: expected conditional but found '{' ".

I guess what happens is that the parser interprets the if-body as the condtional expression; the program compiles fine if I write the conditional expression inside its own curly braces, which is then followed by the same if-body expression.

Compiler used:

rustc 0.11-pre-nightly (540c2a2 2014-04-04 01:01:51 -0700)
host: x86_64-unknown-linux-gnu

Code:

use std::io::println;

fn is_three(num: int) -> bool {
    num % 3 == 0
}

fn is_five(num: int) -> bool {
    num % 5 == 0
}

fn is_fftn(num: int) -> bool {
    num % 15 == 0
}

#[test]
fn test_is_three_with_not_three() {
  if is_three(1) {
    fail!("One is not three");
  }
}

#[test]
fn test_is_three() {
    if is_three(1) {
        fail!("One is not three");
    }
}

fn main() {
  for num in range(1,101) {
    let answer =
      if is_fftn(num) {
        "FizzBuzz"
      }
      else if is_three(num) {
        "Fizz"
      }
      else if { //ERROR: MISSING CONDITIONAL
        "Buzz"
      }
      else {
        ""
      };
    println(answer);
  }
}

Compiler output:

~/Dropbox/rust$ rustc fizzbuzz.rs
fizzbuzz.rs:41:7: 41:11 error: expected `{` but found `else`
fizzbuzz.rs:41       else {
                     ^~~~

(the missing conditional error is at line 38)

[lifthrasiir]

This is because:

if is_fftn(num) {
    "FizzBuzz"
} else if is_three(num) {
    "Fizz"
} else if {
    true
} {
    "Buzz"
}

...would be the valid code (though does not make much sense). Since else is already a special case in the grammar, I guess it is possible to special-case the unexpected else anyway.

[steveklabnik]

Minimal reproduction:

fn main() {
    if true {
    }
    else if { //ERROR: MISSING CONDITIONAL
    }
    else {
    };
}

Same error. I'm not sure if we can do better here, but maybe?

[pmarcelll]

The minimal reproduction now gives (on nightly):

error: expected `{`, found `else`
 --> <anon>:6:5
  |
6 |     else {
  |     ^^^^

error: aborting due to 3 previous errors

It sais 3 previous errors, although only one is presented (beta is fine).

[DikshaGodbole]

Hi, I am attempting the solution for this bug.Thankyou!.

[nikomatsakis]

Hi @DikshaGodbole. So, I'm not sure how much you know about parsers, but that's the region of code you'll want to change to make progress here. The parser is basically the code that takes in input text as a string (e.g., "if { ... }") and turns it into the AST (the compiler's internal data structures to represent the Rust program).

In this case, we'll be mucking about in the piece of code that parses an if expression. When this code is invoked, we've already recognized the word "if" in the input. So the next thing we expect to see if an expression. That is, when everything goes right, Rust syntax looks like:

    if condition.is_true() { 
//  ^^ ^^^^^^^^^^^^^^^^^^^ ^
//  1  2                   3
//
// 1 -- we saw this part
// 2 -- this is the expression we expect to parse next
// 3 -- then we expect to see a `{`
}

Parsing the expression is done by this function call here. If all goes right, after the expression, we expect to see a { (as indicated by the 3 above). In fact, what we expect to see is a block of code, like the one I show here:

    if condition.is_true() // <-- we've parsed this far so far
    { // <-- this is what we will start parsing next...
        println!("Hello, world!");
    } // <-- once `parse_block()` returns, we'll have parsed this far

To parse this, we call self.parse_block(). If all goes right, we then will go on to look for an else. I.e., there might be something like this:

    if condition.is_true() { 
        println!("Hello, world!");
    } // <-- we've parsed this far by now
    else // <-- we're going to parse this `else` next
    { ... }

We look for that by calling eat_keyword(Else) which checks if the next thing is an Else and returns true if so.

Anyway, the problem is that in the user's case, they forgot the expression, so they have something like this (here I commented out the expression that is supposed to be there; this should have the same effect, but it makes the error easier to see):

    if /* condition.is_true() */ { 
        println("Hello, world!");
    } else {
        println("Else block");
    }

So, after we've seen the if, we're going to go look for this expression that's not there -- and the next thing we see is a {. And here is where things get confusing! The problem is that, in Rust, a block like { foo(); bar(); } is also an expression. So we wind up parsing the body of the if and thinking it's the if expression! That leaves us in this state:

    if /* condition.is_true() */ { 
        println("Hello, world!");
    } // <-- parsed up to here, and now we are ready to find the `{`
    else {
        println("Else block");
    }

Now that we have parsed what we think is the expression, we are ready to parse the body of the if, which should be a block that starts with {. But instead of a {, we see the else, and you get the error that we report today.

To fix this, we can insert a check that looks at the next token in the input stream right before we call parse_block(). So we want to do something roughly like this:

 pub fn parse_if_expr(&mut self, attrs: ThinVec<Attribute>) -> PResult<'a, P<Expr>> {
        // XXX I am adding this:
        let if_span = self.prev_span;

        if self.check_keyword(keywords::Let) {
            return self.parse_if_let_expr(attrs);
        }
        let lo = self.prev_span.lo;
        let cond = self.parse_expr_res(Restrictions::RESTRICTION_NO_STRUCT_LITERAL, None)?;

        // XXX adding this code too:
        if self.token.is_keyword(keywords::Else) {
            // Try to give a nice error for the case where user forgot an expression.
            // See #13483 for details.
            return Err(self.span_fatal(if_span, &format!("`if` without any condition expression")));
        }

        let thn = self.parse_block()?;
        /* ... */        
}

Here is what's going on here:

• in the first bit of code I added, the self.last_span contains the span (that is, the location in the file, like a line/column number) of the thing we looked at last, which is the keyword if.
• in the second bit, if -- when we expect to see a {, we see the keyword else -- then we report a custom error, using the location of the keyword if.

OK, that's roughly the code we need to add (I haven't tried that code or anything, but it should be close-ish). Then the next thing you have to do is add a test case. There is some documentation on how our test suite works here and some more documentation here but in short you will want to create a file with a name like src/test/ui/suggest-missing-condition-issue-13483.rs and put in the test case from this comment. Then you can run ./x.py --incremental test src/test/ui to run the tests (with your new code).

Anyway, that's enough for now, let me know how you make out! :)

[DikshaGodbole]

Hi, Thank you so much .I will get started as soon as I finish my submissions for this semester that is on 4 April . Thanking You, Diksha Godbole

…

On Tue, Mar 28, 2017 at 3:19 AM, Niko Matsakis ***@***.***> wrote: Hi @DikshaGodbole <https://github.com/DikshaGodbole>. So, I'm not sure how much you know about parsers, but that's the region of code you'll want to change to make progress here. The parser is basically the code that takes in input text as a string (e.g., "if { ... }") and turns it into the AST (the compiler's internal data structures to represent the Rust program). In this case, we'll be mucking about in the piece of code that parses an if expression <https://github.com/rust-lang/rust/blob/769b95dc9f92edb51146727813ea7eae00b5b651/src/libsyntax/parse/parser.rs#L2924>. When this code is invoked, we've already recognized the word "if" in the input. So the next thing we expect to see if an expression. That is, when everything goes right, Rust syntax looks like: if condition.is_true() { // ^^ ^^^^^^^^^^^^^^^^^^^ ^// 1 2 3//// 1 -- we saw this part// 2 -- this is the expression we expect to parse next// 3 -- then we expect to see a `{` } Parsing the expression is done by this function call here <https://github.com/rust-lang/rust/blob/769b95dc9f92edb51146727813ea7eae00b5b651/src/libsyntax/parse/parser.rs#L2929>. If all goes right, after the expression, we expect to see a { (as indicated by the 3 above). In fact, what we expect to see is a block of code, like the one I show here: if condition.is_true() // <-- we've parsed this far so far { // <-- this is what we will start parsing next... println!("Hello, world!"); } // <-- once `parse_block()` returns, we'll have parsed this far To parse this, we call self.parse_block() <https://github.com/rust-lang/rust/blob/769b95dc9f92edb51146727813ea7eae00b5b651/src/libsyntax/parse/parser.rs#L2930>. If all goes right, we then will go on to look for an else. I.e., there might be something like this: if condition.is_true() { println!("Hello, world!"); } // <-- we've parsed this far by now else // <-- we're going to parse this `else` next { ... } We look for that by calling eat_keyword(Else) <https://github.com/rust-lang/rust/blob/769b95dc9f92edb51146727813ea7eae00b5b651/src/libsyntax/parse/parser.rs#L2933> which checks if the next thing is an Else and returns true if so. Anyway, the problem is that in the user's case, they forgot the expression, so they have something like this (here I commented out the expression that is *supposed* to be there; this should have the same effect, but it makes the error easier to see): if /* condition.is_true() */ { println("Hello, world!"); } else { println("Else block"); } So, after we've seen the if, we're going to go look for this expression that's not there -- and the next thing we see is a {. And here is where things get confusing! The problem is that, in Rust, a block like { foo(); bar(); } *is also an expression*. So we wind up parsing the body of the if and thinking it's the if expression! That leaves us in this state: if /* condition.is_true() */ { println("Hello, world!"); } // <-- parsed up to here, and now we are ready to find the `{` else { println("Else block"); } Now that we have parsed what we think is the expression, we are ready to parse the body of the if, which should be a block that starts with {. But instead of a {, we see the else, and you get the error that we report today. To fix this, we can insert a check that looks at the next token in the input stream right *before* we call parse_block() <https://github.com/rust-lang/rust/blob/769b95dc9f92edb51146727813ea7eae00b5b651/src/libsyntax/parse/parser.rs#L2930>. So we want to do something *roughly* like this: pub fn parse_if_expr(&mut self, attrs: ThinVec<Attribute>) -> PResult<'a, P<Expr>> { // XXX I am adding this: let if_span = self.prev_span; if self.check_keyword(keywords::Let) { return self.parse_if_let_expr(attrs); } let lo = self.prev_span.lo; let cond = self.parse_expr_res(Restrictions::RESTRICTION_NO_STRUCT_LITERAL, None)?; // XXX adding this code too: if self.token.is_keyword(keywords::Else) { // Try to give a nice error for the case where user forgot an expression. // See #13483 for details. return Err(self.span_fatal(if_span, &format!("`if` without any condition expression"))); } let thn = self.parse_block()?; /* ... */ } Here is what's going on here: - in the first bit of code I added, the self.last_span contains the *span* (that is, the location in the file, like a line/column number) of the thing we looked at last, which is the keyword if. - in the second bit, if -- when we expect to see a {, we see the keyword else -- then we report a custom error, using the location of the keyword if. OK, that's roughly the code we need to add (I haven't tried that code or anything, but it should be close-ish). Then the next thing you have to do is add a *test case*. There is some documentation on how our test suite works here <https://forge.rust-lang.org/test-suite.html> and some more documentation here <https://github.com/rust-lang/rust/blob/769b95dc9f92edb51146727813ea7eae00b5b651/src/test/ui/README.md> but in short you will want to create a file with a name like src/test/ui/ suggest-missing-condition-issue-13483.rs and put in the test case from this comment <#13483 (comment)>. Then you can run ./x.py --incremental test src/test/ui to run the tests (with your new code). Anyway, that's enough for now, let me know how you make out! :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13483 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AZHwt6mefZLkfNVA5hMsRh28FPfZY5Qaks5rqC7hgaJpZM4Bx8wQ> .

-- *Diksha Godbole*