Implementation of RFC 2086 - Allow Irrefutable Let patterns by Nokel81 · Pull Request #49469 · rust-lang/rust

Nokel81 · 2018-03-29T03:46:54Z

This is the set of changes for RFC2086. Tracking issue #44495. Rendered here

kennytm · 2018-03-29T07:15:26Z

src/test/compile-fail/feature-gate-allow_irrefutable_if_let_patterns.rs

Change this comment to

// gate-test-irrefutable_let_pattern

so that the computer can recognize it.

kennytm · 2018-03-29T23:35:26Z

[00:05:31] tidy error: The Unstable Book has a 'language feature' section 'allow_irrefutable_lets' which doesn't correspond to an unstable language feature

Please fix the filename of the new unstable book entry. It should be called irrefutable_let_pattern.md.

kennytm · 2018-03-29T23:36:01Z

src/test/compile-fail/feature-gate-allow_irrefutable_if_let_patterns.rs

This should be #[allow(irrefutable_let_pattern)] I believe.

Nokel81 · 2018-03-30T15:56:19Z

I don't quite understand what that message means, how would I make it correspond?

kennytm

You forgot to rename allow_irrefutable_lets.md

kennytm · 2018-03-30T16:21:21Z

src/test/compile-fail/feature-gate-allow_irrefutable_if_let_patterns.rs

This line is too long. Split it into two lines:

#[allow(…)] //~^ ERROR …

kennytm · 2018-03-30T16:42:22Z

@TimNN looks like the bot got some false result

Nokel81 · 2018-03-30T17:05:17Z

Okay, now I am confused. What is the conversion for naming the unstable book file?

(Author fixed the code; Travis is still unhappy)

kennytm · 2018-03-30T21:27:50Z

@Nokel81 🤔 Maybe try to use hyphens instead of underscores? irrefutable-let-pattern.md

Tidy should be fixed to suggest the naming convention if this is the case 😑

Nokel81 · 2018-03-31T23:46:12Z

So here it says to use cx.tcx.sess.features but the compiler now says that that is private, has the api changed?

michaelwoerister · 2018-04-03T09:43:33Z

r? @nikomatsakis

nikomatsakis

These changes look great! Presuming we can pacify travis, the only thing I think we need is a test. I imagine something that (a) enables the feature gate and then (b) adds #![deny(irrefutable_let_pattern)] and shows that indeed we get a lint error.

nikomatsakis · 2018-04-04T20:18:12Z

src/test/compile-fail/feature-gate-allow_irrefutable_if_let_patterns.rs

Why do you have the allow here? Without the feature gate, there shouldn't be any lints generated, right?

That is correct, however, from the RFC the default is a deny lint.

So what I think is we should have, make sure that it fails with no gates or allow. Add the feature gate and check that it denies and then one with the feature gate and the allow

This test was here to make sure that the feature gate was also required

the default is a deny lint

Hmm, that's odd. We don't usually do deny-by-default lints...

In any case I agree testing all the combinations is good. The matter of default setting is separate and can be discussed on the tracking issue.

bors · 2018-04-05T22:35:09Z

