Tracking issue for RFC 2086: Allow Irrefutable Patterns in if-let and while-let statements

[aturon]

This is a tracking issue for the RFC "Allow Irrefutable Patterns in if-let and while-let statements" (rust-lang/rfcs#2086).

Steps:

• Implement the RFC (mentoring instructions here)
• Adjust documentation (rust-lang-nursery/reference#507)
• Stabilization PR (#57535)

[arielb1]

Mentoring Instructions

This is actually an easy one:

The code that emits the error message is the check_arms function here:

rust/src/librustc_const_eval/check_match.rs

Line 317 in 325ba23

fn check_arms<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>,

It nicely contains branches for if let and while let, so we just need to adapt them to emit lints instead of errors. We emit lints using the lint_node function - you can see an example calls of it below in the function. Remember to add a feature gate.

You'll also want to declare the new lints you are using, which is done in:

rust/src/librustc/lint/builtin.rs

Line 19 in 325ba23

declare_lint! {

Just add another declare_lint! for each new lint.

Feature Gates

I should add some version of this to the README.md, but until then:

Also, you'll want to feature gate the new feature, so that it can't be used accidentally until its stabilized. For that, add a feature gate to the list of feature gates here:

rust/src/libsyntax/feature_gate.rs

Line 388 in 325ba23

);

And link this tracking issue (#44495). You can test the feature in tcx.sess (e.g. tcx.sess.features.borrow().specialization). If it's not present, you should maintain the old behavior.

Don't forget to add tests, both for the new behavior and the old one (the test that the old behavior remains without a feature gate should be in src/test/compile-fail/feature-gate-YOUR-FEATURE.rs).

[Nokel81]

I will now start on the implementation of this

[nikomatsakis]

@Nokel81 just checking in -- how's it going? Any questions?

[Nokel81]

No, school work came up but I should have a PR ready by Wednesday

[davidtwco]

After discussion with @nikomatsakis in Gitter, I'm going to take a stab at this one.

[Nokel81]

Okay, I just haven't had time to finish my implementation what with school

[davidtwco]

If you've already started on this then I'm happy to leave it @Nokel81.

[Nokel81]

It looks like I should have some time this weekend to finish it so from a time perspective I would prefer it if I could finish

[davidtwco]

Sure thing.

[nikomatsakis]

Anybody working on this actively?

[Nokel81]

I am and I have what I believe to be a full solution but I have had trouble compiling rustc because I am on windows with VS2017. I even tried following the instructions for not VS2015 but that didn't seem to work

[nikomatsakis]

@Nokel81 Sorry, missed that message -- perhaps give some more details of the problem you are encountering? (Alternatively, can you point us at your branch..?)

[nikomatsakis]

(Though honestly the best thing is probably to hop on gitter or maybe #rust on IRC and ask about, since I at least don't know much about getting Rust to build on Windows.)

[Nokel81]

Okay thanks. I will try that tomorrow

[nikomatsakis]

@Nokel81 any luck?

[austinbes]

@Nokel81 If you're still having trouble building, I'm interested in taking a crack at this

[Nokel81]

I believe that I have solved the building problem and should be able to get a PR tonight. Thanks for the offer though.

[Centril]

@rfcbot merge

This was implemented in #49469 which was merged on 2018-06-26 which was 11 weeks ago (more than enough...). I thus propose that we stabilize it with an amendment to make the default lint level Warn instead of Deny (reasons outlined in #44495 (comment)).

Unstable book:

• https://github.com/rust-lang/rust/blob/master/src/doc/unstable-book/src/language-features/irrefutable-let-patterns.md

Tests:

• https://github.com/rust-lang/rust/blob/master/src/test/ui/should-fail-with_gate_irrefutable_pattern_deny.rs
• https://github.com/rust-lang/rust/blob/master/src/test/ui/run-pass/binding/allow_irrefutable_let_patterns.rs

[rfcbot]

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

• @Centril
• @Zoxc
• @aturon
• @cramertj
• @eddyb
• @estebank
• @joshtriplett
• @michaelwoerister
• @nagisa
• @nikomatsakis
• @nrc
• @oli-obk
• @petrochenkov
• @pnkfelix
• @scottmcm
• @varkor
• @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and none object), 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.

[scottmcm]

@rfcbot reviewed

That said, I still lean towards deny here. I don't think my "WIP reasoning" is a strong argument for warn here, because there obviously aren't any more alternatives that you could be intending to add later.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[Nokel81]

I would lean towards deny for a single if let but warn for multiple if lets because of the set up process that multiple if lets can allow for

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

[Centril]

@Nokel81 would you be willing to write up a stabilization PR?

[Nokel81]

Sure, however, there was a little discussion on the level. Shall I go with what was proposed in the merge by you (that is Warn instead of Deny)

[Centril]

@Nokel81 Thank you.
What was proposed in the FCP merge proposal; that is: Warn.

[Nokel81]

I will see if I can do this tonight

[Centril]

@Nokel81 any progress? :)

[Nokel81]

@Centril I have made the code change but I wanted to make sure that I could build it before making the PR. My build fails (even without the code change so I think it is something to do with my environment) with the following error

error: failed to run custom build command for `alloc_jemalloc v0.0.0 (/home/sebastian/rust/src/liballoc_jemalloc)`                                      

[Nokel81]

#55639

Would any of you be able to review the stabilization PR?

[Centril]

Stabilized in #57535.
Reference issue: rust-lang-nursery/reference#507.

Nothing left to do, so closing out.