Don't run everybody_loops for rustdoc; instead ignore resolution errors by jyn514 · Pull Request #73566 · rust-lang/rust (original) (raw)

r? @eddyb
cc @petrochenkov, @GuillaumeGomez, @Manishearth, @ecstatic-morse, @marmeladema

Blocked on #73743 Merged.
Blocked on crater run. Crater popped up some ICEs (now fixed). See crater run, ICEs.
Blocked on #74070 so that we don't make typeck_tables_of public when it shouldn't be. Merged.

Closes #71820, closes #71104, closes #65863.

What is the motivation for this change?

As seen from a lengthy trail of PRs and issues (#73532, #73103, #71820, #71104), everybody_loops is causing bugs in rustdoc. The main issue is that it does not preserve the validity of the DefId tree, meaning that operations on DefIds may unexpectedly fail when called later. This is blocking intra-doc links (see #73101).

This PR starts by removing everybody_loops, fixing #71104 and #71820. However, that brings back the bugs seen originally in #43348: Since libstd documents items for all platforms, the function bodies sometimes do not type check. Here are the errors from documenting libstd with everybody_loops disabled and no other changes:

error[E0433]: failed to resolve: could not find handle in sys --> src/libstd/sys/windows/ext/process.rs:13:27 | 13 | let handle = sys::handle::Handle::new(handle as *mut _); | ^^^^^^ could not find handle in sys

error[E0425]: cannot find function symlink_inner in module sys::fs --> src/libstd/sys/windows/ext/fs.rs:544:14 | 544 | sys::fs::symlink_inner(src.as_ref(), dst.as_ref(), false) | ^^^^^^^^^^^^^ not found in sys::fs

error[E0425]: cannot find function symlink_inner in module sys::fs --> src/libstd/sys/windows/ext/fs.rs:564:14 | 564 | sys::fs::symlink_inner(src.as_ref(), dst.as_ref(), true) | ^^^^^^^^^^^^^ not found in sys::fs

Why does this need changes to rustc_resolve?

Normally, this could be avoided by simply not calling the typeck_item_bodies pass. However, the errors above happen before type checking, in name resolution itself. Since name resolution is intermingled with macro expansion, and rustdoc needs expansion to happen before it knows all items to be documented, there needs to be someway to ignore resolution errors in function bodies.

An alternative solution suggested by @petrochenkov was to not run everybody_loops on anything containing a nested DefId. This would solve some of the immediate issues, but isn't bullet-proof: the following functions still could not be documented if the items in the body failed to resolve:

This also isn't exactly what rustdoc wants, which is to avoid looking at function bodies in the first place.

What changes were made?

The hack implemented in this PR is to add an option to ignore all resolution errors in function bodies. This is enabled only for rustdoc. Since resolution errors are ignored, the MIR generated will be invalid, as can be seen in the following ICE:

error: internal compiler error: broken MIR in DefId(0:11 ~ doc_cfg[8787]::uses_target_feature[0]) ("return type"): bad type [type error] --> /home/joshua/src/rust/src/test/rustdoc/doc-cfg.rs:51:1 | 51 | / pub unsafe fn uses_target_feature() { 52 | | content::should::be::irrelevant(); 53 | | } | |_^

Fortunately, rustdoc does not need to access MIR in order to generate documentation. Therefore this also removes the call to analyze() in rustdoc::run_core. This has the side effect of not generating all lints by default. Most lints are safe to ignore (does rustdoc really need to run liveness analysis?) but missing_docs in particular is disabled when it should not be. Re-running missing_docs specifically does not help, because it causes the typechecking pass to be run, bringing back the errors from #24658:

error[E0599]: no method named `into_handle` found for struct `sys::unix::pipe::AnonPipe` in the current scope
  --> src/libstd/sys/windows/ext/process.rs:71:27
   |
71 |         self.into_inner().into_handle().into_raw() as *mut _
   |                           ^^^^^^^^^^^ method not found in `sys::unix::pipe::AnonPipe`
   |

Because of #73743, we only run typeck on demand. So this only causes an issue for functions returning impl Trait, which were already special cased by ReplaceFunctionWithBody. However, it now considers async fn f() -> T to be considered impl Future<Output = T>, where before it was considered to have a concrete T type.

How will this affect future changes to rustdoc?

Current status