Add an option not to report resolution errors for rustdoc

- Remove unnecessary `should_loop` variable
- Report errors for trait implementations

These should give resolution errors because they are visible outside the
current scope. Without these errors, rustdoc will give ICEs:

```
thread 'rustc' panicked at 'attempted .def_id() on invalid res: Err', /home/joshua/src/rust/src/libstd/macros.rs:16:9
  15: rustc_hir::def::Res<Id>::def_id
             at /home/joshua/src/rust/src/librustc_hir/def.rs:382
  16: rustdoc::clean::utils::register_res
             at src/librustdoc/clean/utils.rs:627
  17: rustdoc::clean::utils::resolve_type
             at src/librustdoc/clean/utils.rs:587
```

- Add much more extensive tests
  + fn -> impl -> fn
  + fn -> impl -> fn -> macro
  + errors in function parameters
  + errors in trait bounds
  + errors in the type implementing the trait
  + unknown bounds for the type
  + unknown types in function bodies
  + errors generated by macros

- Use explicit state instead of trying to reconstruct it from random info
- Use an enum instead of a boolean
- Add example of ignored error

Author: jyn514

src/librustc_interface/passes.rs

@@ -233,6 +233,8 @@ fn configure_and_expand_inner<'a>(
     resolver_arenas: &'a ResolverArenas<'a>,
     metadata_loader: &'a MetadataLoaderDyn,
 ) -> Result<(ast::Crate, Resolver<'a>)> {
+    use rustc_resolve::IgnoreState;
+
     log::trace!("configure_and_expand_inner");
     pre_expansion_lint(sess, lint_store, &krate);
 
@@ -354,13 +356,18 @@ fn configure_and_expand_inner<'a>(
         )
     });
 
-    let crate_types = sess.crate_types();
-    let is_proc_macro_crate = crate_types.contains(&CrateType::ProcMacro);
+    if let Some(PpMode::PpmSource(PpSourceMode::PpmEveryBodyLoops)) = sess.opts.pretty {
+        log::debug!("replacing bodies with loop {{}}");
+        util::ReplaceBodyWithLoop::new(&mut resolver).visit_crate(&mut krate);
+    }
 
     let has_proc_macro_decls = sess.time("AST_validation", || {
         rustc_ast_passes::ast_validation::check_crate(sess, &krate, &mut resolver.lint_buffer())
     });
 
+    let crate_types = sess.crate_types();
+    let is_proc_macro_crate = crate_types.contains(&CrateType::ProcMacro);
+
     // For backwards compatibility, we don't try to run proc macro injection
     // if rustdoc is run on a proc macro crate without '--crate-type proc-macro' being
     // specified. This should only affect users who manually invoke 'rustdoc', as
@@ -407,17 +414,9 @@ fn configure_and_expand_inner<'a>(
     }
 
     // If we're actually rustdoc then avoid giving a name resolution error for `cfg()` items.
-    // anything, so switch everything to just looping
-    resolver.resolve_crate(&krate, sess.opts.actually_rustdoc);
-
-    let mut should_loop = false;
-    if let Some(PpMode::PpmSource(PpSourceMode::PpmEveryBodyLoops)) = sess.opts.pretty {
-        should_loop |= true;
-    }
-    if should_loop {
-        log::debug!("replacing bodies with loop {{}}");
-        util::ReplaceBodyWithLoop::new(&mut resolver).visit_crate(&mut krate);
-    }
+    let ignore_bodies =
+        if sess.opts.actually_rustdoc { IgnoreState::Ignore } else { IgnoreState::Report };
+    resolver.resolve_crate(&krate, ignore_bodies);
 
     // Needs to go *after* expansion to be able to check the results of macro expansion.
     sess.time("complete_gated_feature_checking", || {

src/librustc_resolve/late.rs

@@ -376,6 +376,19 @@ struct DiagnosticMetadata<'ast> {
     current_let_binding: Option<(Span, Option<Span>, Option<Span>)>,
 }
 
+/// Keeps track of whether errors should be reported.
+///
+/// Used by rustdoc to ignore errors in function bodies.
+/// This is just a fancy boolean so it can have doc-comments.
+#[derive(Copy, Clone, Debug)]
+pub enum IgnoreState {
+    /// We are at global scope or in a trait implementation, so all errors should be reported.
+    Report,
+    /// We are in a function body, so errors shouldn't be reported.
+    Ignore,
+    // Note that we don't need to worry about macros, which must always be resolved (or we wouldn't have gotten to the late pass).
+}
+
 struct LateResolutionVisitor<'a, 'b, 'ast> {
     r: &'b mut Resolver<'a>,
 
@@ -395,10 +408,12 @@ struct LateResolutionVisitor<'a, 'b, 'ast> {
     /// Fields used to add information to diagnostic errors.
     diagnostic_metadata: DiagnosticMetadata<'ast>,
 
-    /// Whether to report resolution errors for item bodies.
+    /// State used to know whether to ignore resolution errors for item bodies.
     ///
     /// In particular, rustdoc uses this to avoid giving errors for `cfg()` items.
-    ignore_bodies: bool,
+    /// In most cases this will be `None`, in which case errors will always be reported.
+    /// If it is `Some(_)`, then it will be updated when entering a nested function or trait body.
+    ignore_bodies: Option<IgnoreState>,
 }
 
 /// Walks the whole crate in DFS order, visiting each item, resolving names as it goes.
