Skip to content

Commit

Permalink
Auto merge of #87073 - jyn514:primitive-docs, r=GuillaumeGomez,jyn514
Browse files Browse the repository at this point in the history
Fix rustdoc handling of primitive items

This is a complicated PR and does a lot of things. I'm willing to split it up a little more if it would help reviewing, but it would be tricky and I'd rather not unless it's necessary.

 ## What does this do?

- Fixes #73423.
- Fixes #79630. I'm not sure how to test this for the standard library explicitly, but you can see from some of the diffs from the `no_std` tests. I also tested it locally and it works correctly: ![image](https://user-images.githubusercontent.com/23638587/125214383-e1fdd000-e284-11eb-8048-76b5df958aad.png)
- Fixes #83083.

## Why are these changes interconnected?

- Allowing anchors (#83083) without fixing the online/offline problem (#79630) will actually just silently discard the anchors, that's not a fix. The online/offline problem is directly related to the fragment hack; links need to go through `fn href()` to be fixed.
- Technically I could fix the online/offline problem without removing the error on anchors; I am willing to separate that out if it would be helpful for reviewing. However I can't fix the anchor problem without adding docs to core, since rustdoc needs all those primitives to have docs to avoid a fallback, and currently `#![no_std]` crates don't have docs for primitives. I also can't fix the online/offline problem without removing the fragment hack, since otherwise diffs like this will be wrong for some primitives but not others:
```diff
`@@` -385,7 +381,7 `@@` fn resolve_primitive_associated_item(
                         ty::AssocKind::Const => "associatedconstant",
                         ty::AssocKind::Type => "associatedtype",
                     };
-                    let fragment = format!("{}#{}.{}", prim_ty.as_sym(), out, item_name);
+                    let fragment = format!("{}.{}", out, item_name);
                     (Res::Primitive(prim_ty), fragment, Some((kind.as_def_kind(), item.def_id)))
                 })
         })
```
- Adding primitive docs to core without making any other change will cause links to go to `core` instead of `std`, even for crates with `extern crate std`. See "Breaking changes to doc(primitive)" below for why this is the case. That said, I could add some special casing to rustdoc at the same time that would let me separate this change from the others (it would fix #73423 but still special-case intra-doc links). I'm willing to separate that out if helpful for reviewing.

### Add primitive documentation to libcore

This works by reusing the same `include!("primitive_docs.rs")` file in both core and std, and then special-casing links in core to use relative links instead of intra-doc links. This doesn't use purely intra-doc links because some of the primitive docs links to items only in std; this doesn't use purely relative links because that introduces new broken links when the docs are re-exported (e.g. String's `&str` deref impl, or Vec's slice deref impl).

Note that this copies the whole file to core, to avoid anyone compiling core to have to set `CARGO_PKG_NAME`. See https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/Who.20should.20review.20changes.20to.20linkchecker.3F/near/249939598 for more context. It also adds a tidy check to make sure the two files are kept in sync.

### Fix inconsistent online/offline primitive docs

This does four things:
- Records modules with `doc(primitive)` in `cache.external_paths`. This is necessary for `href()` to find them later.
- Makes `cache.primitive_locations` available to the intra-doc link pass, by refactoring out a `PrimitiveType::primitive_locations` function that only uses `TyCtxt`.
- Special cases modules with `doc(primitive)` to be treated as always public for the purpose of links.
- Removes the fragment hack. cc `@notriddle,` I know you added some comments about this in the code (thank you for that!)

### Breaking changes to `doc(primitive)`

"Breaking" is a little misleading here - these are changes in behavior, none of them will cause code to fail to compile.

Let me preface this by saying I think stabilizing `doc(primitive)` was a uniquely terrible idea. As far as I can tell, it was stabilized by oversight; it's been stable since 1.0. No one should have need to use it except the standard library, and a crater run shows that in fact no one is using it: #87050 (comment). I hope to actually make `doc(primitive)` a no-op unless you opt-in with a nightly feature, which will keep crates compiling without forcing rustdoc into trying to keep somewhat arbitrary behavior guarantees; but for now, this just subtly changes some of the behavior if you use `doc(primitive)` in a dependency.

That said, here are the changes:
-  Refactoring out `primitive_locations()` is technically a change in behavior, since it no longer looks for primitives in crates that were passed through `--extern`, but not used by the crate; however, that seems like such an unlikely edge case it's not worth dealing with.
- The precedence given to primitive locations is no longer just arbitrary, it can also be inconsistent from run to run. Let me explain that more: previously, primitive locations were sorted by the `CrateNum`; the comment on that sort said "Favor linking to as local extern as possible, so iterate all crates in reverse topological order." Unfortunately, that's not actually what CrateNum tracks: it measures the order crates are loaded, not the number of intermediate crates between that dependency and the root crate. It happened to work as intended before because the compiler injects `extern crate std;` at the top of every crate, which ensured it would have the first CrateNum other than the current, but every other CrateNum was completely arbitrary (for example, `core` often had a later CrateNum than `std`). This now removes the sort on CrateNum completely and special-cases core instead. In particular, if you depend on both `std` and a crate which defines a `doc(primitive)` module, it's arbitrary whether rustdoc will use the docs from std or the ones from the other crate. cc `@alexcrichton,` you wrote this originally.

cc `@rust-lang/rustdoc`
cc `@rust-lang/libs` for the addition to `core` (the commit you're interested in is 91346c8)
  • Loading branch information
bors committed Sep 12, 2021
2 parents 547d937 + 86fd250 commit 0273e3b
Show file tree
Hide file tree
Showing 50 changed files with 1,596 additions and 211 deletions.
1 change: 1 addition & 0 deletions library/core/primitive_docs/box_into_raw.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../std/boxed/struct.Box.html#method.into_raw
1 change: 1 addition & 0 deletions library/core/primitive_docs/fs_file.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../std/fs/struct.File.html
1 change: 1 addition & 0 deletions library/core/primitive_docs/io_bufread.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../std/io/trait.BufRead.html
1 change: 1 addition & 0 deletions library/core/primitive_docs/io_read.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../std/io/trait.Read.html
1 change: 1 addition & 0 deletions library/core/primitive_docs/io_seek.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../std/io/trait.Seek.html
1 change: 1 addition & 0 deletions library/core/primitive_docs/io_write.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../std/io/trait.Write.html
1 change: 1 addition & 0 deletions library/core/primitive_docs/net_tosocketaddrs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../std/net/trait.ToSocketAddrs.html
1 change: 1 addition & 0 deletions library/core/primitive_docs/process_exit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../std/process/fn.exit.html
1 change: 1 addition & 0 deletions library/core/primitive_docs/string_string.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../std/string/struct.String.html
6 changes: 4 additions & 2 deletions library/core/src/bool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

#[lang = "bool"]
impl bool {
/// Returns `Some(t)` if the `bool` is [`true`](keyword.true.html), or `None` otherwise.
/// Returns `Some(t)` if the `bool` is [`true`](../std/keyword.true.html),
/// or `None` otherwise.
///
/// # Examples
///
Expand All @@ -18,7 +19,8 @@ impl bool {
if self { Some(t) } else { None }
}

/// Returns `Some(f())` if the `bool` is [`true`](keyword.true.html), or `None` otherwise.
/// Returns `Some(f())` if the `bool` is [`true`](../std/keyword.true.html),
/// or `None` otherwise.
///
/// # Examples
///
Expand Down
14 changes: 7 additions & 7 deletions library/core/src/char/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ impl char {
/// decoding error.
///
/// It can occur, for example, when giving ill-formed UTF-8 bytes to
/// [`String::from_utf8_lossy`](string/struct.String.html#method.from_utf8_lossy).
/// [`String::from_utf8_lossy`](../std/string/struct.String.html#method.from_utf8_lossy).
#[stable(feature = "assoc_char_consts", since = "1.52.0")]
pub const REPLACEMENT_CHARACTER: char = '\u{FFFD}';

Expand Down Expand Up @@ -96,7 +96,7 @@ impl char {
/// Converts a `u32` to a `char`.
///
/// Note that all `char`s are valid [`u32`]s, and can be cast to one with
/// [`as`](keyword.as.html):
/// [`as`](../std/keyword.as.html):
///
/// ```
/// let c = '💯';
Expand Down Expand Up @@ -372,7 +372,7 @@ impl char {
/// println!("\\u{{2764}}");
/// ```
///
/// Using [`to_string`](string/trait.ToString.html#tymethod.to_string):
/// Using [`to_string`](../std/string/trait.ToString.html#tymethod.to_string):
///
/// ```
/// assert_eq!('❤'.escape_unicode().to_string(), "\\u{2764}");
Expand Down Expand Up @@ -448,7 +448,7 @@ impl char {
/// println!("\\n");
/// ```
///
/// Using [`to_string`](string/trait.ToString.html#tymethod.to_string):
/// Using [`to_string`](../std/string/trait.ToString.html#tymethod.to_string):
///
/// ```
/// assert_eq!('\n'.escape_debug().to_string(), "\\n");
Expand Down Expand Up @@ -502,7 +502,7 @@ impl char {
/// println!("\\\"");
/// ```
///
/// Using [`to_string`](string/trait.ToString.html#tymethod.to_string):
/// Using [`to_string`](../std/string/trait.ToString.html#tymethod.to_string):
///
/// ```
/// assert_eq!('"'.escape_default().to_string(), "\\\"");
Expand Down Expand Up @@ -937,7 +937,7 @@ impl char {
/// println!("i\u{307}");
/// ```
///
/// Using [`to_string`](string/trait.ToString.html#tymethod.to_string):
/// Using [`to_string`](../std/string/trait.ToString.html#tymethod.to_string):
///
/// ```
/// assert_eq!('C'.to_lowercase().to_string(), "c");
Expand Down Expand Up @@ -1002,7 +1002,7 @@ impl char {
/// println!("SS");
/// ```
///
/// Using [`to_string`](string/trait.ToString.html#tymethod.to_string):
/// Using [`to_string`](../std/string/trait.ToString.html#tymethod.to_string):
///
/// ```
/// assert_eq!('c'.to_uppercase().to_string(), "C");
Expand Down
3 changes: 3 additions & 0 deletions library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@
#![feature(decl_macro)]
#![feature(doc_cfg)]
#![feature(doc_notable_trait)]
#![feature(doc_primitive)]
#![feature(exhaustive_patterns)]
#![feature(extern_types)]
#![feature(fundamental)]
Expand Down Expand Up @@ -355,3 +356,5 @@ pub mod arch {
/* compiler built-in */
}
}

include!("primitive_docs.rs");
Loading

0 comments on commit 0273e3b

Please sign in to comment.