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

False positive on collapsible_if with if let #1197

Closed
llogiq opened this issue Aug 27, 2016 · 3 comments
Closed

False positive on collapsible_if with if let #1197

llogiq opened this issue Aug 27, 2016 · 3 comments

Comments

@llogiq
Copy link
Contributor

llogiq commented Aug 27, 2016

This is from linefeed:

fn read_init(&mut self) {
    if let Some(path) = env_init_file() {
        // If `INPUTRC` is present, even if invalid, parse nothing else.
        // Thus, an empty `INPUTRC` will inhibit loading configuration.
        self.read_init_file_if_exists(Some(path));
    } else {
        if !self.read_init_file_if_exists(user_init_file()) {
            self.read_init_file_if_exists(system_init_file());
        }
    }
}

If you try to collapse this, rustc won't have it:

error[E0301]: cannot mutably borrow in a pattern guard
    --> src/reader.rs:2116:20
     |
2116 |         } else if !self.read_init_file_if_exists(user_init_file()) {
     |                    ^^^^ borrowed mutably in pattern guard

error: aborting due to previous error

The difference appears to be that the else if part of an if let desugars to a guard. And guards appear to have different borrowing rules than simple if expressions (I'm not sure why). Perhaps that is a bug in borrowck, but as long as it is there, we should at least note it in the docs.

@mcarton
Copy link
Member

mcarton commented Aug 27, 2016

Commits that seems to add if let desugaring: rust-lang/rust@0e6ff43#diff-7fde1475846894e59317d75947bdb921R72.

Desugar ExprIfLet
From: `if let <pat> = <expr> <body> [<elseopt>]`
to:

  match <expr> {
    <pat> => <body>,
    [_ if <elseopt_if_cond> => <elseopt_if_body>,]
    _ => [<elseopt> | ()]
  }

There isn't much explanation about why this is done that way. But it certainly seems surprising because it leads to very unintuitive errors. I think we should report that on rustc's issue tracker.
See also rust-lang/rust#28449.

@Arnavion
Copy link
Contributor

rust-lang/rust#28449 is closed. rust-lang/rust#37510 is another issue for it that's still open.

@camsteffen
Copy link
Contributor

This must be fixed by now, especially with NLL.

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

No branches or pull requests

4 participants