@@ -502,13 +517,18 @@ impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> {
 
                 visit::walk_fn_ret_ty(this, &declaration.output);
 
+                let previous_ignore = this.ignore_bodies.take();
+                // Ignore errors in function bodies if originally passed `ignore_state: true`
+                // Be sure not to set this until the function signature has been resolved.
+                this.ignore_bodies = previous_ignore.and(Some(IgnoreState::Ignore));
                 // Resolve the function body, potentially inside the body of an async closure
                 match fn_kind {
                     FnKind::Fn(.., body) => walk_list!(this, visit_block, body),
                     FnKind::Closure(_, body) => this.visit_expr(body),
                 };
 
                 debug!("(resolving function) leaving function");
+                this.ignore_bodies = previous_ignore;
             })
         });
         self.diagnostic_metadata.current_function = previous_value;
@@ -634,7 +654,7 @@ impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> {
 impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
     fn new(
         resolver: &'b mut Resolver<'a>,
-        ignore_bodies: bool,
+        ignore_bodies: IgnoreState,
     ) -> LateResolutionVisitor<'a, 'b, 'ast> {
         // During late resolution we only track the module component of the parent scope,
         // although it may be useful to track other components as well for diagnostics.
@@ -652,7 +672,11 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
             label_ribs: Vec::new(),
             current_trait_ref: None,
             diagnostic_metadata: DiagnosticMetadata::default(),
-            ignore_bodies,
+            ignore_bodies: match ignore_bodies {
+                // errors at module scope should always be reported
+                IgnoreState::Ignore => Some(IgnoreState::Report),
+                IgnoreState::Report => None,
+            },
         }
     }
 
@@ -842,7 +866,11 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
             };
             let report_error = |this: &Self, ns| {
                 let what = if ns == TypeNS { "type parameters" } else { "local variables" };
-                this.r.session.span_err(ident.span, &format!("imports cannot refer to {}", what));
+                if this.should_report_errs() {
+                    this.r
+                        .session
+                        .span_err(ident.span, &format!("imports cannot refer to {}", what));
+                }
             };
 
             for &ns in nss {
@@ -1166,6 +1194,9 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
         impl_items: &'ast [P<AssocItem>],
     ) {
         debug!("resolve_implementation");
+        let old_ignore = self.ignore_bodies.take();
+        // Never ignore errors in trait implementations.
+        self.ignore_bodies = old_ignore.and(Some(IgnoreState::Report));
         // If applicable, create a rib for the type parameters.
         self.with_generic_param_rib(generics, ItemRibKind(HasGenericParams::Yes), |this| {
             // Dummy self type for better errors if `Self` is used in the trait path.
@@ -1261,6 +1292,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
                 });
             });
         });
+        self.ignore_bodies = old_ignore;
     }
 
     fn check_trait_item<F>(&mut self, ident: Ident, ns: Namespace, span: Span, err: F)
