rust-lang / rfcs

RFCs for changes to Rust
https://rust-lang.github.io/rfcs/
Apache License 2.0
5.95k stars 1.57k forks source link

Allow negation of `if let` #2616

Open mqudsi opened 5 years ago

mqudsi commented 5 years ago

The RFC for if let was accepted with the rationale that it lowers the boilerplate and improves ergonomics over the equivalent match statement in certain cases, and I wholeheartedly agree.

However, some cases still remain exceedingly awkward; an example of which is attempting to match the "class" of a record enum variant, e.g. given the following enum:

enum Foo {
    Bar(u32),
    Baz(u32),
    Qux
}

and an instance foo: Foo, with behavior predicated on foo not being Bar and a goal of minimizing nesting/brace creep (e.g. for purposes of an early return), the only choice is to type out something like this:

    // If we are not explicitly using Bar, just return now
    if let Foo::Bar(_) = self.integrity_policy {
    } else {
        return Ok(());
    }

    // flow resumes here

or the equally awkward empty match block:

    // If we are not explicitly using Bar, just return now
    match self.integrity_policy {
        Foo::Bar(_) => return Ok(());
        _ => {}
    }

    // flow resumes here

It would be great if this were allowed:

    // If we are not explicitly using Bar, just return now
    if !let Foo::Bar(_) = self.integrity_policy {
        return Ok(());
    }

    // flow resumes here

