From 868927fefb2f6ea62783339552bbb67f828190e1 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 5 Aug 2020 00:11:57 -0400 Subject: [PATCH 1/3] rustdoc: Fix intra-doc links for cross-crate re-exports of traits #58972 ignored extern_traits because before #65983 was fixed, they would always fail to resolve, giving spurious warnings. This 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. --- src/librustdoc/clean/inline.rs | 4 +++- src/librustdoc/fold.rs | 14 +++++--------- src/librustdoc/passes/collect_intra_doc_links.rs | 9 --------- src/test/rustdoc/intra-doc-crate/traits.rs | 2 -- 4 files changed, 8 insertions(+), 21 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 38fa8a402c4..50cb987cf08 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -628,7 +628,9 @@ pub fn record_extern_trait(cx: &DocContext<'_>, did: DefId) { } } - cx.active_extern_traits.borrow_mut().insert(did); + { + cx.active_extern_traits.borrow_mut().insert(did); + } debug!("record_extern_trait: {:?}", did); let trait_ = build_external_trait(cx, did); diff --git a/src/librustdoc/fold.rs b/src/librustdoc/fold.rs index 0a85cb1d5a6..d4ada3278e6 100644 --- a/src/librustdoc/fold.rs +++ b/src/librustdoc/fold.rs @@ -93,15 +93,11 @@ pub trait DocFolder: Sized { c.module = c.module.take().and_then(|module| self.fold_item(module)); { - let mut guard = c.external_traits.borrow_mut(); - let external_traits = std::mem::replace(&mut *guard, Default::default()); - *guard = external_traits - .into_iter() - .map(|(k, mut v)| { - v.items = v.items.into_iter().filter_map(|i| self.fold_item(i)).collect(); - (k, v) - }) - .collect(); + let external_traits = { std::mem::take(&mut *c.external_traits.borrow_mut()) }; + for (k, mut v) in external_traits { + v.items = v.items.into_iter().filter_map(|i| self.fold_item(i)).collect(); + c.external_traits.borrow_mut().insert(k, v); + } } c } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 65d116b9c67..0ef48e7d51b 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -971,15 +971,6 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { self.fold_item_recur(item) } } - - // FIXME: if we can resolve intra-doc links from other crates, we can use the stock - // `fold_crate`, but until then we should avoid scanning `krate.external_traits` since those - // will never resolve properly - fn fold_crate(&mut self, mut c: Crate) -> Crate { - c.module = c.module.take().and_then(|module| self.fold_item(module)); - - c - } } #[derive(Copy, Clone, Debug, PartialEq, Eq)] diff --git a/src/test/rustdoc/intra-doc-crate/traits.rs b/src/test/rustdoc/intra-doc-crate/traits.rs index 07f9fb63313..07decb48019 100644 --- a/src/test/rustdoc/intra-doc-crate/traits.rs +++ b/src/test/rustdoc/intra-doc-crate/traits.rs @@ -1,5 +1,3 @@ -// ignore-test -// ^ this is https://github.com/rust-lang/rust/issues/73829 // aux-build:traits.rs // build-aux-docs // ignore-tidy-line-length From a1c71a17090cd5ef732d852ee878114375ff9254 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 8 Aug 2020 23:47:07 -0400 Subject: [PATCH 2/3] rustdoc,metadata: Debugging --- src/librustc_metadata/creader.rs | 11 ++++++++++- src/librustdoc/core.rs | 1 + src/librustdoc/passes/collect_intra_doc_links.rs | 1 + 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index f8446d83d2b..7562da6d782 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -222,6 +222,7 @@ impl<'a> CrateLoader<'a> { let mut ret = None; self.cstore.iter_crate_data(|cnum, data| { if data.name() != name { + tracing::trace!("{} did not match {}", data.name(), name); return; } @@ -230,7 +231,10 @@ impl<'a> CrateLoader<'a> { ret = Some(cnum); return; } - Some(..) => return, + Some(hash) => { + debug!("actual hash {} did not match expected {}", hash, data.hash()); + return; + } None => {} } @@ -273,6 +277,11 @@ impl<'a> CrateLoader<'a> { .1; if kind.matches(prev_kind) { ret = Some(cnum); + } else { + debug!( + "failed to load existing crate {}; kind {:?} did not match prev_kind {:?}", + name, kind, prev_kind + ); } }); ret diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 89b217dc7d4..7c89b38a92c 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -439,6 +439,7 @@ pub fn run_core( resolver.borrow_mut().access(|resolver| { sess.time("load_extern_crates", || { for extern_name in &extern_names { + debug!("loading extern crate {}", extern_name); resolver .resolve_str_path_error( DUMMY_SP, diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 0ef48e7d51b..fd5b8997d10 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -161,6 +161,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { return Some(res.map_id(|_| panic!("unexpected id"))); } if let Some(module_id) = parent_id { + debug!("resolving {} as a macro in the module {:?}", path_str, module_id); if let Ok((_, res)) = resolver.resolve_str_path_error(DUMMY_SP, path_str, MacroNS, module_id) { From 9131d23cc0fb5461050bc19e40a3858b61487069 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 29 Aug 2020 09:47:39 -0400 Subject: [PATCH 3/3] resolve: Don't speculatively load crates if this is a speculative resolution This avoids a rare rustdoc bug where loading `core` twice caused a 'cannot find a built-in macro' error: 1. `x.py build --stage 1` builds the standard library and creates a sysroot 2. `cargo doc` does something like `cargo check` to create `rmeta`s for all the crates (unrelated to what was built above) 3. the `cargo check`-like `libcore-*.rmeta` is loaded as a transitive dependency *and claims ownership* of builtin macros 4. `rustdoc` later tries to resolve some path in a doc link 5. suggestion logic fires and loads "extern prelude" crates by name 6. the sysroot `libcore-*.rlib` is loaded and *fails to claim ownership* of builtin macros This fixes step 5. by not running suggestion logic if this is a speculative resolution. Additionally, it marks `resolve_ast_path` as a speculative resolution. --- src/librustc_resolve/lib.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index f2319bfe64d..5892edf7652 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -2385,8 +2385,12 @@ impl<'a> Resolver<'a> { Res::Def(DefKind::Mod, _) => true, _ => false, }; - let mut candidates = - self.lookup_import_candidates(ident, TypeNS, parent_scope, is_mod); + // Don't look up import candidates if this is a speculative resolve + let mut candidates = if record_used { + self.lookup_import_candidates(ident, TypeNS, parent_scope, is_mod) + } else { + Vec::new() + }; candidates.sort_by_cached_key(|c| { (c.path.segments.len(), pprust::path_to_string(&c.path)) }); @@ -3200,7 +3204,7 @@ impl<'a> Resolver<'a> { &Segment::from_path(path), Some(ns), parent_scope, - true, + false, path.span, CrateLint::No, ) {