New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stabilise feature(never_type). Introduce feature(exhaustive_patterns) #47630

Merged
merged 14 commits into from Mar 15, 2018

Conversation

@canndrew
Copy link
Contributor

canndrew commented Jan 21, 2018

This stabilizes !, removing the feature gate as well as the old defaulting-to-() behavior. The pattern exhaustiveness checks which were covered by feature(never_type) have been moved behind a new feature(exhaustive_patterns) gate.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 21, 2018

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

@canndrew canndrew referenced this pull request Jan 21, 2018

Open

Tracking issue for promoting `!` to a type (RFC 1216) #35121

12 of 13 tasks complete
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 21, 2018

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

@@ -880,24 +880,24 @@ mod impls {

ord_impl! { char usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 }

#[unstable(feature = "never_type", issue = "35121")]
#[stable(feature = "never_type", since = "1.24.0")]

This comment has been minimized.

@varkor

varkor Jan 21, 2018

Member

Shouldn't this be since = "1.25.0"?

This comment has been minimized.

@canndrew

canndrew Jan 21, 2018

Contributor

True, it's too late to make 1.24. Thanks for the catch!

@canndrew canndrew force-pushed the canndrew:exhaustive-patterns branch from cc3e60c to 72ce701 Jan 21, 2018

@nagisa

This comment has been minimized.

Copy link
Contributor

nagisa commented Jan 21, 2018

@dtolnay
Copy link
Member

dtolnay left a comment

I believe when we split a feature gate into stable part and unstable part, we typically try to rename the stable part rather than the unstable part. So in this case ! would be stabilized under something like feature(never_bang) and what you have as feature(exhaustive_patterns) would continue to be feature(never_type). This minimizes churn for libraries that currently use never_type. Their existing code would continue to compile.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jan 23, 2018

If I understand correctly, this will also resolve the future compatibiltiy question (and make the change to fallback to ! instead of ()). Therefore, It seems like we should do a crater run to test the impact.

@bors try

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jan 23, 2018

If I understand correctly, this will also resolve the future compatibility question in #39216 (and make the change to fallback to ! instead of ()). Therefore, It seems like we should do a crater run to test the impact.

@bors try

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 23, 2018

⌛️ Trying commit 72ce701 with merge e2e6c4a...

bors added a commit that referenced this pull request Jan 23, 2018

Auto merge of #47630 - canndrew:exhaustive-patterns, r=<try>
Stabilise feature(never_type). Introduce feature(exhaustive_patterns)

This stabilizes `!`, removing the feature gate as well as the old defaulting-to-`()` behavior. The pattern exhaustiveness checks which were covered by `feature(never_type)` have been moved behind a new `feature(exhaustive_patterns)` gate.
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 23, 2018

☀️ Test successful - status-travis
State: approved= try=True

@scottmcm scottmcm referenced this pull request Jan 24, 2018

Open

Tracking issue for TryFrom/TryInto traits #33417

0 of 3 tasks complete
@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Jan 24, 2018

The pattern exhaustiveness checks which were covered by feature(never_type) have been moved behind a new feature(exhaustive_patterns) gate.

I haven’t followed all the discussions around !, could you give some examples of code affected by this? In this different from what’s already possible on stable with empty enums?

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Jan 24, 2018

Never mind, I saw your explenation on #35121 (comment). As to empty enums, this also causes error[E0005]: refutable pattern in local binding: `Err(_)` not covered

    enum Void {}
    let r: Result<u32, Void> = Ok(4);
    let Ok(i) = r;
@canndrew

This comment has been minimized.

Copy link
Contributor

canndrew commented Jan 24, 2018

@dtolnay That would mean only the pattern-matching stuff is behind the never_type feature gate though, which would make it a bit of a misnomer.

As it is I deleted the never_type feature entirely. When I tried to add it back in order to follow https://forge.rust-lang.org/stabilization-guide.html it caused errors from anything tagged with stable(feature = "never_type", since = "1.25.0") complaining about mismatching issue numbers. There are no other stable tags with issue numbers so I don't know what it wants exactly.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jan 24, 2018

@canndrew huh, I've never seen those particular errors before

@aidanhs

This comment has been minimized.

Copy link
Member

aidanhs commented Jan 25, 2018

Crater run started.

@aidanhs

This comment has been minimized.

Copy link
Member

aidanhs commented Jan 31, 2018

Crater run is currently stalled due to runs taking up more disk space than they used to. We've started up a bigger box and are working through the backlog - ETA another week or so, sorry :(

@canndrew

This comment has been minimized.

Copy link
Contributor

canndrew commented Feb 1, 2018

@nikomatsakis

huh, I've never seen those particular errors before

Specifically, I added (accepted, never_type, "1.25.0", Some(35121)) to the list of accepted features in features.rs. I then get tidy errors saying mismatches to corresponding lang feature in: ["tracking issue"] for all lines that say #[stable(feature = "never_type", since = "1.25.0")]

@canndrew canndrew force-pushed the canndrew:exhaustive-patterns branch from 72ce701 to 71fa69c Feb 1, 2018

@aidanhs

This comment has been minimized.

Copy link
Member

aidanhs commented Feb 8, 2018

When this is ready for another run can you get bors to do a try? Then we'll prioritise this PR since it's been idle and waiting for a while.

@nikomatsakis nikomatsakis force-pushed the canndrew:exhaustive-patterns branch from 71fa69c to c3beef6 Feb 8, 2018

@canndrew

This comment has been minimized.

Copy link
Contributor

canndrew commented Mar 14, 2018

@nikomatsakis I've addressed the nits. Is this ready to rock?

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Mar 14, 2018

@bors r+

@canndrew 🎸 👏 🎸

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 14, 2018

📌 Commit a8a0c69 has been approved by nikomatsakis

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 14, 2018

⌛️ Testing commit a8a0c69 with merge 5ebf748...

bors added a commit that referenced this pull request Mar 14, 2018

Auto merge of #47630 - canndrew:exhaustive-patterns, r=nikomatsakis
Stabilise feature(never_type). Introduce feature(exhaustive_patterns)

This stabilizes `!`, removing the feature gate as well as the old defaulting-to-`()` behavior. The pattern exhaustiveness checks which were covered by `feature(never_type)` have been moved behind a new `feature(exhaustive_patterns)` gate.

@kennytm kennytm added the relnotes label Mar 15, 2018

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 15, 2018

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

@canndrew

This comment has been minimized.

Copy link
Contributor

canndrew commented Mar 15, 2018

@clarcharr I've submitted a PR for replacing Infallible with !: #49038

@davll davll referenced this pull request Mar 22, 2018

Closed

Replace `Never` with `!` #897

@messense messense referenced this pull request Mar 23, 2018

Open

Compile with stable Rust #19

7 of 12 tasks complete

kennytm added a commit to kennytm/rust that referenced this pull request Sep 14, 2018

Rollup merge of rust-lang#54207 - QuietMisdreavus:never-docs-stab, r=…
…kennytm

re-mark the never docs as unstable

Fixes rust-lang#54198

This stability attribute was removed in rust-lang#47630, but not replaced with a `#[stable]` attribute, and when rust-lang#50121 reverted that stabilization, it didn't set the docs back to unstable. I'm concerned as to why it was allowed to not have the stability attribute at all, but at least this can put it back.

I'm nominating this for beta backport because it's a really small change, and right now our docs are in an awkward position where the `!` type is technically unstable to use, but the docs don't say so the same way any other library feature would. (And this is also the case *on stable* now, but i'm not suggesting a stable backport for a docs fix.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment