Add docs for never primitive #46232

canndrew · 2017-11-24T02:52:10Z

QuietMisdreavus · 2017-11-24T04:13:02Z

src/libstd/primitive_docs.rs

+/// [`Debug`]: fmt/trait.Debug.html
+/// [`Default`]: default/trait.Default.html
+///
+#[stable(feature = "rust1", since = "1.23.0")]


Should this be attached to rust1, or would it be attached to never_type? I would assume the latter.

That's also the reason why tidy errors:

[00:03:19] tidy error: /checkout/src/libstd/primitive_docs.rs:171: mismatches to previous in: ["since"] [00:03:19] tidy error: /checkout/src/libstd/primitive_docs.rs:247: mismatches to previous in: ["since"]

because rust1 has been defined elsewhere already (probably with a since field of "1.0.0") and the since field that @canndrew specified "1.23.0" mismatches.

I changed it to #[unstable(feature = "never_type_impls", issue = "35121")] since that's what's used in other places in libstd/libcore.

kennytm · 2017-11-24T06:52:52Z

The following doc tests failed:

[01:08:18] failures:
[01:08:18]     primitive_docs.rs - prim_never (line 114)
[01:08:18]     primitive_docs.rs - prim_never (line 127)
[01:08:18]     primitive_docs.rs - prim_never (line 144)
[01:08:18]     primitive_docs.rs - prim_never (line 81)
[01:08:18]     primitive_docs.rs - prim_never (line 95)

