rustdoc: Some items in cache.paths are non-local

[jyn514]

I tried this code:

diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs
index 1e1fc2436aa..2d3858d7b17 100644
--- a/src/librustdoc/formats/cache.rs
+++ b/src/librustdoc/formats/cache.rs
@@ -3,7 +3,7 @@
 use std::path::Path;
 
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use rustc_hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX};
+use rustc_hir::def_id::{CrateNum, LocalDefId, DefId, CRATE_DEF_INDEX};
 use rustc_middle::middle::privacy::AccessLevels;
 use rustc_middle::ty::TyCtxt;
 use rustc_span::symbol::sym;
@@ -16,6 +16,36 @@
 use crate::html::render::cache::{get_index_search_type, ExternalLocation};
 use crate::html::render::IndexItem;
 
+#[derive(Clone)]
+crate struct AssertLocalMap<V>(FxHashMap<LocalDefId, V>);
+
+impl<V> Default for AssertLocalMap<V> {
+    fn default() -> Self {
+        Self(FxHashMap::default())
+    }
+}
+
+impl<V> AssertLocalMap<V> {
+    crate fn get(&self, k: &DefId) -> Option<&V> {
+        self.0.get(&k.as_local().unwrap())
+    }
+
+    crate fn contains_key(&self, k: &DefId) -> bool {
+        k.as_local().map_or(false, |local| self.0.contains_key(&local))
+    }
+
+    fn insert(&mut self, k: DefId, v: V) -> Option<V> {
+        let local = k.as_local().unwrap_or_else(|| panic!(
+            "{:?} is not local", k
+        ));
+        self.0.insert(local, v)
+    }
+
+    crate fn into_iter(self) -> impl Iterator<Item = (DefId, V)> {
+        self.0.into_iter().map(|(k, v)| (k.to_def_id(), v))
+    }
+}
+
 /// This cache is used to store information about the [`clean::Crate`] being
 /// rendered in order to provide more useful documentation. This contains
 /// information like all implementors of a trait, all traits a type implements,
@@ -40,7 +70,7 @@
     /// URLs when a type is being linked to. External paths are not located in
     /// this map because the `External` type itself has all the information
     /// necessary.
-    crate paths: FxHashMap<DefId, (Vec<String>, ItemType)>,
+    crate paths: AssertLocalMap<(Vec<String>, ItemType)>,
 
     /// Similar to `paths`, but only holds external paths. This is only used for
     /// generating explicit hyperlinks to other crates.

I expected to see this happen: No change in behavior; according to the comment, all items stored in paths are local.

Instead, this happened: Rustdoc panics on nearly every test case in src/test/rustdoc. Here is an example:

---- [rustdoc] rustdoc/intra-doc/trait-item.rs stdout ----

error: rustdoc failed!
status: exit status: 101

------------------------------------------
stderr:
------------------------------------------
thread 'rustc' panicked at 'DefId(2:2796 ~ core[87c3]::convert::{impl#7}::Error) is not local', src/librustdoc/formats/cache.rs:38:52

Why is this bad?

Various parts of rustdoc depend on paths being local. For example,

rust/src/librustdoc/html/format.rs