or perhaps a variation on that with slightly less accurate mathematical connotation but far clearer in its intent (you can't miss the ! this time):

    // If we are not explicitly using Bar, just return now
    if let Foo::Bar(_) != self.integrity_policy {
        return Ok(());
    }

    // flow resumes here

(although perhaps it is a better idea to tackle this from an entirely different perspective with the goal of greatly increasing overall ergonomics with some form of is operator, e.g. if self.integrity_policy is Foo::Bar ..., but that is certainly a much more contentious proposition.)

H2CO3 commented 5 years ago

or the equally awkward empty match block

I don't understand what's "awkward" in a block.

Furthermore, I don't see any good syntax for realizing this. Certainly, both proposed ones (if !let foo = … and if let foo != …) are much more awkward and a lot harder to read than a perfectly clear match. I would consider such code less ergonomic and less readable than the equivalent match expression.

burdges commented 5 years ago

We've seen numerous proposals for this, so maybe review them before attempting to push any serious discussion.

If the variants contain data then you normally require said data, but you cannot bind with a non-match, so is_[property] methods are frequently best at covering the actual station. If is_[property] methods do not provide a nice abstraction, then maybe all those empty match arms actually serve some purpose, but if not then you can always use a wild card:

let x = match self.integrity_policy {
    Foo::Bar(x) => x,
    _ => return Ok(()),
}

Also, one can always do very fancy comparisons like:

use std::mem::discriminant;
let d = discriminant(self.integrity_policy);
while d != discriminant(self.integrity_policy) {  // loop until our state machine changes state
    ...
}

I believe the only "missing" syntax around bindings is tightly related to refinement types, which require a major type system extension. In my opinion, we should first understand more what design by contract and formal verification might look like in Rust because formal verification is currently the major use case for refinement types. Assuming no problems, there is a lovely syntax in which some refine keyword works vaguely like a partial match. As an example, let y = x? would be equivalent to

let Ok(y) = refine x { Err(z) => return z.into(); }  

In this let Ok(y) = .. could type checks because refine returns a refinement type, specifically an enum variant type ala #2593. I suppose refinement is actually kinda the opposite of what you suggest here, but figured I'd mention it.

mqudsi commented 5 years ago

I looked around for past proposals regarding expanding the if let syntax, starting with the original if let rfc and issues referenced in the comments before opening this.

@burdges Thanks for sharing that info, I have some reading to do. But with regards to

if is_[property] methods do not provide a nice abstraction, then maybe all those empty match arms actually serve some purpose

No, those are exactly what I would like, except they're not available for enum variants, i.e. for enum Foo { Bar(i8), Baz } there is no (automatically-created) Foo::is_bar(&self) method. It is in fact the absence of such an interface that required the use of if let or match.

One problem with the existing if let syntax is that it is a wholly unique expression masquerading as an if statement. It isn't natural to have an if statement you can't negate - the default expectation is that this is a boolean expression that may be treated the same as any other predicate in an if clause.

H2CO3 commented 5 years ago

No, those are exactly what I would like, except they're not available for enum variants.

Oh, I think that can be fixed quite easily. They could be #[derive]d easily based on naming conventions. I'll try to write up an example implementation soon.

burdges commented 5 years ago

I'd suggest #[allow(non_snake_case)] for autogenerated one, so they look like

    #[allow(non_snake_case)]
    pub fn is_A(&self) { if let MyEnum::A = self { true } else { false } }

That said, if you have more than a couple variants then is_ methods might describe meaningful variants collections, not individual variants. We're talking about situations where you do not care about the variant's data after all.

In fact, you'd often want the or/| pattern like

let x = match self.integrity_policy {
    Foo::A(x) | Foo::B(x,_) => x,
    Foo::C { .. } | Foo::D => return Ok(()),
    Foo::E(_) => panic!(),
}
H2CO3 commented 5 years ago

I made a PoC implementation of the "generate is_XXX methods automatically" approach.

Centril commented 5 years ago

@H2CO3 This already exists on crates.io: https://crates.io/crates/derive_is_enum_variant and there's likely more complex variants too involving prisms/lenses and whatnot.

It seems to me however that generating a bunch of .is_$variant() methods is not a solution that scales well and that the need for these are due to the lack of a bool typed operation such as expr is pat (which could be let pat = expr... -- color of bikeshed...).

Fortunately, given or-patterns (https://github.com/rust-lang/rust/issues/54883) let chains (https://github.com/rust-lang/rust/issues/53667), and a trivial macro defined like so:

macro_rules! is { ($(x:tt)+) => { if $(x)+ { true } else { false } } }

we can write:

is!(let Some(E::Bar(x)) && x.some_condition() && let Other::Stuff = foo(x));

and the simpler version of this is:

is!(let E::Bar(..) = something)
// instead of: something.is_bar()
H2CO3 commented 5 years ago

@Centril I'm glad it already exists in a production-ready version. Then OP can just use it without needing to wait for an implementation.

there's likely more complex variants too involving prisms/lenses and whatnot.

What do you exactly mean by this? AFAIK there aren't many kinds of enum variants in Rust. There are only unit, tuple, and struct variants. I'm unaware of special prism and/or lens support in Rust that would lead to the existence of other kinds of variants.

It seems to me however that generating a bunch of .is_$variant() methods is not a solution that scales well

I beg to differ. Since they are generated automatically, there's not much the user has to do… moreover the generated functions are trivial, there are O(number of variants) of them, and they can be annotated with #[inline] so that the binary size won't be bigger compared to manually-written matching or negated if-let.

Fortunately, given or-patterns [and] let chains

Those are very nice, and seem fairly powerful. I would be glad if we could indeed reuse these two new mechanisms plus macros in order to avoid growing the language even more.

Centril commented 5 years ago

@H2CO3

@Centril I'm glad it already exists in a production-ready version. Then OP can just use it without needing to wait for an implementation.

🎉

What do you exactly mean by this? AFAIK there aren't many kinds of enum variants in Rust. There are only unit, tuple, and struct variants. I'm unaware of special prism and/or lens support in Rust that would lead to the existence of other kinds of variants.

I found this a while back: https://docs.rs/refraction/0.1.2/refraction/ and it seemed like an interesting experiment; other solutions might be to generate .extract_Bar(): Option<TupleOfBarsFieldTypes> and then use ? + try { .. } + .and_then() pervasively. You can use .is_some() on that to get the answer to "was it of the expected variant".

I beg to differ. Since they are generated automatically, there's not much the user has to do… moreover the generated functions are trivial, there are O(number of variants) of them, and they can be annotated with #[inline] so that the binary size won't be bigger compared to manually-written matching or negated if-let.

In a small crate I don't think it would pay off to use a derive macro like this; just compiling syn and quote seems too costly to get anyone to accept it. Another problem with the deriving strategy is that when you are working on a codebase (say rustc) and you want to check whether a value is of a certain variant, then you first need to go to the type's definition and add the derive macro; that can inhibit flow.

Those are very nice, and seem fairly powerful. I would be glad if we could indeed reuse these two new mechanisms plus macros in order to avoid growing the language even more.

Sure; this is indeed nice; but imo, it seems natural and useful to think of let pat = expr as an expression typed at bool which is true iff pat matches expr and false otherwise. In that case, if !let pat = expr { ... } is merely the composition of if !expr { ... } and let pat = expr.

alexreg commented 5 years ago

I'm for this. It arguably reduces the semantic complexity of the language, and improves consistency, especially with let-chaining coming in.

Has @rust-lang/libs thought of bundling the is! macro with the language, given how common it is?

mbrubeck commented 5 years ago

See also #1303.

graydon commented 5 years ago

Opposed. Not a big enough use-case to warrant extension at language level. Similar to there not being both while and do while loops, it can be done via numerous user-level escape hatches provided above.

mqudsi commented 5 years ago

It is respectfully nothing like the while vs do while situation. A while loop is one thing, and do while is another. An if statement already exists in the language with clearly defined semantics and permutations. This syntax reuses the if statement in name only, masquerading as a block of code while not actually sharing any of its features apart from reusing the same if keyword, with no reason why that can’t be fixed.

graydon commented 5 years ago

I regard if let the same way you do: as a random recycling of if somewhat out of context; I wouldn't have voted for its addition. But disliking it doesn't mean I think it needs to get more ways to mean something else yet again. It does not. If you dislike my analogy, you can substitute the analogy that we don't (and should not gain) a match-not expression.

(And definitely also not a guard-let or whatever Swift has. It has too many of these.)

comex commented 5 years ago

Which is why I think let should become an actual boolean expression, albeit with special rules around scoping. That solves the “random recycling” problem and also enables if !let.

comex commented 5 years ago

(and while this is admittedly both rude and off topic :\, after checking your recent posts, is there anything you’re not against?)

misha-krainik commented 5 years ago

In Ruby exists unless operator where executes code if conditional is false. If the conditional is true, code specified in the else clause is executed. It may be like

unless let Foo::Bar(_) = self.integrity_policy {
        return Ok(());
}
mark-i-m commented 5 years ago

I’ve always disliked unless operators in languages that have them. There is some cognitive overhead for me to deal with the extra implicit negation.

vext01 commented 5 years ago

Hi,

I've landed here looking for if !let. I wanted to write:

if !let Ok((Index{num_mirs: 0}, None)) = Decoder::new(&mut curs) {
    panic!();
}

Which I think today, is best expressed as:

match Decoder::new(&mut curs) {                                                                 
    Ok((Index{num_mirs: 0}, None)) => (),                                     
    _ => panic!(),                                                            
}
scottmcm commented 5 years ago

@mark-i-m I dislike them when they're just sugar for !, but if there's a difference in what they do (like the unless block must be : !), then it might be plausible. The negation, given the !-typed block, is consistent with things like assert!, so I don't think it's fundamentally confusing.

mark-i-m commented 5 years ago

I feel like that would be a bit unexpected for most people. There is conceptually no reason unless foo {...} would not be a normal expression like if or match.

Pzixel commented 5 years ago

See https://github.com/rust-lang/rfcs/pull/1303

nox commented 5 years ago

I feel like that would be a bit unexpected for most people. There is conceptually no reason unless foo {...} would not be a normal expression like if or match.

AFAIK nobody is confused about guard statements in Swift.

HeroicKatora commented 5 years ago

The usage of a pattern in if !let Foo::Bar(_) = self.integrity_policy is extremely confusing to me. What if I don't ignore the content but use a name pattern? It wouldn't be visible in the block the follows as that is executed when the destructuring failed but the if also makes this a single pattern.

if !let Foo::Bar(policy) = self.integrity_policy else {
    // Wait, `policy` is not a name here.
    return Ok(policy);
}

Guard blocks as in #1303 don't have this problem.

RustyYato commented 5 years ago

@HeroicKatora I think you linked to the wrong RFC (I don't think you mean to link to supporting comments in rustdoc)

HeroicKatora commented 5 years ago

@KrishnaSannasi Indeed. Thanks.

Canadadry commented 4 years ago

I don't really like the different proposition of the if !let .. or if let Some(_) != ... they feel wrong to me but it still think there is a need for something like this.

Here my use case. it's a fake one but I have often encounter similar situation:

fn strange_function(real_unsecure_value:SomeType) -> u32
{
    let x = real_unsecure_value;

    if let Some(foo) = x.getFoo()
    {
        if let Some(bar) = foo.getBar() 
        {
            if let Some(qux) = bar.getQux()
            {
                if qux.some_test()
                {
                    return qux.some_calculus();
                }
                else 
                {
                    return 0;
                }
            }
            else 
            {
                return 1;
            }
        }
        else
        {
            return 2;
        }
    }
    else 
    {
        return 3;
    }
}

We can all agree this does not look pretty neither really readdable. What failed unwrapping does return 2 ? And we only have 4 tests. match will not really help us here.

Some general idea is fail fast. So instead of branching on success we do the opposite.

fn strange_function(real_unsecure_value:SomeType) -> u32
{
    let x = real_unsecure_value;

    if x.getFoo()     == None  { return 3; }
    let foo = x.getFoo().unwrap();
    if foo.getBar()   == None  { return 2; }
    let bar = foo.getBar().unwrap();
    if bar.getQux()   == None  { return 1; }
    let qux = bar.getQux().unwrap();
    if qux.someTest() == false { return 0; }

    return qux.some_calculus();
}

This is much more readable, but there is useless unwrapping in between.

Here what I propose :

fn strange_function(real_unsecure_value:SomeType) -> u32
{
    let x = real_unsecure_value;

    guard let Some(foo) = x.getFoo()    { return 3; }
    guard let Some(bar) = foo.getBar()  { return 2; }
    guard let Some(qux) = bar.getQux()  { return 1; }
    if qux.someTest() == false { return 0; }

    return qux.some_calculus();
}
Pzixel commented 4 years ago

I actually don't see why any special syntax is required here.

This is a perfectly valid C# code

int? obj = null;

if (!(obj is int i)) {
    // int j = i; // CS0165 Use of unassigned local variable 'i'
    return;
}
Console.WriteLine(i);

It's not very surprising that i is only allowed outside the block since you negated the condition. I think considering it confusing is a bit overthought.

hadronized commented 4 years ago

I don't really like the different proposition of the if !let .. or if let Some(_) != ... they feel wrong to me but it still think there is a need for something like this.

Here my use case. it's a fake one but I have often encounter similar situation:

fn strange_function(real_unsecure_value:SomeType) -> u32
{
  let x = real_unsecure_value;

  if let Some(foo) = x.getFoo()
  {
      if let Some(bar) = foo.getBar() 
      {
          if let Some(qux) = bar.getQux()
          {
              if qux.some_test()
              {
                  return qux.some_calculus();
              }
              else 
              {
                  return 0;
              }
          }
          else 
          {
              return 1;
          }
      }
      else
      {
          return 2;
      }
  }
  else 
  {
      return 3;
  }
}

We can all agree this does not look pretty neither really readdable. What failed unwrapping does return 2 ? And we only have 4 tests. match will not really help us here.

Some general idea is fail fast. So instead of branching on success we do the opposite.

fn strange_function(real_unsecure_value:SomeType) -> u32
{
  let x = real_unsecure_value;

  if x.getFoo()     == None  { return 3; }
  let foo = x.getFoo().unwrap();
  if foo.getBar()   == None  { return 2; }
  let bar = foo.getBar().unwrap();
  if bar.getQux()   == None  { return 1; }
  let qux = bar.getQux().unwrap();
  if qux.someTest() == false { return 0; }

  return qux.some_calculus();
}

This is much more readable, but there is useless unwrapping in between.

Here what I propose :

fn strange_function(real_unsecure_value:SomeType) -> u32
{
  let x = real_unsecure_value;

  guard let Some(foo) = x.getFoo()    { return 3; }
  guard let Some(bar) = foo.getBar()  { return 2; }
  guard let Some(qux) = bar.getQux()  { return 1; }
  if qux.someTest() == false { return 0; }

  return qux.some_calculus();
}

I know someone who would like to have monads in Rust… :trollface:

robinmoussu commented 4 years ago

Since it's bikeshedding time, I would like a blue one!

fn strange_function(real_unsecure_value:SomeType) -> u32
{
    let x = real_unsecure_value;

    let Some(foo) = x.getFoo()   otherwise { return 3; }
    let Some(bar) = foo.getBar() otherwise { return 2; }
    let Some(qux) = bar.getQux() otherwise { return 1; }
    if qux.someTest() == false { return 0; }

    return qux.some_calculus();
}

It can also be chained easily


fn strange_function(real_unsecure_value:SomeType) -> u32
{
    let x = real_unsecure_value;

    let Some(foo) = x.getFoo() && Some(bar) = foo.getBar() otherwise { return 1; }

    return bar.some_calculus();
}
burdges commented 4 years ago

In practice, your function always returns -> Result<u32,u32> so the idiomatic solution is .ok_or(1)? or .ok_or_else(..)?.

Also try blocks or nested fns give precisely your style but with fewer bugs thanks to #[must_use].

#![feature(try_blocks)]

fn strange_function(real_unsecure_value:SomeType) -> u32
{
        try {
            let qux = real_unsecure_value
                .getFoo().ok_or(3)?
                .getBar().ok_or(2)?
                .getQux().ok_or(1)?;
            if qux.someTest() { 0 } else { qux.some_calculus() }
        }.map_or_else(|x| x, |x| x)
}

At present, rustfmt cannot correctly group long method chains like this, but either do not use rustfmt, or else if you mut use rustfmt then use temporaries, ala let foo = ..; let bar = ..;.

robinmoussu commented 4 years ago

@burdges That's great to know, thanks.

As a side node, if you want syntax coloration in your snippet, you can do it like so:

``` rust
 // some rust code
```
Canadadry commented 4 years ago

@phaazon I cannot be the only one 😊

@burdges I like your .or_ok(_) but syntaxically speaking I dont see the point on have the try. Is it just a compilation or parsing stuff that I don't see or is there more to know ?

burdges commented 4 years ago

All those ? exit the try block early with different values, but actually you're correct that .and_then accomplishes this on stable:

fn strange_function(real_unsecure_value:SomeType) -> u32
{
    real_unsecure_value.getFoo().ok_or(3)
        .and_then(|foo| foo.getBar().ok_or(2))
        .and_then(|bar| bar.getQux().ok_or(1))
        .and_then(|qux| if qux.someTest() { 0 } else { qux.some_calculus() })
        .unwrap_or_else(|x| x)
}

You might however want the try with ? if you've convoluted logic inside.. or just because you like monads.

Pzixel commented 4 years ago

What's about performing some specific logic?

if !let Some(f) = get_some_int() {
   f = 10;
}
println!("I've got {}", f);
if !let Some(x) = another_maybe_int() {
    println!("Something bad happened");
    return 20;
}
f + x

Anyway, combinators are really a mess, especially when you add lifetimes and generics in mixin. This is why people are so exited to hear about async/await. And this is why and_then chains are not usable comparing to if let chaining.

burdges commented 4 years ago

It becomes cleaner with combinators

let f = get_some_int().unwrap_or(10);
println!("I've got {}", f);
another_maybe_int().map(|x| f+x).unwrap_or_else(|| {
    println!("Something bad happened");
    20
})

Also, your syntax should never work because f has an ambiguous binding, which requires being both moved and not moved.

(edited with trivial fix)

Pzixel commented 4 years ago

And here you introduced a bug. Previosely if another_maybe_int returned None then function result was 20, but not it's f + 20 instead.

It clearly shows why combinators are hard to reason about. They also create scopes because of lambdas. It's not easy to return value from the inner function.


Original code I was replying to was

let f = get_some_int().unwrap_or(10);
println!("I've got {}", f);
f + another_maybe_int().unwrap_or_else(|| {
    println!("Something bad happened");
    20
})

It got edited so my comment became weird. Put edit here to clarify things.

steveklabnik commented 4 years ago

I just ran into this today; I am using char_indicies, and don't care about the index, only want to match if the char is not a specific one.

erik-helmers commented 4 years ago

I think that having an if in an expression that can not be negated nor chained is misleading for many programmers coming from other major languages.

As @mqudsi said,

One problem with the existing if let syntax is that it is a wholly unique expression masquerading as an if statement. It isn't natural to have an if statement you can't negate - the default expectation is that this is a boolean expression that may be treated the same as any other predicate in an if clause.

But this is changing - as some have mentioned above - the let- and while-let chaining is coming, and it allows the if let to behave a bit more like most people could expect : like a boolean.

From the Rust by Example book :

// For some use cases, when matching enums, match is awkward. For example:
match optional {
    Some(i) => {
        println!("This is a really long string and `{:?}`", i);
        // ^ Needed 2 indentations just so we could destructure
        // `i` from the option.
    },
    _ => {},
    // ^ Required because `match` is exhaustive. Doesn't it seem
    // like wasted space?
};

if let is cleaner for this use case and in addition allows various failure options to be specified

So it looks like the main intent of this feature was to make code easier to read. Without the ability to negate the if let, we have to use :

if let Some(i) = optional {} else {
   // The "negation" is now in this strange place, and once again we have an empty block     
    ...
}

This syntax is clearly not ideal, as was originally using a match expression with an empty block ; as such I think that it would be a great thing to allow "if let" negation.

burdges commented 4 years ago

I think .is_none() etc. has already become idiomatic, so you cannot make examples with Option or Result without then arguing to remove .is_none() etc. You need examples with other enums.

Pzixel commented 4 years ago

@burdges this example is completely fine:

if let Some(i) = optional {} else {
   panic!("Very very bad optional");
}
// how do I get access to `i` without unwraps here?

We already have match syntax but match branches cause you to have unnecessary block. One could imagine 3 or 4 checks like this that give you 4 indentation levels instead of zero.

dkasak commented 4 years ago

Is this issue also appropriate to discuss the negation of while let?

I just ran into the following today (conceptually). I want to let the user edit a file and then process it, returning a result. The processing can fail if the file is not valid in some way.

I would like to write

edit(file);

while !let Ok(x) = process(file) {
    edit(file);
}

// use `x`

But due to the lack of while-not-let, I have to duplicate the processing lines and unwrap the result:

edit(file);

let mut y = process(file);
while y.is_err() {
    edit(file)
    y = process(file);
}

let x = y.unwrap();

// use `x`

I also thought of the following, but I'm not sure it's any better.

let mut x;

loop {
    edit(file);

    if let Ok(y) = process(file) {
        x = y;
        break;
    }
}

// use `x`
Pzixel commented 4 years ago

@dkasak you can rewrite it as following

let x = loop {
    edit(file);

    if let Ok(y) = process(file) {
        break y;
    }
};

But using while !let would be more clear IMO

dkasak commented 4 years ago

@Pzixel Ah thanks, I forgot that loop can break with value. But I agree, while !let is still clearer.

burdges commented 4 years ago
let x = repeat(()).filter_map(|_| {
    edit(file);
    process(file).err() 
}).next().unwrap();

or

let mut x;
while {
    edit(file);
    x = process(file);
    x
} { }
95th commented 4 years ago

I think matches macro covers the case somewhat:

if !matches!(self.integrity_policy, Foo::Bar(_)) {
    return Ok(());
}
nbdd0121 commented 4 years ago

I personally prefer

if let pattern != expr {
    expr2
} else {
    expr3
}

which we can desugar to

match expr {
    pattern => expr3,
    _ => expr2,
}

(Of course we could omit the else block so expr3 is ())

Compared to changing let to a boolean expression so we can use !let, or use matches!, this has an advantage that we could use bindings in pattern in the else block, and it is very easy to implement in rustc.

teohhanhui commented 4 years ago

As a user,

if let pattern = expr {

is already mildly confusing.

Taking it to

if let pattern != expr {

is just asking for a lot of confused users.

somebody1234 commented 4 years ago

my two cents, = is assignment; != is inequality so != doesn't make sense at all

95th commented 4 years ago

How about adding a not keyword there. Binding has to be forbidden I guess.

if not let Foo::Bar(_) = self.integrity_policy {
    return Ok(());
}