[WIP] Intra-doc links side of warning about undocumented items

src/librustdoc/clean/mod.rs

@@ -1890,6 +1890,7 @@ fn clean_maybe_renamed_item(
 ) -> Vec<Item> {
     use hir::ItemKind;
 
+    debug!("clean HIR item {:?}", item);
     let def_id = item.def_id.to_def_id();
     let mut name = renamed.unwrap_or_else(|| cx.tcx.hir().name(item.hir_id()));
     cx.with_param_env(def_id, |cx| {
@@ -2017,6 +2018,7 @@ fn clean_impl(impl_: &hir::Impl<'_>, hir_id: hir::HirId, cx: &mut DocContext<'_>
         ret.push(make_item(trait_.clone(), type_alias, items.clone()));
     }
     ret.push(make_item(trait_, for_, items));
+    debug!("create cleaned impl item {:?}", ret);
     ret
 }
 

src/librustdoc/clean/types.rs

@@ -1,3 +1,4 @@
+use core::fmt;
 use std::cell::RefCell;
 use std::default::Default;
 use std::hash::Hash;
@@ -351,7 +352,7 @@ crate enum ExternalLocation {
 /// Anything with a source location and set of attributes and, optionally, a
 /// name. That is, anything that can be documented. This doesn't correspond
 /// directly to the AST's concept of an item; it's a strict superset.
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 crate struct Item {
     /// The name of this item.
     /// Optional because not every item has a name, e.g. impls.
@@ -366,6 +367,27 @@ crate struct Item {
     crate cfg: Option<Arc<Cfg>>,
 }
 
+impl fmt::Debug for Item {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let alternate = f.alternate();
+        // hand-picked fields that don't bloat the logs too much
+        let mut fmt = f.debug_struct("Item");
+        fmt.field("name", &self.name)
+            .field("visibility", &self.visibility)
+            .field("def_id", &self.def_id);
+        // allow printing the full item if someone really wants to
+        if alternate {
+            fmt.field("attrs", &self.attrs).field("kind", &self.kind).field("cfg", &self.cfg);
+        } else {
+            fmt.field("kind", &self.type_());
+            if let Some(docs) = self.doc_value() {
+                fmt.field("docs", &docs);
+            }
+        }
+        fmt.finish()
+    }
+}
+
 // `Item` is used a lot. Make sure it doesn't unintentionally get bigger.
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
 rustc_data_structures::static_assert_size!(Item, 56);
@@ -1101,11 +1123,16 @@ impl Attributes {
                         ast::LitKind::Str(s, _) => {
                             aliases.insert(s);
                         }
-                        _ => unreachable!(),
+                        // This can be reached in case of an invalid attribute.
+                        // We test that rustc_passes gives an error in `src/test/rustdoc-ui/check-doc-alias-attr.rs`
+                        _ => {}
                     }
                 }
-            } else {
-                aliases.insert(attr.value_str().unwrap());
+            // This can be None if the user specified an invalid attribute, e.g. `doc(alias = 0)`.
+            // The error will be caught by rustc_passes, but we still need to handle it here because
+            // the Cache is populated before calling `abort_if_err()`.
+            } else if let Some(value) = attr.value_str() {
+                aliases.insert(value);
             }
         }
         aliases.into_iter().collect::<Vec<_>>().into()

src/librustdoc/config.rs

@@ -339,14 +339,14 @@ impl Options {
                 println!("{:>20} - {}", pass.name, pass.description);
             }
             println!("\nDefault passes for rustdoc:");
-            for p in passes::DEFAULT_PASSES {
+            for &p in passes::DEFAULT_PASSES.0.iter().chain(passes::DEFAULT_PASSES.1) {
                 print!("{:>20}", p.pass.name);
                 println_condition(p.condition);
             }
 
             if nightly_options::match_is_nightly_build(matches) {
                 println!("\nPasses run with `--show-coverage`:");
-                for p in passes::COVERAGE_PASSES {
+                for &p in passes::COVERAGE_PASSES.0.iter().chain(passes::COVERAGE_PASSES.1) {
                     print!("{:>20}", p.pass.name);
                     println_condition(p.condition);
                 }

src/librustdoc/core.rs

@@ -25,10 +25,10 @@ use std::mem;
 use std::rc::Rc;
 
 use crate::clean::inline::build_external_trait;
-use crate::clean::{self, ItemId, TraitWithExtraInfo};
+use crate::clean::{self, Crate, ItemId, TraitWithExtraInfo};
 use crate::config::{Options as RustdocOptions, OutputFormat, RenderOptions};
 use crate::formats::cache::Cache;
-use crate::passes::{self, Condition::*};
+use crate::passes::{self, Condition::*, ConditionalPass};
 
 crate use rustc_session::config::{DebuggingOptions, Input, Options};
 
@@ -438,8 +438,7 @@ crate fn run_global_ctxt(
     }
 
     info!("Executing passes");
-
-    for p in passes::defaults(show_coverage) {
+    let run_pass = |p: ConditionalPass, krate: Crate, ctxt: &mut DocContext<'_>| {
         let run = match p.condition {
             Always => true,
             WhenDocumentPrivate => ctxt.render_options.document_private,
@@ -448,15 +447,27 @@ crate fn run_global_ctxt(
         };
         if run {
             debug!("running pass {}", p.pass.name);
-            krate = tcx.sess.time(p.pass.name, || (p.pass.run)(krate, &mut ctxt));
+            tcx.sess.time(p.pass.name, || (p.pass.run)(krate, ctxt))
+        } else {
+            krate
         }
+    };
+
+    let (passes_before_cache, passes_after_cache) = passes::defaults(show_coverage);
+    for &p in passes_before_cache {
+        krate = run_pass(p, krate, &mut ctxt);
+    }
+
+    krate = tcx.sess.time("create_format_cache", || Cache::populate(&mut ctxt, krate));
+    for &p in passes_after_cache {
+        krate = run_pass(p, krate, &mut ctxt);
     }
 
     if tcx.sess.diagnostic().has_errors_or_lint_errors().is_some() {
         rustc_errors::FatalError.raise();
     }
 
-    krate = tcx.sess.time("create_format_cache", || Cache::populate(&mut ctxt, krate));
+    krate = ctxt.cache.populate_impls(krate);
 
     (krate, ctxt.render_options, ctxt.cache)
 }

src/librustdoc/formats/cache.rs

@@ -179,9 +179,29 @@ impl Cache {
                 }
             }
         }
+        // god what a mess
+        *krate.external_traits.borrow_mut() = mem::take(&mut cx.cache.traits);
 
         krate
     }
+
+    /// `external_trats` / `cache.traits` is modified in various passes.
+    /// Run this separate from the main `populate` call, since `impls` isn't used until later in the HTML formatter.
+    crate fn populate_impls(&mut self, krate: clean::Crate) -> clean::Crate {
+        self.traits = krate.external_traits.take();
+        ImplRemover.fold_crate(krate)
+    }
+}
+
+struct ImplRemover;
+impl DocFolder for ImplRemover {
+    fn fold_item(&mut self, item: clean::Item) -> Option<clean::Item> {
+        let item = self.fold_item_recur(item);
+        match *item.kind {
+            clean::ItemKind::ImplItem(_) => None,
+            _ => Some(item),
+        }
+    }
 }
 
 impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
@@ -210,6 +230,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
                     .def_id(self.cache)
                     .map_or(false, |d| self.cache.masked_crates.contains(&d.krate))
             {
+                debug!("remove masked impl {:?}", i);
                 return None;
             }
         }
@@ -429,7 +450,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
         // Once we've recursively found all the generics, hoard off all the
         // implementations elsewhere.
         let item = self.fold_item_recur(item);
-        let ret = if let clean::Item { kind: box clean::ImplItem(ref i), .. } = item {
+        if let clean::Item { kind: box clean::ImplItem(ref i), .. } = item {
             // Figure out the id of this impl. This may map to a
             // primitive rather than always to a struct/enum.
             // Note: matching twice to restrict the lifetime of the `i` borrow.
@@ -461,19 +482,16 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
                     }
                 }
             }
-            let impl_item = Impl { impl_item: item };
+            let impl_item = Impl { impl_item: item.clone() };
             if impl_item.trait_did().map_or(true, |d| self.cache.traits.contains_key(&d)) {
                 for did in dids {
                     self.cache.impls.entry(did).or_insert_with(Vec::new).push(impl_item.clone());
                 }
             } else {
                 let trait_did = impl_item.trait_did().expect("no trait did");
-                self.cache.orphan_trait_impls.push((trait_did, dids, impl_item));
+                self.cache.orphan_trait_impls.push((trait_did, dids, impl_item.clone()));
             }
-            None
-        } else {
-            Some(item)
-        };
+        }
 
         if pushed {
             self.cache.stack.pop().expect("stack already empty");
@@ -483,6 +501,6 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
         }
         self.cache.stripped_mod = orig_stripped_mod;
         self.cache.parent_is_trait_impl = orig_parent_is_trait_impl;
-        ret
+        Some(item)
     }
 }

src/librustdoc/html/format.rs

@@ -26,6 +26,7 @@ use crate::clean::{
     self, types::ExternalLocation, utils::find_nearest_parent_module, ExternalCrate, ItemId,
     PrimitiveType,
 };
+use crate::formats::cache::Cache;
 use crate::formats::item_type::ItemType;
 use crate::html::escape::Escape;
 use crate::html::render::Context;
@@ -523,6 +524,7 @@ impl clean::GenericArgs {
 }
 
 // Possible errors when computing href link source for a `DefId`
+#[derive(Debug, PartialEq, Eq)]
 crate enum HrefError {
     /// This item is known to rustdoc, but from a crate that does not have documentation generated.
     ///
@@ -551,20 +553,31 @@ crate fn href_with_root_path(
     root_path: Option<&str>,
 ) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
     let tcx = cx.tcx();
+    let cache = cx.cache();
+    let relative_to = &cx.current;
+    href_inner(did, tcx, cache, root_path, relative_to)
+}
+
+crate fn href_inner(
+    did: DefId,
+    tcx: TyCtxt<'_>,
+    cache: &Cache,
+    root_path: Option<&str>,
+    relative_to: &[Symbol],
+) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
     let def_kind = tcx.def_kind(did);
     let did = match def_kind {
-        DefKind::AssocTy | DefKind::AssocFn | DefKind::AssocConst | DefKind::Variant => {
+        DefKind::AssocTy | DefKind::AssocFn | DefKind::AssocConst | DefKind::Variant | DefKind::Field => {
             // documented on their parent's page
             tcx.parent(did).unwrap()
         }
         _ => did,
     };
-    let cache = cx.cache();
-    let relative_to = &cx.current;
     fn to_module_fqp(shortty: ItemType, fqp: &[Symbol]) -> &[Symbol] {
         if shortty == ItemType::Module { fqp } else { &fqp[..fqp.len() - 1] }
     }
 
+    trace!("calculating href for {:?}", did);
     if !did.is_local()
         && !cache.access_levels.is_public(did)
         && !cache.document_private