[01:08:18] failures:
[01:08:18] 
[01:08:18] ---- primitive_docs.rs - prim_never (line 114) stdout ----
[01:08:18] 	error[E0277]: the trait bound `Self: std::marker::Sized` is not satisfied
[01:08:18]  --> primitive_docs.rs:6:5
[01:08:18]   |
[01:08:18] 6 |     fn from_str(s: &str) -> Result<Self, Self::Error>;
[01:08:18]   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Self` does not have a constant size known at compile-time
[01:08:18]   |
[01:08:18]   = help: the trait `std::marker::Sized` is not implemented for `Self`
[01:08:18]   = help: consider adding a `where Self: std::marker::Sized` bound
[01:08:18]   = note: required by `std::result::Result`
[01:08:18] 
[01:08:18] thread 'rustc' panicked at 'couldn't compile the test', /checkout/src/librustdoc/test.rs:290:12
[01:08:18] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:08:18] 
[01:08:18] ---- primitive_docs.rs - prim_never (line 127) stdout ----
[01:08:18] 	error[E0599]: no function or associated item named `from_str` found for type `std::string::String` in the current scope
[01:08:18]  --> primitive_docs.rs:4:13
[01:08:18]   |
[01:08:18] 4 | let Ok(s) = String::from_str("hello");
[01:08:18]   |             ^^^^^^^^^^^^^^^^ function or associated item not found in `std::string::String`
[01:08:18]   |
[01:08:18]   = help: items from traits can only be used if the trait is in scope
[01:08:18] help: the following trait is implemented but not in scope, perhaps add a `use` for it:
[01:08:18]   |
[01:08:18] 3 | use std::str::FromStr;
[01:08:18]   |
[01:08:18] 
[01:08:18] thread 'rustc' panicked at 'couldn't compile the test', /checkout/src/librustdoc/test.rs:290:12
[01:08:18] 
[01:08:18] ---- primitive_docs.rs - prim_never (line 144) stdout ----
[01:08:18] 	error[E0433]: failed to resolve. Use of undeclared type or module `fmt`
[01:08:18]  --> primitive_docs.rs:5:35
[01:08:18]   |
[01:08:18] 5 |     fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
[01:08:18]   |                                   ^^^ Use of undeclared type or module `fmt`
[01:08:18] 
[01:08:18] error[E0433]: failed to resolve. Use of undeclared type or module `fmt`
[01:08:18]  --> primitive_docs.rs:5:54
[01:08:18]   |
[01:08:18] 5 |     fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
[01:08:18]   |                                                      ^^^ Use of undeclared type or module `fmt`
[01:08:18] 
[01:08:18] error[E0405]: cannot find trait `Debug` in this scope
[01:08:18]  --> primitive_docs.rs:4:6
[01:08:18]   |
[01:08:18] 4 | impl Debug for ! {
[01:08:18]   |      ^^^^^ not found in this scope
[01:08:18] help: possible candidate is found in another module, you can import it into scope
[01:08:18]   |
[01:08:18] 3 | use std::fmt::Debug;
[01:08:18]   |
[01:08:18] 
[01:08:18] error: The `!` type is experimental (see issue #35121)
[01:08:18]  --> primitive_docs.rs:4:16
[01:08:18]   |
[01:08:18] 4 | impl Debug for ! {
[01:08:18]   |                ^
[01:08:18]   |
[01:08:18]   = help: add #![feature(never_type)] to the crate attributes to enable
[01:08:18] 
[01:08:18] thread 'rustc' panicked at 'couldn't compile the test', /checkout/src/librustdoc/test.rs:290:12
[01:08:18] 
[01:08:18] ---- primitive_docs.rs - prim_never (line 81) stdout ----
[01:08:18] 	error: The `!` type is experimental (see issue #35121)
[01:08:18]  --> primitive_docs.rs:4:8
[01:08:18]   |
[01:08:18] 4 | let x: ! = {
[01:08:18]   |        ^
[01:08:18]   |
[01:08:18]   = help: add #![feature(never_type)] to the crate attributes to enable
[01:08:18] 
[01:08:18] thread 'rustc' panicked at 'couldn't compile the test', /checkout/src/librustdoc/test.rs:290:12
[01:08:18] 
[01:08:18] ---- primitive_docs.rs - prim_never (line 95) stdout ----
[01:08:18] 	error: expected one of `.`, `;`, `?`, or an operator, found `}`
[01:08:18]  --> primitive_docs.rs:8:1
[01:08:18]   |
[01:08:18] 7 | }
[01:08:18]   |  - expected one of `.`, `;`, `?`, or an operator here
[01:08:18] 8 | }
[01:08:18]   | ^ unexpected token
[01:08:18] 
[01:08:18] thread 'rustc' panicked at 'couldn't compile the test', /checkout/src/librustdoc/test.rs:290:12

canndrew · 2017-11-24T07:44:44Z

Is there a way to run doc tests separately?

Also, I just forced-pushed an amendment to that last commit but it didn't cancel the old travis job. So job #63107 can be cancelled (would be nice if it happened automatically though).

kennytm · 2017-11-24T07:49:55Z

@canndrew Try

./x.py test --stage 1 src/libstd --test-args prim_never

If you mean without recompiling rustc... sorry not yet (#46180).

QuietMisdreavus · 2017-11-24T16:59:12Z

Out of curiosity, is it possible to reuse the never_type gate, instead of the never_type_impls one, since this is really about the type itself? Or would that clash with the language feature of the same name and cause headaches in the unstable book?

canndrew · 2017-11-25T04:37:08Z

Or would that clash with the language feature of the same name and cause headaches in the unstable book?

I don't know what headaches it would cause but I remember that there was a reason for having a separate never_type_impls gate.

est31 · 2017-11-25T04:44:42Z

You can use one single feature gate for lib and language features. See #43089

est31 · 2017-11-25T04:47:27Z

The unstable book handles them not really well. Documentation for the proc_macro feature appears both in the libary feature list and the language feature list.

Maybe it would be an idea to create a "common feature" list.

canndrew · 2017-11-25T06:31:27Z

@est31 Cool. I've made a separate PR to rename never_type_impls to never_type: #46250

QuietMisdreavus · 2017-11-27T17:05:21Z

Looks good, but i have one request: Can you change this line here to primitive_type(f, PrimitiveType::Never, "!") so automatically-printed ! types start linking to this page?

QuietMisdreavus · 2017-11-27T17:07:14Z

@rust-lang/docs Any comments on the doc text here? I think it's good, but it would be nice to have more eyes on it.

GuillaumeGomez · 2017-11-28T08:58:56Z

src/libstd/primitive_docs.rs

@@ -67,6 +67,123 @@
 #[stable(feature = "rust1", since = "1.0.0")]
 mod prim_bool { }

+#[doc(primitive = "never")]
+//


Why this line?

I'm not sure why it was there in the first place, but it does match the other primitive docs.

We should remove them then!

@rust-lang/cleanup-team

GuillaumeGomez · 2017-11-28T08:59:29Z

src/libstd/primitive_docs.rs

@@ -67,6 +67,123 @@
 #[stable(feature = "rust1", since = "1.0.0")]
 mod prim_bool { }

+#[doc(primitive = "never")]
+//
+/// The `!` type, also called "never".


This "introduction" sounds strange. I expected to have something after directly but just a dot. So strange.

This kind of introduction line matches the other primitives. Compare with "Raw, unsafe pointers, *const T, and *mut T", "References, both shared and mutable", "A finite heterogeneous sequence, (T, U, ..)", all of which appear in the current docs. Do you have a better suggestion?

GuillaumeGomez · 2017-11-28T09:05:14Z

src/libstd/primitive_docs.rs

+/// so returns `!`.
+///
+/// `break`, `continue` and `return` expressions also have type `!`. For example we are allowed to
+/// write


Please add :.

GuillaumeGomez · 2017-11-28T09:06:04Z

src/libstd/primitive_docs.rs

+/// }
+/// ```
+///
+/// When implementing this trait for `String` we need to pick a type for `Err`. And since


