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

Allow pattern matching after guard clause #45978

Closed
danielpclark opened this Issue Nov 14, 2017 · 8 comments

Comments

Projects
None yet
5 participants
@danielpclark
Copy link

danielpclark commented Nov 14, 2017

I've opened a separate issue for a Rust style guide on Guard Clause. What this issue is about is that Rust currently doesn't recognize when a path has already been handled by a Guard Clause.

I want to do the following:

use std::fs::File;
let file = File::open("program.cfg");

if file.is_err() { return Err(Error::ConfigLoadFail); }
let Ok(f) = file;

let Ok(f) = file is a nice usage of pattern matching. But Rust complains as it doesn't know I've already accounted for the error path:

error[E0005]: refutable pattern in local binding: `Err(_)` not covered
   --> src/lib.rs:166:7
    |
166 |   let Ok(f) = file;
    |       ^^^^^ pattern `Err(_)` not covered

error: aborting due to previous error

I really look forward to using pattern matching with guard clauses rather than using unwrap() which I find a bit of an eye sore.

I love Rust, keep up the good work ^_^

@kennytm

This comment has been minimized.

Copy link
Member

kennytm commented Nov 14, 2017

You could write this as:

let f = match file {
    Ok(f) => f,
    Err(_) => return Err(Error::ConfigLoadFail),
};

or, since file is a Result, not instance of an arbitrary type:

let f = file.map_err(|_| Error::ConfigLoadFail)?;

If we supported rust-lang/rfcs#1303 (let ... else) this could be written as

let Ok(f) = file else {
    return Err(Error::ConfigLoadFail);
};

I think the compiler should never support the original code by OP as this means we need to inspect the implementation of is_err to determine it will always return false on the Ok branch to determine the let Ok(f) = ... is irrefutable.

I'm not sure if we should allow:

if let Err(_) = file {
   return Err(Error::ConfigLoadFail);
}
let Ok(f) = file;

this looks reasonable, but should require a proper RFC to clarify the control flow interaction.

@danielpclark

This comment has been minimized.

Copy link
Author

danielpclark commented Nov 14, 2017

I'm not sure how the internals for Rust checking paths are implemented, but I'm suggesting this more as a relative Guard Clause thing rather than a Result thing.

In the example above the is a conditional check on file with a immediate exit with return in the code block. The very next item within the same scope is the same item file being pattern matched for assignment. So this looks like it wouldn't be too hard to implement.

What I mean by not strictly a Result thing is, for example, imagine if File.open returned an Option instead. Then my recommendation would look like:

if file.is_none() { return Err(Error::ConfigLoadFail); }
let Some(f) = file;

I do love the map_err example you provided. I didn't know about that. I think my feature request would be more readable and beginner friendly rather than using map_err to achieve the same result.

@kennytm

This comment has been minimized.

Copy link
Member

kennytm commented Nov 14, 2017

What I mean is that the compiler should not be allowed to peek into the definition of is_err() or is_none() to know that let Ok(f) = or let Some(f) = is acceptable, i.e.

if file.is_none() { return; }
let Some(f) = file;
// Unacceptable, the signature `fn is_none(&self) -> bool` tells nothing 
// about the typestate of `file`

but

if let None = file { return; }
let Some(f) = file;
// Acceptable, we know that `Option` can only be 
// `None` or `Some(_)`, and the `None` case will not reach here
// so we are sure that `file` can only be `Some(_)`
// and thus the pattern is irrefutable. 
@danielpclark

This comment has been minimized.

Copy link
Author

danielpclark commented Nov 14, 2017

if let None = file { return; }
let Some(f) = file;

Yes. I find that as very acceptable. Thank you.

@leodasvacas

This comment has been minimized.

Copy link
Contributor

leodasvacas commented Nov 14, 2017

Shuldn't language feature requests live in the rfcs repo?

@kennytm

This comment has been minimized.

Copy link
Member

kennytm commented Nov 14, 2017

@leodasvacas Yes, for major changes like this. Perhaps the discussion should continue in the Pre-RFC on the internals forum: https://internals.rust-lang.org/t/pre-rfc-allow-pattern-matching-after-guard-clause/6238

@durka

This comment has been minimized.

Copy link
Contributor

durka commented Nov 14, 2017

This seems like typestate which existed in Ancient History Rust.

@leodasvacas

This comment has been minimized.

Copy link
Contributor

leodasvacas commented Mar 2, 2018

Triage: Broad feature request, should be moved to the RFCs repo.

@Centril Centril closed this Feb 18, 2019

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.