☔ The latest upstream changes (presumably #49696) made this pull request unmergeable. Please resolve the merge conflicts.

Nokel81 · 2018-04-06T20:32:46Z

So while looking in src/librustc/lint/builtin.rs there are other lints that are Deny. I would be fine with making the default be allow but then why would someone write the code and then explicitly deny it? Since I believe that the allow/deny is a per instance item.

nikomatsakis · 2018-04-09T17:53:16Z

@Nokel81

with making the default be allow

I would have expected the default to be "warn", not allow

Nokel81 · 2018-04-09T17:55:16Z

Makes more sense now

zackmdavis · 2018-04-09T18:21:15Z

(Please remember to squash the commits with an informative message before merging this.)

rust-highfive · 2018-06-15T19:08:43Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

emilyalbini · 2018-06-15T20:27:49Z

Ok, this failure is legit. Apparently Travis times out (30m) while doctesting the new page in the unstable book.

Nokel81 · 2018-06-15T20:30:22Z

Is "doctest" a specific thing? Is there a page I can read so I can manually check the unstable book page?Or do I have to write some explicit tests for the page?

emilyalbini · 2018-06-15T20:37:35Z

Doctesting means trying to compile code snippets in a documentation page. It's done automatically to ensure all code snippets in the Rust documentation are valid.

The only code block in that page is this, so that's probably the cause.

Nokel81 · 2018-06-15T20:41:38Z

I see, will fix this tonight then.

Nokel81 · 2018-06-18T23:37:41Z

@pietroalbini I have looked into the code that is in the unstable book page but it is the exact same (except for comments) of the code in test/run-pass/allow-irrefutable_let_patterns.rs

emilyalbini · 2018-06-25T08:50:12Z

@Nokel81 can you try not having an infinite loop in that snippet?

nikomatsakis · 2018-06-25T21:34:56Z

r=me if travis is happy

kennytm · 2018-06-25T23:21:51Z

@bors r=nikomatsakis

bors · 2018-06-25T23:21:52Z

📌 Commit 9168034 has been approved by nikomatsakis

bors · 2018-06-26T00:04:42Z

⌛ Testing commit 9168034 with merge c191d63009a37669b90dd060ed2000e20b22a0e6...

bors · 2018-06-26T01:32:10Z

💔 Test failed - status-travis

rust-highfive · 2018-06-26T01:32:11Z

The job i686-gnu-nopt of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:53:21] [RUSTC-TIMING] rustc_metadata test:false 307.274
[00:53:21]    Compiling rustc_traits v0.0.0 (file:///checkout/src/librustc_traits)
[00:54:16] [RUSTC-TIMING] rustc_traits test:false 55.033
[00:54:32] [RUSTC-TIMING] rustc_typeck test:false 378.704
No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

rust-highfive · 2018-06-26T01:32:20Z

The job i686-gnu-nopt of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:53:21] [RUSTC-TIMING] rustc_metadata test:false 307.274
[00:53:21]    Compiling rustc_traits v0.0.0 (file:///checkout/src/librustc_traits)
[00:54:16] [RUSTC-TIMING] rustc_traits test:false 55.033
[00:54:32] [RUSTC-TIMING] rustc_typeck test:false 378.704
No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

emilyalbini · 2018-06-26T07:51:28Z

@bors retry

bors · 2018-06-26T09:20:40Z

⌛ Testing commit 9168034 with merge 309fd8a...

…matsakis Implementation of RFC 2086 - Allow Irrefutable Let patterns This is the set of changes for RFC2086. Tracking issue #44495. Rendered [here](rust-lang/rfcs#2086)

bors · 2018-06-26T11:20:13Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 309fd8a to master...

rust-highfive assigned michaelwoerister Mar 29, 2018

kennytm reviewed Mar 29, 2018

View reviewed changes

This comment has been minimized.

Sign in to view

kennytm previously requested changes Mar 30, 2018

View reviewed changes

This comment has been minimized.

Sign in to view

shepmaster added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 30, 2018

This comment has been minimized.

Sign in to view

rust-highfive assigned nikomatsakis and unassigned michaelwoerister Apr 3, 2018

nikomatsakis requested changes Apr 4, 2018

View reviewed changes

make the while let loop terminate

9168034

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 25, 2018

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 26, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 26, 2018

bors merged commit 9168034 into rust-lang:master Jun 26, 2018

Nokel81 deleted the allow-irrefutable-let-patterns branch June 27, 2018 18:29

Centril mentioned this pull request Sep 15, 2018

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

Closed

3 tasks

RalfJung mentioned this pull request Apr 28, 2025

Spurious irrefutable_let_patterns warning with let-chain #139369

Open

Uh oh!

Conversation

Nokel81 commented Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kennytm commented Mar 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Nokel81 commented Mar 30, 2018

Uh oh!

kennytm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

kennytm commented Mar 30, 2018

Uh oh!

This comment has been minimized.

Nokel81 commented Mar 30, 2018

Uh oh!

kennytm commented Mar 30, 2018

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Nokel81 commented Mar 31, 2018

Uh oh!

michaelwoerister commented Apr 3, 2018

Uh oh!

nikomatsakis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bors commented Apr 5, 2018

Uh oh!

Nokel81 commented Apr 6, 2018

Uh oh!

nikomatsakis commented Apr 9, 2018

Uh oh!

Nokel81 commented Apr 9, 2018

Uh oh!

zackmdavis commented Apr 9, 2018

Uh oh!

rust-highfive commented Jun 15, 2018

Uh oh!

emilyalbini commented Jun 15, 2018

Uh oh!

Nokel81 commented Jun 15, 2018 via email • edited by emilyalbini Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emilyalbini commented Jun 15, 2018

Uh oh!

Nokel81 commented Jun 15, 2018 via email • edited by emilyalbini Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nokel81 commented Jun 18, 2018

Uh oh!

emilyalbini commented Jun 25, 2018

Uh oh!

nikomatsakis commented Jun 25, 2018

Uh oh!

Nokel81 commented Mar 29, 2018 •

edited

Loading

Nokel81 commented Jun 15, 2018 via email •

edited by emilyalbini

Loading

Nokel81 commented Jun 15, 2018 via email •

edited by emilyalbini

Loading