Please add urls for String and Err.

GuillaumeGomez · 2017-11-28T09:06:29Z

src/libstd/primitive_docs.rs

+/// let Ok(s) = String::from_str("hello");
+/// ```
+///
+/// Since the `Err` variant contains a `!`, it can never occur. So we can exhaustively match on


URL for Err.

GuillaumeGomez · 2017-11-28T09:06:44Z

src/libstd/primitive_docs.rs

+/// ```
+///
+/// Since the `Err` variant contains a `!`, it can never occur. So we can exhaustively match on
+/// `Result<T, !>` by just taking the `Ok` variant. This illustrates another behaviour of `!` - it


URLs for Result and Ok.

GuillaumeGomez · 2017-11-28T09:06:59Z

src/libstd/primitive_docs.rs

+/// # }
+/// ```
+///
+/// Both match arms must produce values of type `u32`, but since `break` never produces a value at


URL for u32.

GuillaumeGomez · 2017-11-28T09:07:21Z

src/libstd/primitive_docs.rs

+/// ```
+///
+/// Both match arms must produce values of type `u32`, but since `break` never produces a value at
+/// all we know it can never produce a value which isn't a `u32`. This illustrates another


URL for u32.

GuillaumeGomez · 2017-11-28T09:07:46Z

src/libstd/primitive_docs.rs

+/// When implementing this trait for `String` we need to pick a type for `Err`. And since
+/// converting a string into a string will never result in an error, the appropriate type is `!`.
+/// (Currently the type actually used is an enum with no variants, though this is only because `!`
+/// was added to Rust at a later date and it may change in the future). With an `Err` type of `!`,


URL for Err.

GuillaumeGomez · 2017-11-28T09:08:11Z

src/libstd/primitive_docs.rs

+/// converting a string into a string will never result in an error, the appropriate type is `!`.
+/// (Currently the type actually used is an enum with no variants, though this is only because `!`
+/// was added to Rust at a later date and it may change in the future). With an `Err` type of `!`,
+/// if we have to call `String::from_str` for some reason the result will be a `Result<String, !>`


URLs for String::from_str, Result and String.

GuillaumeGomez · 2017-11-28T09:08:30Z

src/libstd/primitive_docs.rs

+/// }
+/// ```
+///
+/// Once again we're using `!`'s ability to coerce into any other type, in this case `fmt::Result`.


URL for fmt::Result.

GuillaumeGomez · 2017-11-28T09:08:51Z

src/libstd/primitive_docs.rs

+/// Since this method takes a `&!` as an argument we know that it can never be called (because
+/// there is no value of type `!` for it to be called with). Writing `*self` essentially tells the
+/// compiler "We know that this code can never be run, so just treat the entire function body has
+/// having type `fmt::Result`". This pattern can be used a lot when implementing traits for `!`.


URL for fmt::Result.

GuillaumeGomez · 2017-11-28T09:09:20Z

src/libstd/primitive_docs.rs

+///
+/// Since `!` has no values, it has no default value either. It's true that we could write an
+/// `impl` for this which simply panics, but the same is true for any type (we could `impl
+/// Default` for (eg.) `File` by just making `default()` panic.)


URLs for File and default().

GuillaumeGomez · 2017-11-28T09:09:44Z

Text seems good to me (at one exception). However a lot of small nits. :)

