"Irrefutable Pattern" Warnings in Let Chains

language design
42

Scoping a variable locally is convenient, but so is scoping two or three variables, no? Allowing the first binding to be irrefutable but not the second seems a little arbitrary. I think I'd personally leave the lint as it is.

43

The second one is already allowed. There is no warning on the second let pattern being irrefutable. It only applies to the first and the last let in the chain.

44

Oh, I misread. The lint considers all leading irrefutable patterns to be a problem

fn main() {
    let a = Some(1234);
    let b = Some(5678);
    if let x = 1 && let y = 2
        && let Some(a) = a && let z = 3 && let Some(b) = b {}
}


warning: leading irrefutable patterns in let chain
 --> src/main.rs:4:8
  |
4 |     if let x = 1 && let y = 2
  |        ^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(irrefutable_let_patterns)]` on by default
  = note: these patterns will always match
  = help: consider moving them outside of the construct

and I thought the idea was to relaxe the lint only for the first let so that it would still trigger for let y = 2, but I see now that's not the case. In that case, the lint would only trigger for a leading let _ = ..., right?

45

Right, the idea is that the warning would trigger only in two cases:

  1. Leading irrefutable leading patterns that don't bind any names, such as if let _ = foo() && or if let (_, _) = ... &&
  2. All trailing irrefutable patterns such as if ... && let x = foo() { ... }, including simple irrefutable if let without a chain such as if let x = foo() { ... }.

The rationale is that in case 1 you can just move the first let expression before the if statement, in case 2 you can move the last let expression inside the if body.

The current rule is almost the same, except all leading irrefutable patterns are linted against, even if they bind a name.

4 Likes
46

I would rather encourage this version. It makes it clearer that the drop order is important. Using the if let syntax as a with statement, especially if there are other conditions after it, would obscure that importance. Adding another level of indentation is unfortunate, but it is the standard way of controlling drop order. I do realize that there is some advantage regarding the drop order, but I don't think that this advantage is very accessible to most users of the language. I for example, just had to look it up in the reference to remember the details. This is obviously just my personal opinion.

Since the current "accepted" style is the one being enforced right now, changing the lint to allow irrefultable patterns which bind a type with drop glue would effectively adopt that style. Also, anyone who was 'protected' by the lint, and used a type which simply happens to have drop glue, would not get notified. The drop glue could be almost trivial, like for Box or Rc.

If the lint was changed to suggest the new block version if the type has drop glue, then the drop order concern would be addressed.

(Note that trivial drop glue is still an issue - suggesting the rewrite for it would encourage redundant indentation)

11 Likes
47

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.