Lines 486 to 492 in 56dee7c

	let (fqp, shortty, mut url_parts) = match cache.paths.get(&did) {
	Some(&(ref fqp, shortty)) => (fqp, shortty, {
	let module_fqp = to_module_fqp(shortty, fqp);
	href_relative_parts(module_fqp, relative_to)
	}),
	None => {
	let &(ref fqp, shortty) = cache.external_paths.get(&did)?;

only checks external_paths if paths is None. I don't know if this explictly leads to wrong behavior, but it does mean we can't distiguish between "not in extern_paths because it's local" and "not in extern_paths because of a bug".

Here are all the places paths is read (not written):

• rust/src/librustdoc/html/render/context.rs

  Line 233 in 56dee7c

  if let Some(&(ref names, ty)) = self.cache.paths.get(&it.def_id.expect_real()) {

  - this one I think could lead to rustdoc writing to the documentation folders of other crates 😅

• rust/src/librustdoc/formats/cache.rs

  Line 265 in 56dee7c

  let path = match self.cache.paths.get(&did) {

  - not sure of the impact of this one

• rust/src/librustdoc/html/render/mod.rs

  Line 2120 in 56dee7c

  .map_or(false, |d| !cx.cache.paths.contains_key(&d))

  - this will result in some foreign impls not showing up in the sidebar

• rust/src/librustdoc/html/render/write_shared.rs

  Line 491 in 56dee7c

  let &(ref remote_path, remote_item_type) = match cx.cache.paths.get(&did) {

  - no idea, that whole code looks very sketchy

• rust/src/librustdoc/html/render/write_shared.rs

  Line 531 in 56dee7c

  if implementors.is_empty() && !cx.cache.paths.contains_key(&did) {

  - I think this will result in items duplicated in the JS sidebar file and the HTML (cc @jsha, I think you ran into this recently?)

• rust/src/librustdoc/html/render/cache.rs

  Line 36 in 56dee7c

  if let Some(&(ref fqp, _)) = cache.paths.get(&did) {

Meta

HEAD is 0ce2112. I found this while reviewing #86662.

Backtrace

stack backtrace:
   0: rust_begin_unwind
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/std/src/panicking.rs:515:5
   1: std::panicking::begin_panic_fmt
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/std/src/panicking.rs:457:5
   2: rustdoc::formats::cache::AssertLocalMap<V>::insert::{{closure}}
             at ./src/librustdoc/formats/cache.rs:38:52
   3: core::option::Option<T>::unwrap_or_else
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/core/src/option.rs:429:21
   4: rustdoc::formats::cache::AssertLocalMap<V>::insert
             at ./src/librustdoc/formats/cache.rs:38:21
   5: <rustdoc::formats::cache::CacheBuilder as rustdoc::fold::DocFolder>::fold_item
             at ./src/librustdoc/formats/cache.rs:384:25
   6: rustdoc::fold::DocFolder::fold_inner_recur::{{closure}}
             at ./src/librustdoc/fold.rs:46:62
   7: core::iter::adapters::filter_map::filter_map_try_fold::{{closure}}
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/core/src/iter/adapters/filter_map.rs:46:28
   8: core::iter::traits::iterator::Iterator::try_fold
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/core/src/iter/traits/iterator.rs:1997:21
   9: <core::iter::adapters::filter_map::FilterMap<I,F> as core::iter::traits::iterator::Iterator>::try_fold
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/core/src/iter/adapters/filter_map.rs:77:9
  10: <I as alloc::vec::source_iter_marker::SpecInPlaceCollect<T,I>>::collect_in_place
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/alloc/src/vec/source_iter_marker.rs:119:13
  11: alloc::vec::source_iter_marker::<impl alloc::vec::spec_from_iter::SpecFromIter<T,I> for alloc::vec::Vec<T>>::from_iter
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/alloc/src/vec/source_iter_marker.rs:55:19
  12: <alloc::vec::Vec<T> as core::iter::traits::collect::FromIterator<T>>::from_iter
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/alloc/src/vec/mod.rs:2461:9
  13: core::iter::traits::iterator::Iterator::collect
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/core/src/iter/traits/iterator.rs:1748:9
  14: rustdoc::fold::DocFolder::fold_inner_recur
             at ./src/librustdoc/fold.rs:46:27
  15: rustdoc::fold::DocFolder::fold_item_recur
             at ./src/librustdoc/fold.rs:70:18
  16: <rustdoc::formats::cache::CacheBuilder as rustdoc::fold::DocFolder>::fold_item
             at ./src/librustdoc/formats/cache.rs:463:20
  17: rustdoc::fold::DocFolder::fold_mod::{{closure}}
             at ./src/librustdoc/fold.rs:78:55
  18: core::iter::adapters::filter_map::filter_map_try_fold::{{closure}}
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/core/src/iter/adapters/filter_map.rs:46:28
  19: core::iter::traits::iterator::Iterator::try_fold
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/core/src/iter/traits/iterator.rs:1997:21
  20: <core::iter::adapters::filter_map::FilterMap<I,F> as core::iter::traits::iterator::Iterator>::try_fold
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/core/src/iter/adapters/filter_map.rs:77:9
  21: <I as alloc::vec::source_iter_marker::SpecInPlaceCollect<T,I>>::collect_in_place
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/alloc/src/vec/source_iter_marker.rs:119:13
  22: alloc::vec::source_iter_marker::<impl alloc::vec::spec_from_iter::SpecFromIter<T,I> for alloc::vec::Vec<T>>::from_iter
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/alloc/src/vec/source_iter_marker.rs:55:19
  23: <alloc::vec::Vec<T> as core::iter::traits::collect::FromIterator<T>>::from_iter
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/alloc/src/vec/mod.rs:2461:9
  24: core::iter::traits::iterator::Iterator::collect
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/library/core/src/iter/traits/iterator.rs:1748:9
  25: rustdoc::fold::DocFolder::fold_mod
             at ./src/librustdoc/fold.rs:78:20
  26: rustdoc::fold::DocFolder::fold_inner_recur
             at ./src/librustdoc/fold.rs:19:41
  27: rustdoc::fold::DocFolder::fold_item_recur
             at ./src/librustdoc/fold.rs:70:18
  28: <rustdoc::formats::cache::CacheBuilder as rustdoc::fold::DocFolder>::fold_item
             at ./src/librustdoc/formats/cache.rs:463:20
  29: rustdoc::fold::DocFolder::fold_crate
             at ./src/librustdoc/fold.rs:83:20
  30: rustdoc::formats::cache::Cache::populate
             at ./src/librustdoc/formats/cache.rs:205:17
  31: rustdoc::core::run_global_ctxt::{{closure}}
             at ./src/librustdoc/core.rs:514:9
  32: rustc_data_structures::profiling::VerboseTimingGuard::run
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_data_structures/src/profiling.rs:573:9
  33: rustc_session::utils::<impl rustc_session::session::Session>::time
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_session/src/utils.rs:16:9
  34: rustdoc::core::run_global_ctxt
             at ./src/librustdoc/core.rs:513:13
  35: rustdoc::main_options::{{closure}}::{{closure}}::{{closure}}::{{closure}}
             at ./src/librustdoc/lib.rs:744:21
  36: rustc_data_structures::profiling::VerboseTimingGuard::run
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_data_structures/src/profiling.rs:573:9
  37: rustc_session::utils::<impl rustc_session::session::Session>::time
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_session/src/utils.rs:16:9
  38: rustdoc::main_options::{{closure}}::{{closure}}::{{closure}}
             at ./src/librustdoc/lib.rs:743:55
  39: rustc_interface::passes::QueryContext::enter::{{closure}}
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_interface/src/passes.rs:799:42
  40: rustc_middle::ty::context::tls::enter_context::{{closure}}
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_middle/src/ty/context.rs:1740:50
  41: rustc_middle::ty::context::tls::set_tlv
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_middle/src/ty/context.rs:1724:9
  42: rustc_middle::ty::context::tls::enter_context
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_middle/src/ty/context.rs:1740:9
  43: rustc_interface::passes::QueryContext::enter
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_interface/src/passes.rs:799:9
  44: rustdoc::main_options::{{closure}}::{{closure}}
             at ./src/librustdoc/lib.rs:742:13
  45: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_interface/src/queries.rs:431:19
  46: rustdoc::main_options::{{closure}}
             at ./src/librustdoc/lib.rs:722:9
  47: rustc_interface::interface::create_compiler_and_run::{{closure}}
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_interface/src/interface.rs:208:13
  48: rustc_span::with_source_map
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_span/src/lib.rs:873:5
  49: rustc_interface::interface::create_compiler_and_run
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_interface/src/interface.rs:202:5
  50: rustdoc::main_options
             at ./src/librustdoc/lib.rs:721:5
  51: rustdoc::main_args::{{closure}}
             at ./src/librustdoc/lib.rs:649:17
  52: rustc_interface::util::setup_callbacks_and_run_in_thread_pool_with_globals::{{closure}}::{{closure}}
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_interface/src/util.rs:158:13
  53: scoped_tls::ScopedKey<T>::set
             at /cargo/registry/src/github.com-1ecc6299db9ec823/scoped-tls-1.0.0/src/lib.rs:137:9
  54: rustc_span::with_session_globals
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_span/src/lib.rs:104:5
  55: rustc_interface::util::setup_callbacks_and_run_in_thread_pool_with_globals::{{closure}}
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_interface/src/util.rs:156:9
  56: rustc_interface::util::scoped_thread::{{closure}}
             at /rustc/0ce21126b295608ab4704f678d53ae8d2e9b60d1/compiler/rustc_interface/src/util.rs:131:24
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.55.0-nightly (0ce21126b 2021-06-29) running on x86_64-unknown-linux-gnu

query stack during panic:
end of query stack

[jyn514]

Fortunately this is only written in a couple places, so hopefully it will be easy to fix ????????

> rg '\.paths' src/librustdoc/ -A1 | grep insert
src/librustdoc/formats/cache.rs:                        self.cache.paths.insert(
src/librustdoc/formats/cache.rs-                    .insert(item.def_id.expect_real(), (self.cache.stack.clone(), item.type_()));

[jyn514]

yeah I have no idea why I thought this would be simple, adding a if def_id.is_local() check to the first insert fails a bunch of tests:

---- [rustdoc] rustdoc/intra-doc/trait-item.rs stdout ----
stderr:
------------------------------------------
6: @has check failed
	`XPATH PATTERN` did not match
	// @has - '//*[@href="{{channel}}/core/default/trait.Default.html#tymethod.default"]' 'Default::default()'

[jyn514]

I think a more correct fix is to unify paths and extern_paths, and check def_id.is_local() explicitly for places that need to care about it.

[Stupremee]

@rustbot claim