canndrew · 2017-11-28T15:22:19Z

Thanks for the feedback. I've added all those missing links.

QuietMisdreavus

Sorry for letting this get stale! I looked over the docs again and i have a couple nits. Once these are settled we can go ahead and merge it!

QuietMisdreavus · 2017-12-05T21:06:42Z

src/libstd/primitive_docs.rs

+/// let x: ! = {
+///     return 123
+/// };
+/// # }


I feel like we should show the feature flag here, since it's needed to give x the proper type.

QuietMisdreavus · 2017-12-05T21:28:45Z

src/libstd/primitive_docs.rs

+/// [`Result<String, !>`] which we can unpack like this:
+///
+/// ```ignore (string-from-str-error-type-is-not-never-yet)
+/// let Ok(s) = String::from_str("hello");


I know these ignore blocks get a little info tooltip and callout border now, but i'd prefer it if we had a comment in here along the lines of "NOTE: This does not work today!", just to sate my paranoia about people blindly copy/pasting examples from docs.

shepmaster · 2017-12-09T15:45:23Z

Heads up from triage @canndrew — you've got some pending feedback to address!

canndrew · 2017-12-10T07:15:00Z

Thanks for the reminder! I've pushed a couple of small fixes.

QuietMisdreavus · 2017-12-10T16:40:09Z

Thanks so much!

@bors r+

bors · 2017-12-10T16:40:10Z

📌 Commit 172f16b has been approved by QuietMisdreavus

bors · 2017-12-10T19:04:31Z

⌛ Testing commit 172f16b with merge 2d4df95...

@nikomatsakis

Add docs for never primitive cc @nikomatsakis, @QuietMisdreavus

bors · 2017-12-10T21:34:57Z

☀️ Test successful - status-appveyor, status-travis
Approved by: QuietMisdreavus
Pushing 2d4df95 to master...

Add docs for never primitive

6612590

canndrew force-pushed the never-docs branch from 2dddf79 to 6612590 Compare November 24, 2017 03:23

QuietMisdreavus reviewed Nov 24, 2017

View reviewed changes

change stability of prim_never

3184520

kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 24, 2017

kennytm assigned QuietMisdreavus Nov 24, 2017

canndrew force-pushed the never-docs branch from 18a4650 to dee6b97 Compare November 24, 2017 07:40

Fix doc tests

bd7d541

canndrew force-pushed the never-docs branch from dee6b97 to bd7d541 Compare November 24, 2017 08:51

canndrew mentioned this pull request Nov 25, 2017

Rename never_type_impls gate #46250

Merged

link to never type docs

afd094a

Havvy approved these changes Nov 28, 2017

View reviewed changes

GuillaumeGomez reviewed Nov 28, 2017

View reviewed changes

Add more links to ! doc text

a2e79a7

QuietMisdreavus reviewed Dec 5, 2017

View reviewed changes

shepmaster added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 9, 2017

Update never_type docs based on feedback

172f16b

kennytm 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 Dec 10, 2017

bors added a commit that referenced this pull request Dec 10, 2017

Auto merge of #46232 - canndrew:never-docs, r=QuietMisdreavus

2d4df95

Add docs for never primitive cc @nikomatsakis, @QuietMisdreavus

bors merged commit 172f16b into rust-lang:master Dec 10, 2017

Add docs for never primitive #46232

Add docs for never primitive #46232

Conversation

canndrew commented Nov 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kennytm commented Nov 24, 2017

canndrew commented Nov 24, 2017

kennytm commented Nov 24, 2017

QuietMisdreavus commented Nov 24, 2017

canndrew commented Nov 25, 2017

est31 commented Nov 25, 2017

est31 commented Nov 25, 2017

canndrew commented Nov 25, 2017

QuietMisdreavus commented Nov 27, 2017

QuietMisdreavus commented Nov 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez Nov 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez Nov 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Nov 28, 2017

canndrew commented Nov 28, 2017

QuietMisdreavus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shepmaster commented Dec 9, 2017

canndrew commented Dec 10, 2017

QuietMisdreavus commented Dec 10, 2017

bors commented Dec 10, 2017

bors commented Dec 10, 2017

bors commented Dec 10, 2017

GuillaumeGomez Nov 28, 2017 •

edited

Loading

GuillaumeGomez Nov 28, 2017 •

edited

Loading