RFC: Allow Irrefutable Patterns in if-let and while-let statements

[Nokel81]

This is an RFC for allowing irrefutable patterns in if-let and while-let statements

Edit: Rendered

[updated to link to final rendered version]

[eddyb]

@Nokel81 The title of the PR should be the RFC name (you can edit it).

[main--]

It allows programmers to manually write the line if let _ = expr { } else { } which is generally obfuscating and not desirable. However, a different RFC should solve this.

I thoroughly disagree. This is already solved right now - the code is wrong and thus an error. You're proposing a different solution which has to deal with this.

If anything, this is an unresolved question: If we can't solve it by making it an error, how do we solve it?

This RFC is also missing its Alternatives section even though I can think of two options off the top of my head:

• The trivial alternative: Do nothing. As your motivation explains, this only matters for macros anyways plus there already is an acceptable workaround (match). Code that needs this frequently can just package this workaround in its own macro and be done.
• Turn the error into a lint that errors by default. Unreachable match arms are usually wrong except in some macros and that's why you can disable the warning there with #[allow(unreachable_patterns)]. The same goes for irrefutable if/while-let patterns, so it only seems natural to apply a similar solution.

[Nokel81]

I will add those alternatives. From what I understand #[allow] is not allowed infront of if statements

[main--]

Indeed it is not but perhaps that could be changed?

In any case, you could always just add the attribute to a surrounding block.

[Nokel81]

I would say that it should be changed since from what I understand it is unique to if since while allows it. Also the warning should be equivalent to the one for #2087 since it is essentially also a tautology.

[burdges]

I think if cfg! should work sensibly, and similarly for the patterns given to macros that you worry about here, but an obviously irrefutable pattern like if let (a,b) = foo() { .. } should be written { let (a,b) = foo(); .. } instead.

Would it work if there were a way to declare an irrefutable pattern refutable? #![ensure_refutable] perhaps?

[Nokel81]

So instead of a warning, keep it a error but still have an [#allow] for it?

[nrc]

I'd be in favour of the lint option here - seems reasonable for macros to #[allow] a statement to opt-in to this behaviour. Technically this doesn't work for expressions, but I think that just means the macro needs to use a temporary variable.

[joshtriplett]

I likewise prefer the solution of making this an error-by-default lint that macros can turn off. For non-macro code, errors like this help Rust users (especially new users) learn and avoid using suboptimal forms that just happen to work. And macros can easily turn the lint off.

[tikue]

I am definitely in favor of some way to relax this error. I've stumbled on this in macros, and existing workarounds are not great. It's worth pointing out that match is not a drop-in solution to this RFC because in macros you can't always determine if there should be a _ => ... arm or not; this comes up when dealing with enums with an unknown number of variants. For such cases, the only solution right now (that I've found) is to add a NotIrrefutable variant. Pretty hacky. I didn't know about #[allow(unreachable_patterns)]. That's not so bad actually, but not very discoverable right now.

[joshtriplett]

@Nokel81 In the @rust-lang/lang meeting, we were all in favor of the solution of making this an error-by-default lint. Could you please adjust the RFC to that effect, and then we'd be inclined to accept it?

[Nokel81]

Yup, done. I hope the wording is to your liking

[joshtriplett]

@Nokel81 Thanks for the fast response!

What's there looks reasonable, but it would help if the summary section had an explanation of the proposed change and reason (e.g. "This can break macros that want to accept patterns generically; this RFC proposes changing this to an error-by-default lint that can be disabled by such macros."). Also, in the detailed design section, please come up with a proposed name for the lint (e.g. irrefutable_conditional_pattern), and write an example of some code using it that the compiler should accept.

[Nokel81]

How is this?

[joshtriplett]

@Nokel81 The verbiage looks great. One minor nit: lint names use underscores, not dashes, so can you s/irrefutable-let-pattern/irrefutable_let_pattern/g? With that, I think this will be ready for review.

[Nokel81]

Done

[aturon]

Thanks @Nokel81 and @joshtriplett!

@rfcbot fcp merge

[rfcbot]

Team member @aturon has proposed to merge this. The next step is review by the rest of the tagged teams:

• @aturon
• @eddyb
• @nikomatsakis
• @nrc
• @pnkfelix
• @withoutboats

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[Nokel81]

@aturon Shouldn't S-waiting-on-author be removed?

[durka]

As a minor note, I don't like the lint name, because a "let pattern" (i.e. Foo { x, y } in let Foo { x, y }: Foo = make_foo();) already is, and must be, irrefutable. But I don't really have a better suggestion and it can be bikeshedded later.

[Nokel81]

The reason that I went with this name is because the current error is "irrefutable if-let pattern". We could change the lint but then what about for while loops (though that may be out of the scope of this RFC)

[yodaldevoid]

I have a minor nit with the RFC as written. Under the "Motivation" section the following phrase makes no sense to me:

... to be both much larger and include a compiler allow.

I'm not sure what this is actually trying to say, really, but perhaps it could be reworded?

[ExpHP]

These two adjacent code blocks are confusing, especially since the second one starts a new thought.

I picture something more along the lines of:

Currently, one must write

(first example)

rather than the more natural syntax

(second example)

because reasons.

[ExpHP]

error-by-default lint?

[rfcbot]

The final comment period is now complete.

[aturon]

This RFC has been merged! Tracking issue.

Thanks @Nokel81!

[Nokel81]

Thank you

[vitiral]

@Nokel81 the rendered link is now broken

[Nokel81]

@vitiral fixed