@@ -1298,6 +1330,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
     }
 
     fn resolve_local(&mut self, local: &'ast Local) {
+        debug!("resolving local ({:?})", local);
         // Resolve the type.
         walk_list!(self, visit_ty, &local.ty);
 
@@ -1686,18 +1719,27 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
         source: PathSource<'ast>,
         crate_lint: CrateLint,
     ) -> PartialRes {
+        log::debug!("smart_resolve_path_fragment(id={:?},qself={:?},path={:?}", id, qself, path);
         let ns = source.namespace();
         let is_expected = &|res| source.is_expected(res);
 
         let report_errors = |this: &mut Self, res: Option<Res>| {
-            let (err, candidates) = this.smart_resolve_report_errors(path, span, source, res);
-
-            let def_id = this.parent_scope.module.normal_ancestor_id;
-            let instead = res.is_some();
-            let suggestion =
-                if res.is_none() { this.report_missing_type_error(path) } else { None };
-
-            this.r.use_injections.push(UseError { err, candidates, def_id, instead, suggestion });
+            if this.should_report_errs() {
+                let (err, candidates) = this.smart_resolve_report_errors(path, span, source, res);
+
+                let def_id = this.parent_scope.module.normal_ancestor_id;
+                let instead = res.is_some();
+                let suggestion =
+                    if res.is_none() { this.report_missing_type_error(path) } else { None };
+
+                this.r.use_injections.push(UseError {
+                    err,
+                    candidates,
+                    def_id,
+                    instead,
+                    suggestion,
+                });
+            }
 
             PartialRes::new(Res::Err)
         };
@@ -1755,13 +1797,17 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
 
             let def_id = this.parent_scope.module.normal_ancestor_id;
 
-            this.r.use_injections.push(UseError {
-                err,
-                candidates,
-                def_id,
-                instead: false,
-                suggestion: None,
-            });
+            if this.should_report_errs() {
+                this.r.use_injections.push(UseError {
+                    err,
+                    candidates,
+                    def_id,
+                    instead: false,
+                    suggestion: None,
+                });
+            } else {
+                err.cancel();
+            }
 
             // We don't return `Some(parent_err)` here, because the error will
             // be already printed as part of the `use` injections
@@ -1856,11 +1902,20 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
     ///
     /// This doesn't emit errors for function bodies if `ignore_bodies` is set.
     fn report_error(&self, span: Span, resolution_error: ResolutionError<'_>) {
-        if !self.ignore_bodies || self.diagnostic_metadata.current_function.is_none() {
+        if self.should_report_errs() {
             self.r.report_error(span, resolution_error);
         }
     }
 
+    #[inline]
+    fn should_report_errs(&self) -> bool {
+        debug!("should_report_errs(state={:?})", self.ignore_bodies);
+        match self.ignore_bodies {
+            None | Some(IgnoreState::Report) => true,
+            Some(IgnoreState::Ignore) => false,
+        }
+    }
+
     // Resolve in alternative namespaces if resolution in the primary namespace fails.
     fn resolve_qpath_anywhere(
         &mut self,
@@ -2357,7 +2412,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
 }
 
 impl<'a> Resolver<'a> {
-    pub(crate) fn late_resolve_crate(&mut self, krate: &Crate, ignore_bodies: bool) {
+    pub(crate) fn late_resolve_crate(&mut self, krate: &Crate, ignore_bodies: IgnoreState) {
         let mut late_resolution_visitor = LateResolutionVisitor::new(self, ignore_bodies);
         visit::walk_crate(&mut late_resolution_visitor, krate);
         for (id, span) in late_resolution_visitor.diagnostic_metadata.unused_labels.iter() {

src/librustc_resolve/lib.rs

@@ -15,6 +15,7 @@
 #![feature(or_patterns)]
 #![recursion_limit = "256"]
 
+pub use late::IgnoreState;
 pub use rustc_hir::def::{Namespace, PerNS};
 
 use Determinacy::*;
@@ -1441,7 +1442,7 @@ impl<'a> Resolver<'a> {
     }
 
     /// Entry point to crate resolution.
-    pub fn resolve_crate(&mut self, krate: &Crate, ignore_bodies: bool) {
+    pub fn resolve_crate(&mut self, krate: &Crate, ignore_bodies: IgnoreState) {
         let _prof_timer = self.session.prof.generic_activity("resolve_crate");
 
         ImportResolver { r: self }.finalize_imports();

src/test/rustdoc-ui/impl-fn-nesting.rs

@@ -0,0 +1,49 @@
+// Ensure that rustdoc gives errors for trait impls inside function bodies that don't resolve.
+// See https://github.com/rust-lang/rust/pull/73566
+pub struct ValidType;
+pub trait ValidTrait {}
+pub trait NeedsBody {
+    type Item;
+    fn f();
+}
+
+/// This function has docs
+pub fn f<B: UnknownBound>(a: UnknownType, b: B) {
+//~^ ERROR cannot find trait `UnknownBound` in this scope
+//~| ERROR cannot find type `UnknownType` in this scope
+    impl UnknownTrait for ValidType {} //~ ERROR cannot find trait `UnknownTrait`
+    impl<T: UnknownBound> UnknownTrait for T {}
+    //~^ ERROR cannot find trait `UnknownBound` in this scope
+    //~| ERROR cannot find trait `UnknownTrait` in this scope
+    impl ValidTrait for UnknownType {}
+    //~^ ERROR cannot find type `UnknownType` in this scope
+    impl ValidTrait for ValidType where ValidTrait: UnknownBound {}
+    //~^ ERROR cannot find trait `UnknownBound` in this scope
+
+    /// This impl has documentation
+    impl NeedsBody for ValidType {
+        type Item = UnknownType;
+        //~^ ERROR cannot find type `UnknownType` in this scope
+
+        /// This function has documentation
+        fn f() {
+            <UnknownTypeShouldBeIgnored>::a();
+            content::shouldnt::matter();
+            unknown_macro!();
+            //~^ ERROR cannot find macro `unknown_macro` in this scope
+
+            /// This is documentation for a macro
+            macro_rules! can_define_macros_here_too {
+                () => {
+                    this::content::should::also::be::ignored()
+                }
+            }
+            can_define_macros_here_too!();
+
+            /// This also is documented.
+            pub fn doubly_nested(c: UnknownTypeShouldBeIgnored) {
+
+            }
+        }
+    }
+}

src/test/rustdoc-ui/impl-fn-nesting.stderr

@@ -0,0 +1,60 @@
+error: cannot find macro `unknown_macro` in this scope
+  --> $DIR/impl-fn-nesting.rs:32:13
+   |
+LL |             unknown_macro!();
+   |             ^^^^^^^^^^^^^
+
+error[E0405]: cannot find trait `UnknownBound` in this scope
+  --> $DIR/impl-fn-nesting.rs:11:13
+   |
+LL | pub fn f<B: UnknownBound>(a: UnknownType, b: B) {
+   |             ^^^^^^^^^^^^ not found in this scope
+
+error[E0412]: cannot find type `UnknownType` in this scope
+  --> $DIR/impl-fn-nesting.rs:11:30
+   |
+LL | pub fn f<B: UnknownBound>(a: UnknownType, b: B) {
+   |                              ^^^^^^^^^^^ not found in this scope
+
+error[E0405]: cannot find trait `UnknownTrait` in this scope
+  --> $DIR/impl-fn-nesting.rs:14:10
+   |
+LL |     impl UnknownTrait for ValidType {}
+   |          ^^^^^^^^^^^^ not found in this scope
+
+error[E0405]: cannot find trait `UnknownTrait` in this scope
+  --> $DIR/impl-fn-nesting.rs:15:27
+   |
+LL |     impl<T: UnknownBound> UnknownTrait for T {}
+   |                           ^^^^^^^^^^^^ not found in this scope
+
+error[E0405]: cannot find trait `UnknownBound` in this scope
+  --> $DIR/impl-fn-nesting.rs:15:13
+   |
+LL |     impl<T: UnknownBound> UnknownTrait for T {}
+   |             ^^^^^^^^^^^^ not found in this scope
+
+error[E0412]: cannot find type `UnknownType` in this scope
+  --> $DIR/impl-fn-nesting.rs:18:25
+   |
+LL |     impl ValidTrait for UnknownType {}
+   |                         ^^^^^^^^^^^ not found in this scope
+
+error[E0405]: cannot find trait `UnknownBound` in this scope
+  --> $DIR/impl-fn-nesting.rs:20:53
+   |
+LL |     impl ValidTrait for ValidType where ValidTrait: UnknownBound {}
+   |                                                     ^^^^^^^^^^^^ not found in this scope
+
+error[E0412]: cannot find type `UnknownType` in this scope
+  --> $DIR/impl-fn-nesting.rs:25:21
+   |
+LL |         type Item = UnknownType;
+   |                     ^^^^^^^^^^^ not found in this scope
+
+error: Compilation failed, aborting rustdoc
+
+error: aborting due to 10 previous errors
+
+Some errors have detailed explanations: E0405, E0412.
+For more information about an error, try `rustc --explain E0405`.

src/test/rustdoc/doc-cfg.rs

@@ -57,7 +57,5 @@ pub unsafe fn uses_target_feature() {
 //        'This is supported with target feature avx only.'
 #[doc(cfg(target_feature = "avx"))]
 pub fn uses_cfg_target_feature() {
-    unsafe {
-        uses_target_feature();
-    }
+    uses_target_feature();
 }