Skip to content

Commit

Permalink
Rollup merge of rust-lang#87073 - jyn514:primitive-docs, r=GuillaumeG…
Browse files Browse the repository at this point in the history
…omez

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 rust-lang#73423.
- Fixes rust-lang#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 rust-lang#83083.

## Why are these changes interconnected?

- Allowing anchors (rust-lang#83083) without fixing the online/offline problem (rust-lang#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 rust-lang#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).

### 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: rust-lang#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 rust-lang@36729b0)
  • Loading branch information
GuillaumeGomez authored Aug 26, 2021
2 parents 3b3ce37 + 7e435ce commit e948f31
Show file tree
Hide file tree
Showing 49 changed files with 1,587 additions and 188 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
2 changes: 1 addition & 1 deletion 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
3 changes: 3 additions & 0 deletions library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@
#![feature(decl_macro)]
#![feature(doc_cfg)]
#![feature(doc_notable_trait)]
#![cfg_attr(not(bootstrap), feature(doc_primitive))]
#![feature(exhaustive_patterns)]
#![feature(extern_types)]
#![feature(fundamental)]
Expand Down Expand Up @@ -357,3 +358,5 @@ pub mod arch {
/* compiler built-in */
}
}

include!("primitive_docs.rs");
Loading

0 comments on commit e948f31

Please sign in to comment.