Fix intra-doc links for cross-crate re-exports of default trait methods

[jyn514]

The original fix for this was very simple: #58972 ignored extern_traits because before #65983 was fixed, they would always fail to resolve, giving spurious warnings. So the first commit just undoes that change, so extern traits are now seen by the collect_intra_doc_links pass. There are also some minor changes in librustdoc/fold.rs to avoid borrowing the extern_traits RefCell more than once at a time.

However, that brought up a much more thorny problem. rustc_resolve started giving 'error: cannot find a built-in macro with name cfg' when documenting libproc_macro (I still haven't been able to reproduce on anything smaller than the full standard library). The chain of events looked like this (thanks @eddyb for the help debugging!):

0. x.py build --stage 1 builds the standard library and creates a sysroot
1. cargo doc does something like cargo check to create rmetas for all the crates (unrelated to what was built above)
2. the cargo check-like libcore-*.rmeta is loaded as a transitive dependency and claims ownership of builtin macros
3. rustdoc later tries to resolve some path in a doc link
4. suggestion logic fires and loads "extern prelude" crates by name
5. the sysroot libcore-*.rlib is loaded and fails to claim ownership of builtin macros

rustc_resolve gives the error after step 5. However, rustdoc doesn't need suggestions at all - resolve_str_path_error completely discards the ResolutionError! The fix implemented in this PR is to skip the suggestion logic for resolve_ast_path: pass record_used: false and skip lookup_import_candidates when record_used isn't set.

It's possible that if/when #74207 is implemented this will need a more in-depth fix which returns a ResolutionError from compile_macro, to allow rustdoc to reuse the suggestions from rustc_resolve. However, that's a much larger change and there's no need for it yet, so I haven't implemented it here.

Fixes #73829.

r? @GuillaumeGomez

[GuillaumeGomez]

error: cannot find a built-in macro with name `cfg`

A strange error...

[jyn514]

I think this is because of the change to record_extern_trait, I'll try extending the map instead of replacing it altogether.

[jyn514]

Update: that did not help :( Maybe the error shows up because rustdoc is doing more work than before and ran into some edge case from #73566?

[jyn514]

Backtrace

 Documenting proc_macro v0.0.0 (/home/joshua/rustc/library/proc_macro)
error: cannot find a built-in macro with name `cfg`
    --> /home/joshua/rustc/library/core/src/macros/mod.rs:1183:5
     |
1183 | /     macro_rules! cfg {
1184 | |         ($($cfg:tt)*) => {
1185 | |             /* compiler built-in */
1186 | |         };
1187 | |     }
     | |_____^

thread 'rustc' panicked at 'aborting due to `-Z treat-err-as-bug=1`', src/librustc_errors/lib.rs:941:13
stack backtrace:
   0: std::backtrace_rs::backtrace::libunwind::trace
             at ./library/std/src/../../backtrace/src/backtrace/libunwind.rs:96
   1: std::backtrace_rs::backtrace::trace_unsynchronized
             at ./library/std/src/../../backtrace/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at ./library/std/src/sys_common/backtrace.rs:77
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at ./library/std/src/sys_common/backtrace.rs:58
   4: core::fmt::write
             at ./library/core/src/fmt/mod.rs:1117
   5: std::io::Write::write_fmt
             at ./library/std/src/io/mod.rs:1510
   6: std::sys_common::backtrace::_print
             at ./library/std/src/sys_common/backtrace.rs:61
   7: std::sys_common::backtrace::print
             at ./library/std/src/sys_common/backtrace.rs:48
   8: std::panicking::default_hook::{{closure}}
             at ./library/std/src/panicking.rs:198
   9: std::panicking::default_hook
             at ./library/std/src/panicking.rs:217
  10: <alloc::boxed::Box<F> as core::ops::function::Fn<A>>::call
             at ./library/alloc/src/boxed.rs:1044
  11: rustc_driver::report_ice
             at ./src/librustc_driver/lib.rs:1158
  12: std::panicking::rust_panic_with_hook
             at ./library/std/src/panicking.rs:530
  13: std::panicking::begin_panic
             at ./library/std/src/panicking.rs:456
  14: rustc_errors::HandlerInner::panic_if_treat_err_as_bug
             at ./src/librustc_errors/lib.rs:941
  15: rustc_errors::HandlerInner::bump_err_count
             at ./src/librustc_errors/lib.rs:927
  16: rustc_errors::HandlerInner::emit_diagnostic
             at ./src/librustc_errors/lib.rs:767
  17: rustc_errors::Handler::emit_diag_at_span
             at ./src/librustc_errors/lib.rs:706
  18: rustc_errors::Handler::span_err
             at ./src/librustc_errors/lib.rs:605
  19: rustc_session::session::Session::span_err
             at ./src/librustc_session/session.rs:381
  20: rustc_resolve::macros::<impl rustc_resolve::Resolver>::compile_macro
             at ./src/librustc_resolve/macros.rs:1085
  21: rustc_resolve::build_reduced_graph::<impl rustc_resolve::Resolver>::get_macro_by_def_id
             at ./src/librustc_resolve/build_reduced_graph.rs:167
  22: rustc_resolve::build_reduced_graph::<impl rustc_resolve::Resolver>::get_macro
             at ./src/librustc_resolve/build_reduced_graph.rs:155
  23: rustc_resolve::macros::<impl rustc_resolve::Resolver>::check_reserved_macro_name
             at ./src/librustc_resolve/macros.rs:1057
  24: rustc_resolve::imports::<impl rustc_resolve::Resolver>::try_define
             at ./src/librustc_resolve/imports.rs:494
  25: rustc_resolve::build_reduced_graph::<impl rustc_resolve::Resolver>::define
             at ./src/librustc_resolve/build_reduced_graph.rs:93
  26: rustc_resolve::build_reduced_graph::BuildReducedGraphVisitor::build_reduced_graph_for_external_crate_res
  27: rustc_resolve::build_reduced_graph::<impl rustc_resolve::Resolver>::build_reduced_graph_external
             at ./src/librustc_resolve/build_reduced_graph.rs:190
  28: rustc_resolve::Resolver::resolutions
             at ./src/librustc_resolve/lib.rs:1487
  29: rustc_resolve::ModuleData::for_each_child
             at ./src/librustc_resolve/lib.rs:514
  30: rustc_resolve::diagnostics::<impl rustc_resolve::Resolver>::lookup_import_candidates_from_module
             at ./src/librustc_resolve/diagnostics.rs:738
  31: rustc_resolve::diagnostics::<impl rustc_resolve::Resolver>::lookup_import_candidates
             at ./src/librustc_resolve/diagnostics.rs:876
  32: rustc_resolve::Resolver::resolve_path_with_ribs
             at ./src/librustc_resolve/lib.rs:2331
  33: rustc_resolve::Resolver::resolve_path
             at ./src/librustc_resolve/lib.rs:2081
  34: rustc_resolve::Resolver::resolve_ast_path
             at ./src/librustc_resolve/lib.rs:3091
  35: rustc_resolve::Resolver::resolve_str_path_error
             at ./src/librustc_resolve/lib.rs:3080
  36: rustdoc::passes::collect_intra_doc_links::LinkCollector::macro_resolve::{{closure}}
             at ./src/librustdoc/passes/collect_intra_doc_links.rs:158
  37: rustc_interface::passes::BoxedResolver::access::{{closure}}
             at ./src/librustc_data_structures/box_region.rs:113
  38: rustc_interface::passes::configure_and_expand::{{closure}}
             at ./src/librustc_interface/passes.rs:135
  39: alloc::boxed::<impl core::ops::generator::Generator<R> for core::pin::Pin<alloc::boxed::Box<G>>>::resume
             at ./library/alloc/src/boxed.rs:1145
  40: rustc_data_structures::box_region::PinnedGenerator<I,A,R>::access
             at ./src/librustc_data_structures/box_region.rs:55
  41: rustc_interface::passes::BoxedResolver::access
             at ./src/librustc_data_structures/box_region.rs:119
  42: rustdoc::core::DocContext::enter_resolver
             at ./src/librustdoc/core.rs:83
  43: rustdoc::passes::collect_intra_doc_links::LinkCollector::macro_resolve
             at ./src/librustdoc/passes/collect_intra_doc_links.rs:141
  44: <rustdoc::passes::collect_intra_doc_links::LinkCollector as rustdoc::fold::DocFolder>::fold_item
             at ./src/librustdoc/passes/collect_intra_doc_links.rs:699
  45: rustdoc::fold::DocFolder::fold_inner_recur::{{closure}}
             at ./src/librustdoc/fold.rs:61
  46: core::ops::function::impls::<impl core::ops::function::FnMut<A> for &mut F>::call_mut
             at ./library/core/src/ops/function.rs:275
  47: core::iter::traits::iterator::Iterator::find_map::check::{{closure}}
             at ./library/core/src/iter/traits/iterator.rs:2238
  48: core::iter::traits::iterator::Iterator::try_fold
             at ./library/core/src/iter/traits/iterator.rs:1870
  49: core::iter::traits::iterator::Iterator::find_map
             at ./library/core/src/iter/traits/iterator.rs:2244
  50: <core::iter::adapters::FilterMap<I,F> as core::iter::traits::iterator::Iterator>::next
             at ./library/core/src/iter/adapters/mod.rs:1132
  51: alloc::vec::Vec<T>::extend_desugared
             at ./library/alloc/src/vec.rs:2254
  52: <alloc::vec::Vec<T> as alloc::vec::SpecExtend<T,I>>::spec_extend
             at ./library/alloc/src/vec.rs:2147
  53: <alloc::vec::Vec<T> as alloc::vec::SpecExtend<T,I>>::from_iter
             at ./library/alloc/src/vec.rs:2142
  54: <alloc::vec::Vec<T> as core::iter::traits::collect::FromIterator<T>>::from_iter
             at ./library/alloc/src/vec.rs:2032
  55: core::iter::traits::iterator::Iterator::collect
             at ./library/core/src/iter/traits/iterator.rs:1652
  56: rustdoc::fold::DocFolder::fold_crate
             at ./src/librustdoc/fold.rs:98
  57: rustdoc::passes::collect_intra_doc_links::collect_intra_doc_links
             at ./src/librustdoc/passes/collect_intra_doc_links.rs:42
  58: rustdoc::core::run_core::{{closure}}::{{closure}}::{{closure}}
             at ./src/librustdoc/core.rs:601
  59: rustc_middle::ty::context::tls::enter_global::{{closure}}
             at ./src/librustc_middle/ty/context.rs:1743
  60: rustc_middle::ty::context::tls::enter_context::{{closure}}
             at ./src/librustc_middle/ty/context.rs:1720
  61: rustc_middle::ty::context::tls::set_tlv
             at ./src/librustc_middle/ty/context.rs:1704
  62: rustc_middle::ty::context::tls::enter_context
             at ./src/librustc_middle/ty/context.rs:1720
  63: rustc_middle::ty::context::tls::enter_global
             at ./src/librustc_middle/ty/context.rs:1743
  64: rustc_interface::passes::QueryContext::enter
             at ./src/librustc_interface/passes.rs:755
  65: rustdoc::core::run_core::{{closure}}::{{closure}}
             at ./src/librustdoc/core.rs:451
  66: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
             at ./src/librustc_interface/queries.rs:385
  67: rustdoc::core::run_core::{{closure}}
             at ./src/librustdoc/core.rs:413
  68: rustc_interface::interface::create_compiler_and_run::{{closure}}
             at ./src/librustc_interface/interface.rs:192
  69: rustc_span::with_source_map
             at ./src/librustc_span/lib.rs:736
  70: rustc_interface::interface::create_compiler_and_run
             at ./src/librustc_interface/interface.rs:186
  71: rustdoc::core::run_core
             at ./src/librustdoc/core.rs:412
  72: rustdoc::main_options::{{closure}}
             at ./src/librustdoc/lib.rs:507
  73: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at ./library/std/src/panic.rs:318
  74: std::panicking::try::do_call
             at ./library/std/src/panicking.rs:348
  75: std::panicking::try
             at ./library/std/src/panicking.rs:325
  76: std::panic::catch_unwind
             at ./library/std/src/panic.rs:394
  77: rustc_driver::catch_fatal_errors
             at ./src/librustc_driver/lib.rs:1123
  78: rustdoc::main_options
             at ./src/librustdoc/lib.rs:503
  79: rustdoc::main_args::{{closure}}
             at ./src/librustdoc/lib.rs:438
  80: rustc_interface::util::setup_callbacks_and_run_in_thread_pool_with_globals::{{closure}}::{{closure}}::{{closure}}
             at ./src/librustc_interface/util.rs:151
  81: scoped_tls::ScopedKey<T>::set
             at /home/joshua/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/scoped-tls-1.0.0/src/lib.rs:137
  82: rustc_interface::util::setup_callbacks_and_run_in_thread_pool_with_globals::{{closure}}::{{closure}}
             at ./src/librustc_interface/util.rs:147
  83: scoped_tls::ScopedKey<T>::set
             at /home/joshua/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/scoped-tls-1.0.0/src/lib.rs:137
  84: rustc_ast::attr::with_session_globals::{{closure}}
             at ./src/librustc_ast/attr/mod.rs:49
  85: scoped_tls::ScopedKey<T>::set
             at /home/joshua/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/scoped-tls-1.0.0/src/lib.rs:137
  86: rustc_ast::attr::with_session_globals
             at ./src/librustc_ast/attr/mod.rs:48
  87: rustc_interface::util::setup_callbacks_and_run_in_thread_pool_with_globals::{{closure}}
             at ./src/librustc_interface/util.rs:146
  88: rustc_interface::util::scoped_thread::{{closure}}
             at ./src/librustc_interface/util.rs:121
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

edit: so I remember next time, the way to get this backtrace is with RUSTFLAGS_NOT_BOOTSTRAP='-Z treat-err-as-bug'.
edit: more helpful commands: RUST_BACKTRACE=1 RUSTDOC_LOG=rustc_metadata::creader=debug,rustdoc::passes::collect=debug RUSTFLAGS_NOT_BOOTSTRAP='-Z treat-err-as-bug' xpy doc --stage 1 library/std --keep-stage 1

[jyn514]

The error comes from here:

rust/src/librustc_resolve/macros.rs

Lines 1077 to 1087 in e539dd6

	if result.is_builtin {
	// The macro was marked with `#[rustc_builtin_macro]`.
	if let Some(ext) = self.builtin_macros.remove(&item.ident.name) {
	// The macro is a built-in, replace its expander function
	// while still taking everything else from the source code.
	result.kind = ext.kind;
	} else {
	let msg = format!("cannot find a built-in macro with name `{}`", item.ident);
	self.session.span_err(item.span, &msg);
	}
	}

Note that it uses builtin_macros.remove instead of .get. So I think it expects to only ever be called once. So I think we need to find out why it was called more than once.

There are a bunch of other questions that pop up, like

• Why does this only happen when documenting proc_macro?
• How is this possibly affected by my change?

[jyn514]

I think this might be because we clone the Resolver and it gets confused whether or not it's seen the cfg already. Maybe the DefIds are changing somehow?

[jyn514]

Just trying to print the DefId causes a panic.

diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index 11c7793b3ad..afce46395a9 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -164,7 +164,10 @@ impl<'a> Resolver<'a> {
         }
 
         let ext = Lrc::new(match self.cstore().load_macro_untracked(def_id, &self.session) {
-            LoadedMacro::MacroDef(item, edition) => self.compile_macro(&item, edition),
+            LoadedMacro::MacroDef(item, edition) => {
+                debug!("loading macro {} with DefId {:?}", item.ident.name, def_id);
+                self.compile_macro(&item, edition)
+            }
             LoadedMacro::ProcMacro(ext) => ext,
         });

2020-08-09T04:55:57.741630Z DEBUG rustc_resolve::build_reduced_graph: loading macro write with DefId DefId(2:11 ~ core[8913]::macros[0]::write[0])
thread 'rustc' panicked at 'index out of bounds: the len is 18 but the index is 18', src/librustc_metadata/creader.rs:128:21

[jyn514]

Just trying to print the DefId causes a panic.

This is the same panic as from #66159

[jyn514]

#66159 (comment)

My gut feeling is that we probably need to stop capturing/cloning the resolver as we do here: https://github.com/rust-lang/rust/blob/master/src/librustdoc/core.rs#L346 and instead try and find a resolver as part of the closure here: https://github.com/rust-lang/rust/blob/master/src/librustdoc/core.rs#L354

Of course, it looks like a TyCtxt has an immutable copy of the resolver's outputs, and no access to the resolver itself, urgh, and we have no easy way to inject changes into the context at this point.

@kinnison how hard do you think it would be to add the whole resolver to the TyCtxt? Or should I ask someone from T-compiler?

[Mark-Simulacrum]

@petrochenkov is the person to ask, but I suspect the answer is no.

[jyn514]

It turns out this is a known issue. #68427

[jyn514]

Current status: I still think this is because of cloning the resolver somehow, but I haven't figured out exactly why it's giving an error. So there might be some hack I could pile on top of #66211 that doesn't require rewriting half of rustdoc. I want to figure out the current issue before I go chasing too many windmills.

[petrochenkov]

r=me after removing all the added debugging stuff (at least in resolve)

[jyn514]

I removed the debugging in resolve and rebased it out so it doesn't show up in the git history. @eddyb let me know if you don't like the debug logging in metadata and I can make a follow-up PR removing it.

@bors r=GuillaumeGomez,petrochenkov

[bors]

📌 Commit 9131d23 has been approved by GuillaumeGomez,petrochenkov

[bors]

⌛ Testing commit 9131d23 with merge db534b3...

[jyn514]

The 'check' failure is spurious:

 warning: spurious network error (2 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/pretty_assertions/0.6.1/download`, got 502
warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/pretty_assertions/0.6.1/download`, got 502
error: failed to download from `https://crates.io/api/v1/crates/pretty_assertions/0.6.1/download`

Caused by:
  failed to get 200 response from `https://crates.io/api/v1/crates/pretty_assertions/0.6.1/download`, got 502

Hopefully bors succeeds.

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: GuillaumeGomez,petrochenkov
Pushing db534b3 to master...

[jyn514]

This change causes the same error:

diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs
index 39b0ca63301..e97d77a1eb2 100644
--- a/library/std/src/lib.rs
+++ b/library/std/src/lib.rs
@@ -338,6 +338,8 @@
 #[allow(unused)]
 use prelude::v1::*;
 
+extern crate self as std;
+
 // Access to Bencher, etc.
 #[cfg(test)]
 extern crate test;

The use case for this is allowing std::x in intra-doc links for the standard library itself (#73445).

Backtrace

thread 'rustc' panicked at 'aborting due to `-Z treat-err-as-bug=1`', compiler/rustc_errors/src/lib.rs:990:27
stack backtrace:
   0: std::panicking::begin_panic
   1: rustc_errors::HandlerInner::emit_diagnostic
   2: rustc_errors::diagnostic_builder::DiagnosticBuilder::emit
   3: rustc_resolve::macros::<impl rustc_resolve::Resolver>::compile_macro
   4: rustc_resolve::build_reduced_graph::<impl rustc_resolve::Resolver>::get_macro_by_def_id
   5: rustc_resolve::macros::<impl rustc_resolve::Resolver>::check_reserved_macro_name
   6: rustc_resolve::imports::<impl rustc_resolve::Resolver>::try_define
   7: rustc_resolve::build_reduced_graph::<impl rustc_resolve::Resolver>::define
   8: rustc_resolve::build_reduced_graph::BuildReducedGraphVisitor::build_reduced_graph_for_external_crate_res
   9: rustc_resolve::Resolver::resolutions
  10: rustc_resolve::diagnostics::<impl rustc_resolve::Resolver>::lookup_import_candidates_from_module
  11: rustc_resolve::diagnostics::<impl rustc_resolve::Resolver>::lookup_import_candidates
  12: rustc_resolve::late::diagnostics::<impl rustc_resolve::late::LateResolutionVisitor>::smart_resolve_report_errors
  13: rustc_resolve::late::LateResolutionVisitor::smart_resolve_path_fragment
  14: rustc_resolve::late::LateResolutionVisitor::smart_resolve_path
  15: rustc_resolve::late::LateResolutionVisitor::resolve_expr
  16: rustc_resolve::late::LateResolutionVisitor::resolve_expr
  17: <rustc_resolve::late::LateResolutionVisitor as rustc_ast::visit::Visitor>::visit_local
  18: <rustc_resolve::late::LateResolutionVisitor as rustc_ast::visit::Visitor>::visit_block
  19: <rustc_resolve::late::LateResolutionVisitor as rustc_ast::visit::Visitor>::visit_fn
  20: rustc_ast::visit::walk_assoc_item
  21: rustc_resolve::late::LateResolutionVisitor::with_generic_param_rib
  22: rustc_resolve::late::LateResolutionVisitor::resolve_item
  23: rustc_ast::visit::walk_item
  24: rustc_resolve::late::LateResolutionVisitor::resolve_item
  25: rustc_ast::visit::walk_item
  26: rustc_resolve::late::LateResolutionVisitor::resolve_item
  27: rustc_ast::visit::walk_item
  28: rustc_resolve::late::LateResolutionVisitor::resolve_item
  29: rustc_resolve::Resolver::resolve_crate
  30: rustc_interface::passes::configure_and_expand_inner
  31: rustc_interface::passes::configure_and_expand::{{closure}}
  32: rustc_data_structures::box_region::PinnedGenerator<I,A,R>::new
  33: rustc_interface::passes::configure_and_expand
  34: rustc_interface::queries::Queries::expansion
  35: rustc_interface::interface::create_compiler_and_run
  36: rustdoc::core::run_core
  37: rustdoc::main_options
  38: scoped_tls::ScopedKey<T>::set

Note that this happens all the way in configure_and_expand, not in intra-doc links, so it's not related to import suggestions. Maybe it is actually related to #68427 this time?