From a5275ff41521bb8e5a70f49f8ed420eaac7ed7de Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 20 Jun 2020 15:57:23 -0400 Subject: [PATCH 01/23] Don't run everybody_loops for rustdoc Instead, ignore resolution errors that occur in item bodies. The reason this can't ignore item bodies altogether is because `const fn` could be used in generic types, for example `[T; f()]` --- src/librustc_interface/passes.rs | 29 +++++++++++----------- src/librustc_resolve/late.rs | 42 +++++++++++++++++++++++--------- src/librustc_resolve/lib.rs | 4 +-- src/test/rustdoc/doc-cfg.rs | 4 ++- 4 files changed, 50 insertions(+), 29 deletions(-) diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 241ba869d3eb1..1862b47b9adb3 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -354,24 +354,13 @@ fn configure_and_expand_inner<'a>( ) }); - // If we're actually rustdoc then there's no need to actually compile - // anything, so switch everything to just looping - let mut should_loop = sess.opts.actually_rustdoc; - 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 crate_types = sess.crate_types(); + let is_proc_macro_crate = crate_types.contains(&CrateType::ProcMacro); 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 @@ -417,7 +406,19 @@ fn configure_and_expand_inner<'a>( println!("{}", json::as_json(&krate)); } - resolver.resolve_crate(&krate); + // 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 = 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); + } // Needs to go *after* expansion to be able to check the results of macro expansion. sess.time("complete_gated_feature_checking", || { diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index c165a601408fd..ddce82494e1ba 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -394,6 +394,11 @@ 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. + /// + /// In particular, rustdoc uses this to avoid giving errors for `cfg()` items. + ignore_bodies: bool, } /// Walks the whole crate in DFS order, visiting each item, resolving names as it goes. @@ -627,7 +632,10 @@ impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> { } impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { - fn new(resolver: &'b mut Resolver<'a>) -> LateResolutionVisitor<'a, 'b, 'ast> { + fn new( + resolver: &'b mut Resolver<'a>, + ignore_bodies: bool, + ) -> 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. let graph_root = resolver.graph_root; @@ -644,6 +652,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { label_ribs: Vec::new(), current_trait_ref: None, diagnostic_metadata: DiagnosticMetadata::default(), + ignore_bodies, } } @@ -757,7 +766,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { return if self.is_label_valid_from_rib(i) { Some(*id) } else { - self.r.report_error( + self.report_error( original_span, ResolutionError::UnreachableLabel { name: label.name, @@ -775,7 +784,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { suggestion = suggestion.or_else(|| self.suggestion_for_label_in_rib(i, label)); } - self.r.report_error( + self.report_error( original_span, ResolutionError::UndeclaredLabel { name: label.name, suggestion }, ); @@ -1008,7 +1017,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { if seen_bindings.contains_key(&ident) { let span = seen_bindings.get(&ident).unwrap(); let err = ResolutionError::NameAlreadyUsedInParameterList(ident.name, *span); - self.r.report_error(param.ident.span, err); + self.report_error(param.ident.span, err); } seen_bindings.entry(ident).or_insert(param.ident.span); @@ -1274,7 +1283,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { .is_err() { let path = &self.current_trait_ref.as_ref().unwrap().1.path; - self.r.report_error(span, err(ident.name, &path_names_to_string(path))); + self.report_error(span, err(ident.name, &path_names_to_string(path))); } } } @@ -1390,7 +1399,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { if inconsistent_vars.contains_key(name) { v.could_be_path = false; } - self.r.report_error( + self.report_error( *v.origin.iter().next().unwrap(), ResolutionError::VariableNotBoundInPattern(v), ); @@ -1400,7 +1409,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { let mut inconsistent_vars = inconsistent_vars.iter().collect::>(); inconsistent_vars.sort(); for (name, v) in inconsistent_vars { - self.r.report_error(v.0, ResolutionError::VariableBoundWithDifferentMode(*name, v.1)); + self.report_error(v.0, ResolutionError::VariableBoundWithDifferentMode(*name, v.1)); } // 5) Finally bubble up all the binding maps. @@ -1550,7 +1559,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // `Variant(a, a)`: _ => IdentifierBoundMoreThanOnceInSamePattern, }; - self.r.report_error(ident.span, error(ident.name)); + self.report_error(ident.span, error(ident.name)); } // Record as bound if it's valid: @@ -1624,7 +1633,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // to something unusable as a pattern (e.g., constructor function), // but we still conservatively report an error, see // issues/33118#issuecomment-233962221 for one reason why. - self.r.report_error( + self.report_error( ident.span, ResolutionError::BindingShadowsSomethingUnacceptable( pat_src.descr(), @@ -1809,7 +1818,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { Err(err) => { if let Some(err) = report_errors_for_call(self, err) { - self.r.report_error(err.span, err.node); + self.report_error(err.span, err.node); } PartialRes::new(Res::Err) @@ -1843,6 +1852,15 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { if let Some(LexicalScopeBinding::Res(res)) = binding { res != Res::Err } else { false } } + /// A wrapper around [`Resolver::report_error`]. + /// + /// 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() { + self.r.report_error(span, resolution_error); + } + } + // Resolve in alternative namespaces if resolution in the primary namespace fails. fn resolve_qpath_anywhere( &mut self, @@ -2339,8 +2357,8 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { } impl<'a> Resolver<'a> { - pub(crate) fn late_resolve_crate(&mut self, krate: &Crate) { - let mut late_resolution_visitor = LateResolutionVisitor::new(self); + pub(crate) fn late_resolve_crate(&mut self, krate: &Crate, ignore_bodies: bool) { + 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() { self.lint_buffer.buffer_lint(lint::builtin::UNUSED_LABELS, *id, *span, "unused label"); diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index a265c15c18bc9..786dc28ba0eec 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1441,13 +1441,13 @@ impl<'a> Resolver<'a> { } /// Entry point to crate resolution. - pub fn resolve_crate(&mut self, krate: &Crate) { + pub fn resolve_crate(&mut self, krate: &Crate, ignore_bodies: bool) { let _prof_timer = self.session.prof.generic_activity("resolve_crate"); ImportResolver { r: self }.finalize_imports(); self.finalize_macro_resolutions(); - self.late_resolve_crate(krate); + self.late_resolve_crate(krate, ignore_bodies); self.check_unused(krate); self.report_errors(krate); diff --git a/src/test/rustdoc/doc-cfg.rs b/src/test/rustdoc/doc-cfg.rs index aa407b7e92618..8664930bc94f4 100644 --- a/src/test/rustdoc/doc-cfg.rs +++ b/src/test/rustdoc/doc-cfg.rs @@ -57,5 +57,7 @@ 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() { - uses_target_feature(); + unsafe { + uses_target_feature(); + } } From b3187aabd20637e0bb9a930b4b930a079b785ca9 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 20 Jun 2020 16:41:39 -0400 Subject: [PATCH 02/23] Don't run analysis pass in rustdoc - Explicitly check for missing docs - Don't run any lints except those we explicitly specified --- src/librustc_interface/passes.rs | 1 - src/librustdoc/core.rs | 12 +++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 1862b47b9adb3..b814283555b83 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -410,7 +410,6 @@ fn configure_and_expand_inner<'a>( // anything, so switch everything to just looping resolver.resolve_crate(&krate, sess.opts.actually_rustdoc); - //let mut should_loop = sess.opts.actually_rustdoc; let mut should_loop = false; if let Some(PpMode::PpmSource(PpSourceMode::PpmEveryBodyLoops)) = sess.opts.pretty { should_loop |= true; diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index a222920c7d292..061d2d21ec927 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -372,7 +372,10 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt crate_name, lint_caps, register_lints: None, - override_queries: None, + override_queries: Some(|_sess, local_providers, external_providers| { + local_providers.lint_mod = |_, _| {}; + external_providers.lint_mod = |_, _| {}; + }), registry: rustc_driver::diagnostics_registry(), }; @@ -416,10 +419,9 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt let mut global_ctxt = abort_on_err(queries.global_ctxt(), sess).take(); global_ctxt.enter(|tcx| { - tcx.analysis(LOCAL_CRATE).ok(); - - // Abort if there were any errors so far - sess.abort_if_errors(); + sess.time("missing_docs", || { + rustc_lint::check_crate(tcx, rustc_lint::builtin::MissingDoc::new); + }); let access_levels = tcx.privacy_access_levels(LOCAL_CRATE); // Convert from a HirId set to a DefId set since we don't always have easy access From 1b8accb7497e6fe66be331e40f8663d198a6b648 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 20 Jun 2020 17:01:03 -0400 Subject: [PATCH 03/23] 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::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 --- src/librustc_interface/passes.rs | 25 +++--- src/librustc_resolve/late.rs | 99 +++++++++++++++++----- src/librustc_resolve/lib.rs | 3 +- src/test/rustdoc-ui/impl-fn-nesting.rs | 49 +++++++++++ src/test/rustdoc-ui/impl-fn-nesting.stderr | 60 +++++++++++++ src/test/rustdoc/doc-cfg.rs | 4 +- 6 files changed, 201 insertions(+), 39 deletions(-) create mode 100644 src/test/rustdoc-ui/impl-fn-nesting.rs create mode 100644 src/test/rustdoc-ui/impl-fn-nesting.stderr diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index b814283555b83..690ed9decb9ef 100644 --- a/src/librustc_interface/passes.rs +++ b/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", || { diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index ddce82494e1ba..637326bb88d86 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -376,6 +376,19 @@ struct DiagnosticMetadata<'ast> { current_let_binding: Option<(Span, Option, Option)>, } +/// 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, } /// Walks the whole crate in DFS order, visiting each item, resolving names as it goes. @@ -502,6 +517,10 @@ 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), @@ -509,6 +528,7 @@ impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> { }; 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], ) { 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(&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| { - 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() { diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 786dc28ba0eec..23bd0028bd1dd 100644 --- a/src/librustc_resolve/lib.rs +++ b/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(); diff --git a/src/test/rustdoc-ui/impl-fn-nesting.rs b/src/test/rustdoc-ui/impl-fn-nesting.rs new file mode 100644 index 0000000000000..d2dd8d835fd79 --- /dev/null +++ b/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(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 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() { + ::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) { + + } + } + } +} diff --git a/src/test/rustdoc-ui/impl-fn-nesting.stderr b/src/test/rustdoc-ui/impl-fn-nesting.stderr new file mode 100644 index 0000000000000..f8629964c0701 --- /dev/null +++ b/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(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(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 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 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`. diff --git a/src/test/rustdoc/doc-cfg.rs b/src/test/rustdoc/doc-cfg.rs index 8664930bc94f4..aa407b7e92618 100644 --- a/src/test/rustdoc/doc-cfg.rs +++ b/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(); } From 14a8707cde48c7914af307f4687056d829ad2de9 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 7 Jun 2020 15:13:40 -0700 Subject: [PATCH 04/23] Add `rustdoc` tests from #72088 --- src/test/rustdoc/macro-in-async-block.rs | 9 +++++++++ src/test/rustdoc/macro-in-closure.rs | 7 +++++++ 2 files changed, 16 insertions(+) create mode 100644 src/test/rustdoc/macro-in-async-block.rs diff --git a/src/test/rustdoc/macro-in-async-block.rs b/src/test/rustdoc/macro-in-async-block.rs new file mode 100644 index 0000000000000..b4aaacf7b3d40 --- /dev/null +++ b/src/test/rustdoc/macro-in-async-block.rs @@ -0,0 +1,9 @@ +// Regression issue for rustdoc ICE encountered in PR #72088. +// edition:2018 +#![feature(decl_macro)] + +fn main() { + async { + macro m() {} + }; +} diff --git a/src/test/rustdoc/macro-in-closure.rs b/src/test/rustdoc/macro-in-closure.rs index 298ff601de89f..b4411d927e271 100644 --- a/src/test/rustdoc/macro-in-closure.rs +++ b/src/test/rustdoc/macro-in-closure.rs @@ -6,4 +6,11 @@ fn main() { || { macro m() {} }; + + let _ = || { + macro n() {} + }; + + let cond = true; + let _ = || if cond { macro n() {} } else { panic!() }; } From 768d6a4950d66f1a0e1e7793a984fb638494d1c5 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 3 Jul 2020 18:41:23 -0400 Subject: [PATCH 05/23] Don't ICE on errors in function returning impl trait Instead, report the error. This emits the errors on-demand, without special-casing `impl Trait`, so it should catch all ICEs of this kind, including ones that haven't been found yet. Since the error is emitted during type-checking there is less info about the error; see comments in the code for details. - Add test case for -> impl Trait - Add test for impl trait with alias - Move EmitIgnoredResolutionErrors to rustdoc This makes `fn typeck_item_bodies` public, which is not desired behavior. That change should be removed once https://github.com/rust-lang/rust/pull/74070 is merged. - Don't visit nested closures twice --- src/librustc_typeck/check/mod.rs | 2 +- src/librustc_typeck/lib.rs | 2 +- src/librustdoc/core.rs | 78 +++++++++++++++++++ src/librustdoc/lib.rs | 1 + src/test/rustdoc-ui/error-in-impl-trait.rs | 28 +++++++ .../rustdoc-ui/error-in-impl-trait.stderr | 39 ++++++++++ src/test/rustdoc/impl-trait-alias.rs | 14 ++++ 7 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 src/test/rustdoc-ui/error-in-impl-trait.rs create mode 100644 src/test/rustdoc-ui/error-in-impl-trait.stderr create mode 100644 src/test/rustdoc/impl-trait-alias.rs diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index bc01da324b66f..514600b4733d4 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -955,7 +955,7 @@ where val.fold_with(&mut FixupFolder { tcx }) } -fn typeck_tables_of<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> &ty::TypeckTables<'tcx> { +pub fn typeck_tables_of<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> &ty::TypeckTables<'tcx> { let fallback = move || tcx.type_of(def_id.to_def_id()); typeck_tables_of_with_fallback(tcx, def_id, fallback) } diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index 9ba2545ba63cb..79e1585ce3cdb 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -78,7 +78,7 @@ extern crate rustc_middle; pub mod expr_use_visitor; mod astconv; -mod check; +pub mod check; mod check_unused; mod coherence; mod collect; diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 061d2d21ec927..3d0da0e9157f7 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -375,6 +375,15 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt override_queries: Some(|_sess, local_providers, external_providers| { local_providers.lint_mod = |_, _| {}; external_providers.lint_mod = |_, _| {}; + //let old_typeck = local_providers.typeck_tables_of; + local_providers.typeck_tables_of = move |tcx, def_id| { + let hir = tcx.hir(); + let body = hir.body(hir.body_owned_by(hir.as_local_hir_id(def_id))); + debug!("visiting body for {:?}", def_id); + EmitIgnoredResolutionErrors::new(&tcx.sess).visit_body(body); + rustc_typeck::check::typeck_tables_of(tcx, def_id) + //DEFAULT_TYPECK.with(|typeck| typeck(tcx, def_id)) + }; }), registry: rustc_driver::diagnostics_registry(), }; @@ -572,6 +581,75 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt }) } +use rustc_hir::def::Res; +use rustc_hir::{ + intravisit::{NestedVisitorMap, Visitor}, + Path, +}; +use rustc_middle::hir::map::Map; + +/* +thread_local!(static DEFAULT_TYPECK: for<'tcx> fn(rustc_middle::ty::TyCtxt<'tcx>, rustc_span::def_id::LocalDefId) -> &'tcx rustc_middle::ty::TypeckTables<'tcx> = { + let mut providers = rustc_middle::ty::query::Providers::default(); + rustc_typeck::provide(&mut providers); + providers.typeck_tables_of +}); +*/ + +/// Due to https://github.com/rust-lang/rust/pull/73566, +/// the name resolution pass may find errors that are never emitted. +/// If typeck is called after this happens, then we'll get an ICE: +/// 'Res::Error found but not reported'. To avoid this, emit the errors now. +struct EmitIgnoredResolutionErrors<'a> { + session: &'a Session, +} + +impl<'a> EmitIgnoredResolutionErrors<'a> { + fn new(session: &'a Session) -> Self { + Self { session } + } +} + +impl<'a> Visitor<'a> for EmitIgnoredResolutionErrors<'_> { + type Map = Map<'a>; + + fn nested_visit_map(&mut self) -> NestedVisitorMap { + // If we visit nested bodies, then we will report errors twice for e.g. nested closures + NestedVisitorMap::None + } + + fn visit_path(&mut self, path: &'v Path<'v>, _id: HirId) { + log::debug!("visiting path {:?}", path); + if path.res == Res::Err { + // We have less context here than in rustc_resolve, + // so we can only emit the name and span. + // However we can give a hint that rustc_resolve will have more info. + // NOTE: this is a very rare case (only 4 out of several hundred thousand crates in a crater run) + // NOTE: so it's ok for it to be slow + let label = format!( + "could not resolve path `{}`", + path.segments + .iter() + .map(|segment| segment.ident.as_str().to_string()) + .collect::>() + .join("::") + ); + let mut err = rustc_errors::struct_span_err!( + self.session, + path.span, + E0433, + "failed to resolve: {}", + label + ); + err.span_label(path.span, label); + err.note("this error was originally ignored because you are running `rustdoc`"); + err.note("try running again with `rustc` and you may get a more detailed error"); + err.emit(); + } + // NOTE: this does _not_ visit the path segments + } +} + /// `DefId` or parameter index (`ty::ParamTy.index`) of a synthetic type parameter /// for `impl Trait` in argument position. #[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 57151e2b20002..4bd6b1260ccef 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -94,6 +94,7 @@ pub fn main() { 32_000_000 // 32MB on other platforms }; rustc_driver::set_sigpipe_handler(); + rustc_driver::install_ice_hook(); env_logger::init_from_env("RUSTDOC_LOG"); let res = std::thread::Builder::new() .stack_size(thread_stack_size) diff --git a/src/test/rustdoc-ui/error-in-impl-trait.rs b/src/test/rustdoc-ui/error-in-impl-trait.rs new file mode 100644 index 0000000000000..fbe663a61890f --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait.rs @@ -0,0 +1,28 @@ +// edition:2018 +#![feature(type_alias_impl_trait)] + +pub trait ValidTrait {} +type ImplTrait = impl ValidTrait; + +/// This returns impl trait +pub fn g() -> impl ValidTrait { + error::_in::impl_trait() + //~^ ERROR failed to resolve +} + +/// This returns impl trait, but using a type alias +pub fn h() -> ImplTrait { + error::_in::impl_trait::alias(); + //~^ ERROR failed to resolve + (|| error::_in::impl_trait::alias::nested::closure())() + //~^ ERROR failed to resolve +} + +/// This used to work with ResolveBodyWithLoop. +/// However now that we ignore type checking instead of modifying the function body, +/// the return type is seen as `impl Future`, not a `u32`. +/// So it no longer allows errors in the function body. +pub async fn a() -> u32 { + error::_in::async_fn() + //~^ ERROR failed to resolve +} diff --git a/src/test/rustdoc-ui/error-in-impl-trait.stderr b/src/test/rustdoc-ui/error-in-impl-trait.stderr new file mode 100644 index 0000000000000..4df40da9b7cea --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait.stderr @@ -0,0 +1,39 @@ +error[E0433]: failed to resolve: could not resolve path `error::_in::impl_trait` + --> $DIR/error-in-impl-trait.rs:9:5 + | +LL | error::_in::impl_trait() + | ^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait` + | + = note: this error was originally ignored because you are running `rustdoc` + = note: try running again with `rustc` and you may get a more detailed error + +error[E0433]: failed to resolve: could not resolve path `error::_in::impl_trait::alias` + --> $DIR/error-in-impl-trait.rs:15:5 + | +LL | error::_in::impl_trait::alias(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait::alias` + | + = note: this error was originally ignored because you are running `rustdoc` + = note: try running again with `rustc` and you may get a more detailed error + +error[E0433]: failed to resolve: could not resolve path `error::_in::impl_trait::alias::nested::closure` + --> $DIR/error-in-impl-trait.rs:17:9 + | +LL | (|| error::_in::impl_trait::alias::nested::closure())() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait::alias::nested::closure` + | + = note: this error was originally ignored because you are running `rustdoc` + = note: try running again with `rustc` and you may get a more detailed error + +error[E0433]: failed to resolve: could not resolve path `error::_in::async_fn` + --> $DIR/error-in-impl-trait.rs:26:5 + | +LL | error::_in::async_fn() + | ^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::async_fn` + | + = note: this error was originally ignored because you are running `rustdoc` + = note: try running again with `rustc` and you may get a more detailed error + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0433`. diff --git a/src/test/rustdoc/impl-trait-alias.rs b/src/test/rustdoc/impl-trait-alias.rs new file mode 100644 index 0000000000000..54c3f856ddb3c --- /dev/null +++ b/src/test/rustdoc/impl-trait-alias.rs @@ -0,0 +1,14 @@ +#![feature(type_alias_impl_trait)] + +trait MyTrait {} +impl MyTrait for i32 {} + +// @has impl_trait_alias/type.Foo.html 'Foo' +/// debug type +pub type Foo = impl MyTrait; + +// @has impl_trait_alias/fn.foo.html 'foo' +/// debug function +pub fn foo() -> Foo { + 1 +} From a93bcc9a7b8e48865d3df59fc936a0553e4d1e37 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 9 Jul 2020 09:13:59 -0400 Subject: [PATCH 06/23] Recurse into function bodies, but don't typeck closures Previously, rustdoc would issue a delay_span_bug ICE on the following code: ```rust pub fn a() -> impl Fn() -> u32 { || content::doesnt::matter() } ``` This wasn't picked up earlier because having `type Alias = impl Trait;` in the same module caused _all closures_ to be typechecked, even if they wouldn't normally. Additionally, if _any_ error was emitted, no delay_span_bug would be emitted. So as part of this commit all of the tests were separated out into different files. --- src/librustdoc/core.rs | 28 ++++++++----- src/test/rustdoc-ui/error-in-impl-trait.rs | 28 ------------- .../rustdoc-ui/error-in-impl-trait.stderr | 39 ------------------- .../rustdoc-ui/error-in-impl-trait/README.md | 7 ++++ .../rustdoc-ui/error-in-impl-trait/async.rs | 10 +++++ .../error-in-impl-trait/async.stderr | 12 ++++++ .../rustdoc-ui/error-in-impl-trait/closure.rs | 5 +++ .../error-in-impl-trait/closure.stderr | 12 ++++++ .../impl-keyword-closure.rs | 6 +++ .../impl-keyword-closure.stderr | 12 ++++++ .../error-in-impl-trait/impl-keyword.rs | 6 +++ .../error-in-impl-trait/impl-keyword.stderr | 12 ++++++ .../trait-alias-closure.rs | 10 +++++ .../trait-alias-closure.stderr | 12 ++++++ .../error-in-impl-trait/trait-alias.rs | 10 +++++ .../error-in-impl-trait/trait-alias.stderr | 12 ++++++ 16 files changed, 145 insertions(+), 76 deletions(-) delete mode 100644 src/test/rustdoc-ui/error-in-impl-trait.rs delete mode 100644 src/test/rustdoc-ui/error-in-impl-trait.stderr create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/README.md create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/async.rs create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/async.stderr create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/closure.rs create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/closure.stderr create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/impl-keyword-closure.rs create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/impl-keyword-closure.stderr create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/impl-keyword.rs create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/impl-keyword.stderr create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/trait-alias-closure.rs create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/trait-alias-closure.stderr create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/trait-alias.rs create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/trait-alias.stderr diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 3d0da0e9157f7..413faff283e19 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -377,10 +377,18 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt external_providers.lint_mod = |_, _| {}; //let old_typeck = local_providers.typeck_tables_of; local_providers.typeck_tables_of = move |tcx, def_id| { + // Closures' tables come from their outermost function, + // as they are part of the same "inference environment". + // This avoids emitting errors for the parent twice (see similar code in `typeck_tables_of_with_fallback`) + let outer_def_id = tcx.closure_base_def_id(def_id.to_def_id()).expect_local(); + if outer_def_id != def_id { + return tcx.typeck_tables_of(outer_def_id); + } + let hir = tcx.hir(); let body = hir.body(hir.body_owned_by(hir.as_local_hir_id(def_id))); debug!("visiting body for {:?}", def_id); - EmitIgnoredResolutionErrors::new(&tcx.sess).visit_body(body); + EmitIgnoredResolutionErrors::new(&tcx.sess, hir).visit_body(body); rustc_typeck::check::typeck_tables_of(tcx, def_id) //DEFAULT_TYPECK.with(|typeck| typeck(tcx, def_id)) }; @@ -600,22 +608,24 @@ thread_local!(static DEFAULT_TYPECK: for<'tcx> fn(rustc_middle::ty::TyCtxt<'tcx> /// the name resolution pass may find errors that are never emitted. /// If typeck is called after this happens, then we'll get an ICE: /// 'Res::Error found but not reported'. To avoid this, emit the errors now. -struct EmitIgnoredResolutionErrors<'a> { +struct EmitIgnoredResolutionErrors<'a, 'hir> { session: &'a Session, + hir_map: Map<'hir>, } -impl<'a> EmitIgnoredResolutionErrors<'a> { - fn new(session: &'a Session) -> Self { - Self { session } +impl<'a, 'hir> EmitIgnoredResolutionErrors<'a, 'hir> { + fn new(session: &'a Session, hir_map: Map<'hir>) -> Self { + Self { session, hir_map } } } -impl<'a> Visitor<'a> for EmitIgnoredResolutionErrors<'_> { - type Map = Map<'a>; +impl<'hir> Visitor<'hir> for EmitIgnoredResolutionErrors<'_, 'hir> { + type Map = Map<'hir>; fn nested_visit_map(&mut self) -> NestedVisitorMap { - // If we visit nested bodies, then we will report errors twice for e.g. nested closures - NestedVisitorMap::None + // We need to recurse into nested closures, + // since those will fallback to the parent for type checking. + NestedVisitorMap::OnlyBodies(self.hir_map) } fn visit_path(&mut self, path: &'v Path<'v>, _id: HirId) { diff --git a/src/test/rustdoc-ui/error-in-impl-trait.rs b/src/test/rustdoc-ui/error-in-impl-trait.rs deleted file mode 100644 index fbe663a61890f..0000000000000 --- a/src/test/rustdoc-ui/error-in-impl-trait.rs +++ /dev/null @@ -1,28 +0,0 @@ -// edition:2018 -#![feature(type_alias_impl_trait)] - -pub trait ValidTrait {} -type ImplTrait = impl ValidTrait; - -/// This returns impl trait -pub fn g() -> impl ValidTrait { - error::_in::impl_trait() - //~^ ERROR failed to resolve -} - -/// This returns impl trait, but using a type alias -pub fn h() -> ImplTrait { - error::_in::impl_trait::alias(); - //~^ ERROR failed to resolve - (|| error::_in::impl_trait::alias::nested::closure())() - //~^ ERROR failed to resolve -} - -/// This used to work with ResolveBodyWithLoop. -/// However now that we ignore type checking instead of modifying the function body, -/// the return type is seen as `impl Future`, not a `u32`. -/// So it no longer allows errors in the function body. -pub async fn a() -> u32 { - error::_in::async_fn() - //~^ ERROR failed to resolve -} diff --git a/src/test/rustdoc-ui/error-in-impl-trait.stderr b/src/test/rustdoc-ui/error-in-impl-trait.stderr deleted file mode 100644 index 4df40da9b7cea..0000000000000 --- a/src/test/rustdoc-ui/error-in-impl-trait.stderr +++ /dev/null @@ -1,39 +0,0 @@ -error[E0433]: failed to resolve: could not resolve path `error::_in::impl_trait` - --> $DIR/error-in-impl-trait.rs:9:5 - | -LL | error::_in::impl_trait() - | ^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait` - | - = note: this error was originally ignored because you are running `rustdoc` - = note: try running again with `rustc` and you may get a more detailed error - -error[E0433]: failed to resolve: could not resolve path `error::_in::impl_trait::alias` - --> $DIR/error-in-impl-trait.rs:15:5 - | -LL | error::_in::impl_trait::alias(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait::alias` - | - = note: this error was originally ignored because you are running `rustdoc` - = note: try running again with `rustc` and you may get a more detailed error - -error[E0433]: failed to resolve: could not resolve path `error::_in::impl_trait::alias::nested::closure` - --> $DIR/error-in-impl-trait.rs:17:9 - | -LL | (|| error::_in::impl_trait::alias::nested::closure())() - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait::alias::nested::closure` - | - = note: this error was originally ignored because you are running `rustdoc` - = note: try running again with `rustc` and you may get a more detailed error - -error[E0433]: failed to resolve: could not resolve path `error::_in::async_fn` - --> $DIR/error-in-impl-trait.rs:26:5 - | -LL | error::_in::async_fn() - | ^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::async_fn` - | - = note: this error was originally ignored because you are running `rustdoc` - = note: try running again with `rustc` and you may get a more detailed error - -error: aborting due to 4 previous errors - -For more information about this error, try `rustc --explain E0433`. diff --git a/src/test/rustdoc-ui/error-in-impl-trait/README.md b/src/test/rustdoc-ui/error-in-impl-trait/README.md new file mode 100644 index 0000000000000..1176a4a8c4cf8 --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/README.md @@ -0,0 +1,7 @@ +Each of these needs to be in a separate file, +because the `delay_span_bug` ICE in rustdoc won't be triggerred +if even a single other error was emitted. + +However, conceptually they are all testing basically the same thing. +See https://github.com/rust-lang/rust/pull/73566#issuecomment-653689128 +for more details. diff --git a/src/test/rustdoc-ui/error-in-impl-trait/async.rs b/src/test/rustdoc-ui/error-in-impl-trait/async.rs new file mode 100644 index 0000000000000..112a2c494a5c2 --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/async.rs @@ -0,0 +1,10 @@ +// edition:2018 + +/// This used to work with ResolveBodyWithLoop. +/// However now that we ignore type checking instead of modifying the function body, +/// the return type is seen as `impl Future`, not a `u32`. +/// So it no longer allows errors in the function body. +pub async fn a() -> u32 { + error::_in::async_fn() + //~^ ERROR failed to resolve +} diff --git a/src/test/rustdoc-ui/error-in-impl-trait/async.stderr b/src/test/rustdoc-ui/error-in-impl-trait/async.stderr new file mode 100644 index 0000000000000..eae3cadf653e1 --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/async.stderr @@ -0,0 +1,12 @@ +error[E0433]: failed to resolve: could not resolve path `error::_in::async_fn` + --> $DIR/async.rs:8:5 + | +LL | error::_in::async_fn() + | ^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::async_fn` + | + = note: this error was originally ignored because you are running `rustdoc` + = note: try running again with `rustc` and you may get a more detailed error + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0433`. diff --git a/src/test/rustdoc-ui/error-in-impl-trait/closure.rs b/src/test/rustdoc-ui/error-in-impl-trait/closure.rs new file mode 100644 index 0000000000000..df40c121d579e --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/closure.rs @@ -0,0 +1,5 @@ +// manually desugared version of an `async fn` (but with a closure instead of a generator) +pub fn a() -> impl Fn() -> u32 { + || content::doesnt::matter() + //~^ ERROR failed to resolve +} diff --git a/src/test/rustdoc-ui/error-in-impl-trait/closure.stderr b/src/test/rustdoc-ui/error-in-impl-trait/closure.stderr new file mode 100644 index 0000000000000..9355165997ac9 --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/closure.stderr @@ -0,0 +1,12 @@ +error[E0433]: failed to resolve: could not resolve path `content::doesnt::matter` + --> $DIR/closure.rs:3:8 + | +LL | || content::doesnt::matter() + | ^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `content::doesnt::matter` + | + = note: this error was originally ignored because you are running `rustdoc` + = note: try running again with `rustc` and you may get a more detailed error + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0433`. diff --git a/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword-closure.rs b/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword-closure.rs new file mode 100644 index 0000000000000..399fb827517fa --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword-closure.rs @@ -0,0 +1,6 @@ +pub trait ValidTrait {} +/// This returns impl trait +pub fn g() -> impl ValidTrait { + (|| error::_in::impl_trait::alias::nested::closure())() + //~^ ERROR failed to resolve +} diff --git a/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword-closure.stderr b/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword-closure.stderr new file mode 100644 index 0000000000000..569f2ab8ff8ea --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword-closure.stderr @@ -0,0 +1,12 @@ +error[E0433]: failed to resolve: could not resolve path `error::_in::impl_trait::alias::nested::closure` + --> $DIR/impl-keyword-closure.rs:4:9 + | +LL | (|| error::_in::impl_trait::alias::nested::closure())() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait::alias::nested::closure` + | + = note: this error was originally ignored because you are running `rustdoc` + = note: try running again with `rustc` and you may get a more detailed error + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0433`. diff --git a/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword.rs b/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword.rs new file mode 100644 index 0000000000000..24b5734dbd0bf --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword.rs @@ -0,0 +1,6 @@ +pub trait ValidTrait {} +/// This returns impl trait +pub fn g() -> impl ValidTrait { + error::_in::impl_trait() + //~^ ERROR failed to resolve +} diff --git a/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword.stderr b/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword.stderr new file mode 100644 index 0000000000000..68bc71f90b288 --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword.stderr @@ -0,0 +1,12 @@ +error[E0433]: failed to resolve: could not resolve path `error::_in::impl_trait` + --> $DIR/impl-keyword.rs:4:5 + | +LL | error::_in::impl_trait() + | ^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait` + | + = note: this error was originally ignored because you are running `rustdoc` + = note: try running again with `rustc` and you may get a more detailed error + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0433`. diff --git a/src/test/rustdoc-ui/error-in-impl-trait/trait-alias-closure.rs b/src/test/rustdoc-ui/error-in-impl-trait/trait-alias-closure.rs new file mode 100644 index 0000000000000..1498fa4f890d0 --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/trait-alias-closure.rs @@ -0,0 +1,10 @@ +#![feature(type_alias_impl_trait)] + +pub trait ValidTrait {} +type ImplTrait = impl ValidTrait; + +/// This returns impl trait, but using a type alias +pub fn h() -> ImplTrait { + (|| error::_in::impl_trait::alias::nested::closure())() + //~^ ERROR failed to resolve +} diff --git a/src/test/rustdoc-ui/error-in-impl-trait/trait-alias-closure.stderr b/src/test/rustdoc-ui/error-in-impl-trait/trait-alias-closure.stderr new file mode 100644 index 0000000000000..f3edb0385c821 --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/trait-alias-closure.stderr @@ -0,0 +1,12 @@ +error[E0433]: failed to resolve: could not resolve path `error::_in::impl_trait::alias::nested::closure` + --> $DIR/trait-alias-closure.rs:8:9 + | +LL | (|| error::_in::impl_trait::alias::nested::closure())() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait::alias::nested::closure` + | + = note: this error was originally ignored because you are running `rustdoc` + = note: try running again with `rustc` and you may get a more detailed error + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0433`. diff --git a/src/test/rustdoc-ui/error-in-impl-trait/trait-alias.rs b/src/test/rustdoc-ui/error-in-impl-trait/trait-alias.rs new file mode 100644 index 0000000000000..cf9bc48c7f872 --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/trait-alias.rs @@ -0,0 +1,10 @@ +#![feature(type_alias_impl_trait)] + +pub trait ValidTrait {} +type ImplTrait = impl ValidTrait; + +/// This returns impl trait, but using a type alias +pub fn h() -> ImplTrait { + error::_in::impl_trait::alias() + //~^ ERROR failed to resolve +} diff --git a/src/test/rustdoc-ui/error-in-impl-trait/trait-alias.stderr b/src/test/rustdoc-ui/error-in-impl-trait/trait-alias.stderr new file mode 100644 index 0000000000000..ddb0fb88cc7fa --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/trait-alias.stderr @@ -0,0 +1,12 @@ +error[E0433]: failed to resolve: could not resolve path `error::_in::impl_trait::alias` + --> $DIR/trait-alias.rs:8:5 + | +LL | error::_in::impl_trait::alias() + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait::alias` + | + = note: this error was originally ignored because you are running `rustdoc` + = note: try running again with `rustc` and you may get a more detailed error + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0433`. From d01044305a5f2eb177521f51a7d7bfaee1ccf688 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 9 Jul 2020 10:30:34 -0400 Subject: [PATCH 07/23] Add test case for #65863 --- src/test/rustdoc/return-impl-trait.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 src/test/rustdoc/return-impl-trait.rs diff --git a/src/test/rustdoc/return-impl-trait.rs b/src/test/rustdoc/return-impl-trait.rs new file mode 100644 index 0000000000000..1ccf5ac46119a --- /dev/null +++ b/src/test/rustdoc/return-impl-trait.rs @@ -0,0 +1,15 @@ +#![feature(type_alias_impl_trait)] + +pub trait Backend {} + +impl Backend for () {} + +pub struct Module(T); + +pub type BackendImpl = impl Backend; + +// @has return_impl_trait/fn.make_module.html +/// Documentation +pub fn make_module() -> Module { + Module(()) +} From cf844d2eabc8929edb0923d71ec6ff076ac3428b Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 9 Jul 2020 22:11:15 -0400 Subject: [PATCH 08/23] Don't make typeck_tables_of public --- src/librustc_typeck/check/mod.rs | 2 +- src/librustc_typeck/lib.rs | 2 +- src/librustdoc/core.rs | 6 +----- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 514600b4733d4..bc01da324b66f 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -955,7 +955,7 @@ where val.fold_with(&mut FixupFolder { tcx }) } -pub fn typeck_tables_of<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> &ty::TypeckTables<'tcx> { +fn typeck_tables_of<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> &ty::TypeckTables<'tcx> { let fallback = move || tcx.type_of(def_id.to_def_id()); typeck_tables_of_with_fallback(tcx, def_id, fallback) } diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index 79e1585ce3cdb..9ba2545ba63cb 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -78,7 +78,7 @@ extern crate rustc_middle; pub mod expr_use_visitor; mod astconv; -pub mod check; +mod check; mod check_unused; mod coherence; mod collect; diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 413faff283e19..78b4456ba9c6c 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -375,7 +375,6 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt override_queries: Some(|_sess, local_providers, external_providers| { local_providers.lint_mod = |_, _| {}; external_providers.lint_mod = |_, _| {}; - //let old_typeck = local_providers.typeck_tables_of; local_providers.typeck_tables_of = move |tcx, def_id| { // Closures' tables come from their outermost function, // as they are part of the same "inference environment". @@ -389,8 +388,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt let body = hir.body(hir.body_owned_by(hir.as_local_hir_id(def_id))); debug!("visiting body for {:?}", def_id); EmitIgnoredResolutionErrors::new(&tcx.sess, hir).visit_body(body); - rustc_typeck::check::typeck_tables_of(tcx, def_id) - //DEFAULT_TYPECK.with(|typeck| typeck(tcx, def_id)) + DEFAULT_TYPECK.with(|typeck| typeck(tcx, def_id)) }; }), registry: rustc_driver::diagnostics_registry(), @@ -596,13 +594,11 @@ use rustc_hir::{ }; use rustc_middle::hir::map::Map; -/* thread_local!(static DEFAULT_TYPECK: for<'tcx> fn(rustc_middle::ty::TyCtxt<'tcx>, rustc_span::def_id::LocalDefId) -> &'tcx rustc_middle::ty::TypeckTables<'tcx> = { let mut providers = rustc_middle::ty::query::Providers::default(); rustc_typeck::provide(&mut providers); providers.typeck_tables_of }); -*/ /// Due to https://github.com/rust-lang/rust/pull/73566, /// the name resolution pass may find errors that are never emitted. From 0cbc1cddcc6b9657fb727e35dce753d38e52cc52 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 10 Jul 2020 16:55:06 -0400 Subject: [PATCH 09/23] Avoid unnecessary enum Just use a boolean instead. --- src/librustc_interface/passes.rs | 7 +--- src/librustc_resolve/late.rs | 56 ++++++++++---------------------- src/librustc_resolve/lib.rs | 5 ++- 3 files changed, 20 insertions(+), 48 deletions(-) diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 690ed9decb9ef..6505803eba8c6 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -233,8 +233,6 @@ 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); @@ -413,10 +411,7 @@ fn configure_and_expand_inner<'a>( println!("{}", json::as_json(&krate)); } - // If we're actually rustdoc then avoid giving a name resolution error for `cfg()` items. - let ignore_bodies = - if sess.opts.actually_rustdoc { IgnoreState::Ignore } else { IgnoreState::Report }; - resolver.resolve_crate(&krate, ignore_bodies); + resolver.resolve_crate(&krate); // Needs to go *after* expansion to be able to check the results of macro expansion. sess.time("complete_gated_feature_checking", || { diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index 637326bb88d86..528444b0e9894 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -376,19 +376,6 @@ struct DiagnosticMetadata<'ast> { current_let_binding: Option<(Span, Option, Option)>, } -/// 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>, @@ -408,12 +395,12 @@ struct LateResolutionVisitor<'a, 'b, 'ast> { /// Fields used to add information to diagnostic errors. diagnostic_metadata: DiagnosticMetadata<'ast>, - /// State used to know whether to ignore resolution errors for item bodies. + /// State used to know whether to ignore resolution errors for function bodies. /// /// In particular, rustdoc uses this to avoid giving errors for `cfg()` items. /// 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, + in_func_body: bool, } /// Walks the whole crate in DFS order, visiting each item, resolving names as it goes. @@ -517,10 +504,10 @@ 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` + let previous_state = this.in_func_body; + // Ignore errors in function bodies if this is rustdoc // Be sure not to set this until the function signature has been resolved. - this.ignore_bodies = previous_ignore.and(Some(IgnoreState::Ignore)); + this.in_func_body = true; // Resolve the function body, potentially inside the body of an async closure match fn_kind { FnKind::Fn(.., body) => walk_list!(this, visit_block, body), @@ -528,7 +515,7 @@ impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> { }; debug!("(resolving function) leaving function"); - this.ignore_bodies = previous_ignore; + this.in_func_body = previous_state; }) }); self.diagnostic_metadata.current_function = previous_value; @@ -652,10 +639,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: IgnoreState, - ) -> LateResolutionVisitor<'a, 'b, 'ast> { + fn new(resolver: &'b mut Resolver<'a>) -> 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. let graph_root = resolver.graph_root; @@ -672,11 +656,8 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { label_ribs: Vec::new(), current_trait_ref: None, diagnostic_metadata: DiagnosticMetadata::default(), - ignore_bodies: match ignore_bodies { - // errors at module scope should always be reported - IgnoreState::Ignore => Some(IgnoreState::Report), - IgnoreState::Report => None, - }, + // errors at module scope should always be reported + in_func_body: false, } } @@ -1194,9 +1175,9 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { impl_items: &'ast [P], ) { debug!("resolve_implementation"); - let old_ignore = self.ignore_bodies.take(); + let old_ignore = self.in_func_body; // Never ignore errors in trait implementations. - self.ignore_bodies = old_ignore.and(Some(IgnoreState::Report)); + self.in_func_body = false; // 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. @@ -1292,7 +1273,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { }); }); }); - self.ignore_bodies = old_ignore; + self.in_func_body = old_ignore; } fn check_trait_item(&mut self, ident: Ident, ns: Namespace, span: Span, err: F) @@ -1900,7 +1881,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { /// A wrapper around [`Resolver::report_error`]. /// - /// This doesn't emit errors for function bodies if `ignore_bodies` is set. + /// This doesn't emit errors for function bodies if this is r fn report_error(&self, span: Span, resolution_error: ResolutionError<'_>) { if self.should_report_errs() { self.r.report_error(span, resolution_error); @@ -1908,12 +1889,9 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { } #[inline] + /// If we're actually rustdoc then avoid giving a name resolution error for `cfg()` items. 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, - } + !(self.r.session.opts.actually_rustdoc && self.in_func_body) } // Resolve in alternative namespaces if resolution in the primary namespace fails. @@ -2412,8 +2390,8 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { } impl<'a> Resolver<'a> { - pub(crate) fn late_resolve_crate(&mut self, krate: &Crate, ignore_bodies: IgnoreState) { - let mut late_resolution_visitor = LateResolutionVisitor::new(self, ignore_bodies); + pub(crate) fn late_resolve_crate(&mut self, krate: &Crate) { + let mut late_resolution_visitor = LateResolutionVisitor::new(self); visit::walk_crate(&mut late_resolution_visitor, krate); for (id, span) in late_resolution_visitor.diagnostic_metadata.unused_labels.iter() { self.lint_buffer.buffer_lint(lint::builtin::UNUSED_LABELS, *id, *span, "unused label"); diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 23bd0028bd1dd..a265c15c18bc9 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -15,7 +15,6 @@ #![feature(or_patterns)] #![recursion_limit = "256"] -pub use late::IgnoreState; pub use rustc_hir::def::{Namespace, PerNS}; use Determinacy::*; @@ -1442,13 +1441,13 @@ impl<'a> Resolver<'a> { } /// Entry point to crate resolution. - pub fn resolve_crate(&mut self, krate: &Crate, ignore_bodies: IgnoreState) { + pub fn resolve_crate(&mut self, krate: &Crate) { let _prof_timer = self.session.prof.generic_activity("resolve_crate"); ImportResolver { r: self }.finalize_imports(); self.finalize_macro_resolutions(); - self.late_resolve_crate(krate, ignore_bodies); + self.late_resolve_crate(krate); self.check_unused(krate); self.report_errors(krate); From 3576f5d7e153d10aae36b2be067bc6243a4c77db Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 10 Jul 2020 17:24:17 -0400 Subject: [PATCH 10/23] Address review comments about code style --- src/librustdoc/core.rs | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 78b4456ba9c6c..d5389d06906b8 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -5,13 +5,18 @@ use rustc_driver::abort_on_err; use rustc_errors::emitter::{Emitter, EmitterWriter}; use rustc_errors::json::JsonEmitter; use rustc_feature::UnstableFeatures; -use rustc_hir::def::Namespace::TypeNS; +use rustc_hir::def::{Namespace::TypeNS, Res}; use rustc_hir::def_id::{CrateNum, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc_hir::HirId; +use rustc_hir::{ + intravisit::{NestedVisitorMap, Visitor}, + Path, +}; use rustc_interface::interface; +use rustc_middle::hir::map::Map; use rustc_middle::middle::cstore::CrateStore; use rustc_middle::middle::privacy::AccessLevels; -use rustc_middle::ty::{Ty, TyCtxt}; +use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_resolve as resolve; use rustc_session::config::{self, CrateType, ErrorOutputType}; use rustc_session::lint; @@ -587,15 +592,8 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt }) } -use rustc_hir::def::Res; -use rustc_hir::{ - intravisit::{NestedVisitorMap, Visitor}, - Path, -}; -use rustc_middle::hir::map::Map; - -thread_local!(static DEFAULT_TYPECK: for<'tcx> fn(rustc_middle::ty::TyCtxt<'tcx>, rustc_span::def_id::LocalDefId) -> &'tcx rustc_middle::ty::TypeckTables<'tcx> = { - let mut providers = rustc_middle::ty::query::Providers::default(); +thread_local!(static DEFAULT_TYPECK: for<'tcx> fn(TyCtxt<'tcx>, LocalDefId) -> &'tcx ty::TypeckTables<'tcx> = { + let mut providers = ty::query::Providers::default(); rustc_typeck::provide(&mut providers); providers.typeck_tables_of }); @@ -625,13 +623,11 @@ impl<'hir> Visitor<'hir> for EmitIgnoredResolutionErrors<'_, 'hir> { } fn visit_path(&mut self, path: &'v Path<'v>, _id: HirId) { - log::debug!("visiting path {:?}", path); + debug!("visiting path {:?}", path); if path.res == Res::Err { // We have less context here than in rustc_resolve, // so we can only emit the name and span. // However we can give a hint that rustc_resolve will have more info. - // NOTE: this is a very rare case (only 4 out of several hundred thousand crates in a crater run) - // NOTE: so it's ok for it to be slow let label = format!( "could not resolve path `{}`", path.segments From bbe4971095717912463d8dbc00ba8ce9a5988963 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 10 Jul 2020 17:50:03 -0400 Subject: [PATCH 11/23] Don't crash on Vec --- src/librustdoc/core.rs | 9 ++++++--- .../rustdoc-ui/error-in-impl-trait/generic-argument.rs | 7 +++++++ 2 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/generic-argument.rs diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index d5389d06906b8..adc6b536699eb 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -9,7 +9,7 @@ use rustc_hir::def::{Namespace::TypeNS, Res}; use rustc_hir::def_id::{CrateNum, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc_hir::HirId; use rustc_hir::{ - intravisit::{NestedVisitorMap, Visitor}, + intravisit::{self, NestedVisitorMap, Visitor}, Path, }; use rustc_interface::interface; @@ -622,7 +622,7 @@ impl<'hir> Visitor<'hir> for EmitIgnoredResolutionErrors<'_, 'hir> { NestedVisitorMap::OnlyBodies(self.hir_map) } - fn visit_path(&mut self, path: &'v Path<'v>, _id: HirId) { + fn visit_path(&mut self, path: &'hir Path<'_>, _id: HirId) { debug!("visiting path {:?}", path); if path.res == Res::Err { // We have less context here than in rustc_resolve, @@ -648,7 +648,10 @@ impl<'hir> Visitor<'hir> for EmitIgnoredResolutionErrors<'_, 'hir> { err.note("try running again with `rustc` and you may get a more detailed error"); err.emit(); } - // NOTE: this does _not_ visit the path segments + // We could have an outer resolution that succeeded, + // but with generic parameters that failed. + // Recurse into the segments so we catch those too. + intravisit::walk_path(self, path); } } diff --git a/src/test/rustdoc-ui/error-in-impl-trait/generic-argument.rs b/src/test/rustdoc-ui/error-in-impl-trait/generic-argument.rs new file mode 100644 index 0000000000000..0ccf2e3866fc9 --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/generic-argument.rs @@ -0,0 +1,7 @@ +trait ValidTrait {} + +/// This has docs +pub fn f() -> impl ValidTrait { + Vec::::new() + //~^ ERROR failed to resolve +} From 2f29e696ab0ced54f016bed0514a53f6e281ac8a Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 10 Jul 2020 17:51:38 -0400 Subject: [PATCH 12/23] Mention `cargo check` in help message --- src/librustdoc/core.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index adc6b536699eb..bdacf608ff59d 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -645,7 +645,7 @@ impl<'hir> Visitor<'hir> for EmitIgnoredResolutionErrors<'_, 'hir> { ); err.span_label(path.span, label); err.note("this error was originally ignored because you are running `rustdoc`"); - err.note("try running again with `rustc` and you may get a more detailed error"); + err.note("try running again with `rustc` or `cargo check` and you may get a more detailed error"); err.emit(); } // We could have an outer resolution that succeeded, From 763d373dabb7ccf581737749a2a1adec335d8249 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 10 Jul 2020 17:59:29 -0400 Subject: [PATCH 13/23] Use tcx as the only context for visitor Previously two different parts of the context had to be passed separately; there were two sources of truth. --- src/librustdoc/core.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index bdacf608ff59d..b87d7b19dcd5e 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -392,7 +392,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt let hir = tcx.hir(); let body = hir.body(hir.body_owned_by(hir.as_local_hir_id(def_id))); debug!("visiting body for {:?}", def_id); - EmitIgnoredResolutionErrors::new(&tcx.sess, hir).visit_body(body); + EmitIgnoredResolutionErrors::new(&tcx).visit_body(body); DEFAULT_TYPECK.with(|typeck| typeck(tcx, def_id)) }; }), @@ -602,27 +602,26 @@ thread_local!(static DEFAULT_TYPECK: for<'tcx> fn(TyCtxt<'tcx>, LocalDefId) -> & /// the name resolution pass may find errors that are never emitted. /// If typeck is called after this happens, then we'll get an ICE: /// 'Res::Error found but not reported'. To avoid this, emit the errors now. -struct EmitIgnoredResolutionErrors<'a, 'hir> { - session: &'a Session, - hir_map: Map<'hir>, +struct EmitIgnoredResolutionErrors<'a, 'tcx> { + tcx: &'a TyCtxt<'tcx>, } -impl<'a, 'hir> EmitIgnoredResolutionErrors<'a, 'hir> { - fn new(session: &'a Session, hir_map: Map<'hir>) -> Self { - Self { session, hir_map } +impl<'a, 'tcx> EmitIgnoredResolutionErrors<'a, 'tcx> { + fn new(tcx: &'a TyCtxt<'tcx>) -> Self { + Self { tcx } } } -impl<'hir> Visitor<'hir> for EmitIgnoredResolutionErrors<'_, 'hir> { - type Map = Map<'hir>; +impl<'tcx> Visitor<'tcx> for EmitIgnoredResolutionErrors<'_, 'tcx> { + type Map = Map<'tcx>; fn nested_visit_map(&mut self) -> NestedVisitorMap { // We need to recurse into nested closures, // since those will fallback to the parent for type checking. - NestedVisitorMap::OnlyBodies(self.hir_map) + NestedVisitorMap::OnlyBodies(self.tcx.hir()) } - fn visit_path(&mut self, path: &'hir Path<'_>, _id: HirId) { + fn visit_path(&mut self, path: &'tcx Path<'_>, _id: HirId) { debug!("visiting path {:?}", path); if path.res == Res::Err { // We have less context here than in rustc_resolve, @@ -637,7 +636,7 @@ impl<'hir> Visitor<'hir> for EmitIgnoredResolutionErrors<'_, 'hir> { .join("::") ); let mut err = rustc_errors::struct_span_err!( - self.session, + self.tcx.sess, path.span, E0433, "failed to resolve: {}", From 0759a55feff2d7c4a15b563adc087ac4f59acb1b Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 10 Jul 2020 18:07:31 -0400 Subject: [PATCH 14/23] Remove unnecessary lifetime parameter TyCtxt is a reference type and so can be passed by value. --- src/librustdoc/core.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index b87d7b19dcd5e..39c214b1fb42b 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -392,7 +392,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt let hir = tcx.hir(); let body = hir.body(hir.body_owned_by(hir.as_local_hir_id(def_id))); debug!("visiting body for {:?}", def_id); - EmitIgnoredResolutionErrors::new(&tcx).visit_body(body); + EmitIgnoredResolutionErrors::new(tcx).visit_body(body); DEFAULT_TYPECK.with(|typeck| typeck(tcx, def_id)) }; }), @@ -602,17 +602,17 @@ thread_local!(static DEFAULT_TYPECK: for<'tcx> fn(TyCtxt<'tcx>, LocalDefId) -> & /// the name resolution pass may find errors that are never emitted. /// If typeck is called after this happens, then we'll get an ICE: /// 'Res::Error found but not reported'. To avoid this, emit the errors now. -struct EmitIgnoredResolutionErrors<'a, 'tcx> { - tcx: &'a TyCtxt<'tcx>, +struct EmitIgnoredResolutionErrors<'tcx> { + tcx: TyCtxt<'tcx>, } -impl<'a, 'tcx> EmitIgnoredResolutionErrors<'a, 'tcx> { - fn new(tcx: &'a TyCtxt<'tcx>) -> Self { +impl<'tcx> EmitIgnoredResolutionErrors<'tcx> { + fn new(tcx: TyCtxt<'tcx>) -> Self { Self { tcx } } } -impl<'tcx> Visitor<'tcx> for EmitIgnoredResolutionErrors<'_, 'tcx> { +impl<'tcx> Visitor<'tcx> for EmitIgnoredResolutionErrors<'tcx> { type Map = Map<'tcx>; fn nested_visit_map(&mut self) -> NestedVisitorMap { From 2d0e8e2162a2e2be233a63ba5a8cbf3e19770b17 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 10 Jul 2020 19:00:33 -0400 Subject: [PATCH 15/23] --bless --- src/test/rustdoc-ui/error-in-impl-trait/async.stderr | 2 +- .../rustdoc-ui/error-in-impl-trait/closure.stderr | 2 +- .../error-in-impl-trait/generic-argument.stderr | 12 ++++++++++++ .../error-in-impl-trait/impl-keyword-closure.stderr | 2 +- .../error-in-impl-trait/impl-keyword.stderr | 2 +- .../error-in-impl-trait/trait-alias-closure.stderr | 2 +- .../error-in-impl-trait/trait-alias.stderr | 2 +- 7 files changed, 18 insertions(+), 6 deletions(-) create mode 100644 src/test/rustdoc-ui/error-in-impl-trait/generic-argument.stderr diff --git a/src/test/rustdoc-ui/error-in-impl-trait/async.stderr b/src/test/rustdoc-ui/error-in-impl-trait/async.stderr index eae3cadf653e1..086db1be72274 100644 --- a/src/test/rustdoc-ui/error-in-impl-trait/async.stderr +++ b/src/test/rustdoc-ui/error-in-impl-trait/async.stderr @@ -5,7 +5,7 @@ LL | error::_in::async_fn() | ^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::async_fn` | = note: this error was originally ignored because you are running `rustdoc` - = note: try running again with `rustc` and you may get a more detailed error + = note: try running again with `rustc` or `cargo check` and you may get a more detailed error error: aborting due to previous error diff --git a/src/test/rustdoc-ui/error-in-impl-trait/closure.stderr b/src/test/rustdoc-ui/error-in-impl-trait/closure.stderr index 9355165997ac9..4ee9c4d1f438d 100644 --- a/src/test/rustdoc-ui/error-in-impl-trait/closure.stderr +++ b/src/test/rustdoc-ui/error-in-impl-trait/closure.stderr @@ -5,7 +5,7 @@ LL | || content::doesnt::matter() | ^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `content::doesnt::matter` | = note: this error was originally ignored because you are running `rustdoc` - = note: try running again with `rustc` and you may get a more detailed error + = note: try running again with `rustc` or `cargo check` and you may get a more detailed error error: aborting due to previous error diff --git a/src/test/rustdoc-ui/error-in-impl-trait/generic-argument.stderr b/src/test/rustdoc-ui/error-in-impl-trait/generic-argument.stderr new file mode 100644 index 0000000000000..72716c258dc1e --- /dev/null +++ b/src/test/rustdoc-ui/error-in-impl-trait/generic-argument.stderr @@ -0,0 +1,12 @@ +error[E0433]: failed to resolve: could not resolve path `DoesNotExist` + --> $DIR/generic-argument.rs:5:11 + | +LL | Vec::::new() + | ^^^^^^^^^^^^ could not resolve path `DoesNotExist` + | + = note: this error was originally ignored because you are running `rustdoc` + = note: try running again with `rustc` or `cargo check` and you may get a more detailed error + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0433`. diff --git a/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword-closure.stderr b/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword-closure.stderr index 569f2ab8ff8ea..55f9b609a1105 100644 --- a/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword-closure.stderr +++ b/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword-closure.stderr @@ -5,7 +5,7 @@ LL | (|| error::_in::impl_trait::alias::nested::closure())() | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait::alias::nested::closure` | = note: this error was originally ignored because you are running `rustdoc` - = note: try running again with `rustc` and you may get a more detailed error + = note: try running again with `rustc` or `cargo check` and you may get a more detailed error error: aborting due to previous error diff --git a/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword.stderr b/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword.stderr index 68bc71f90b288..3257079f94219 100644 --- a/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword.stderr +++ b/src/test/rustdoc-ui/error-in-impl-trait/impl-keyword.stderr @@ -5,7 +5,7 @@ LL | error::_in::impl_trait() | ^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait` | = note: this error was originally ignored because you are running `rustdoc` - = note: try running again with `rustc` and you may get a more detailed error + = note: try running again with `rustc` or `cargo check` and you may get a more detailed error error: aborting due to previous error diff --git a/src/test/rustdoc-ui/error-in-impl-trait/trait-alias-closure.stderr b/src/test/rustdoc-ui/error-in-impl-trait/trait-alias-closure.stderr index f3edb0385c821..84b28139dbcd5 100644 --- a/src/test/rustdoc-ui/error-in-impl-trait/trait-alias-closure.stderr +++ b/src/test/rustdoc-ui/error-in-impl-trait/trait-alias-closure.stderr @@ -5,7 +5,7 @@ LL | (|| error::_in::impl_trait::alias::nested::closure())() | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait::alias::nested::closure` | = note: this error was originally ignored because you are running `rustdoc` - = note: try running again with `rustc` and you may get a more detailed error + = note: try running again with `rustc` or `cargo check` and you may get a more detailed error error: aborting due to previous error diff --git a/src/test/rustdoc-ui/error-in-impl-trait/trait-alias.stderr b/src/test/rustdoc-ui/error-in-impl-trait/trait-alias.stderr index ddb0fb88cc7fa..9be6a3d8d6bba 100644 --- a/src/test/rustdoc-ui/error-in-impl-trait/trait-alias.stderr +++ b/src/test/rustdoc-ui/error-in-impl-trait/trait-alias.stderr @@ -5,7 +5,7 @@ LL | error::_in::impl_trait::alias() | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait::alias` | = note: this error was originally ignored because you are running `rustdoc` - = note: try running again with `rustc` and you may get a more detailed error + = note: try running again with `rustc` or `cargo check` and you may get a more detailed error error: aborting due to previous error From 02a24c8e2fd370041a24b7d93e8c3710b7b76015 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 11 Jul 2020 00:28:42 -0400 Subject: [PATCH 16/23] Don't ICE on infinitely recursive types `evaluate_obligation` can only be run on types that are already valid. So rustdoc still has to run typeck even though it doesn't care about the result. --- Cargo.lock | 1 + src/librustdoc/Cargo.toml | 1 + src/librustdoc/core.rs | 15 +++++++++++++++ src/librustdoc/lib.rs | 2 ++ src/test/rustdoc-ui/infinite-recursive-type.rs | 4 ++++ .../rustdoc-ui/infinite-recursive-type.stderr | 17 +++++++++++++++++ 6 files changed, 40 insertions(+) create mode 100644 src/test/rustdoc-ui/infinite-recursive-type.rs create mode 100644 src/test/rustdoc-ui/infinite-recursive-type.stderr diff --git a/Cargo.lock b/Cargo.lock index 5309c03ee23ae..992421dcd7abb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4027,6 +4027,7 @@ name = "rustdoc" version = "0.0.0" dependencies = [ "itertools 0.8.0", + "lazy_static", "minifier", "pulldown-cmark", "rustc-rayon", diff --git a/src/librustdoc/Cargo.toml b/src/librustdoc/Cargo.toml index 4af13e4cd587a..baceb13cc6141 100644 --- a/src/librustdoc/Cargo.toml +++ b/src/librustdoc/Cargo.toml @@ -16,3 +16,4 @@ serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" tempfile = "3" itertools = "0.8" +lazy_static = "1" diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 39c214b1fb42b..a77b177bd2864 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -364,6 +364,9 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt ..Options::default() }; + lazy_static! { + static ref EMPTY_MAP: FxHashSet = FxHashSet::default(); + } let config = interface::Config { opts: sessopts, crate_cfg: interface::parse_cfgspecs(cfgs), @@ -378,8 +381,13 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt lint_caps, register_lints: None, override_queries: Some(|_sess, local_providers, external_providers| { + // Most lints will require typechecking, so just don't run them. local_providers.lint_mod = |_, _| {}; external_providers.lint_mod = |_, _| {}; + local_providers.typeck_item_bodies = |_, _| {}; + // hack so that `used_trait_imports` won't try to call typeck_tables_of + local_providers.used_trait_imports = |_, _| &EMPTY_MAP; + // In case typeck does end up being called, don't ICE in case there were name resolution errors local_providers.typeck_tables_of = move |tcx, def_id| { // Closures' tables come from their outermost function, // as they are part of the same "inference environment". @@ -439,6 +447,13 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt let mut global_ctxt = abort_on_err(queries.global_ctxt(), sess).take(); global_ctxt.enter(|tcx| { + // Some queries require that they only run on valid types: + // https://github.com/rust-lang/rust/pull/73566#issuecomment-656954425 + // Therefore typecheck this crate before running lints. + // NOTE: this does not typeck item bodies or run the default rustc lints + // (see `override_queries` in the `config`) + let _ = rustc_typeck::check_crate(tcx); + tcx.sess.abort_if_errors(); sess.time("missing_docs", || { rustc_lint::check_crate(tcx, rustc_lint::builtin::MissingDoc::new); }); diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 4bd6b1260ccef..cbf53d52ef009 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -15,6 +15,8 @@ #![recursion_limit = "256"] extern crate env_logger; +#[macro_use] +extern crate lazy_static; extern crate rustc_ast; extern crate rustc_ast_pretty; extern crate rustc_attr; diff --git a/src/test/rustdoc-ui/infinite-recursive-type.rs b/src/test/rustdoc-ui/infinite-recursive-type.rs new file mode 100644 index 0000000000000..32793fc4f76c0 --- /dev/null +++ b/src/test/rustdoc-ui/infinite-recursive-type.rs @@ -0,0 +1,4 @@ +enum E { +//~^ ERROR recursive type `E` has infinite size + V(E), +} diff --git a/src/test/rustdoc-ui/infinite-recursive-type.stderr b/src/test/rustdoc-ui/infinite-recursive-type.stderr new file mode 100644 index 0000000000000..897445f200cb7 --- /dev/null +++ b/src/test/rustdoc-ui/infinite-recursive-type.stderr @@ -0,0 +1,17 @@ +error[E0072]: recursive type `E` has infinite size + --> $DIR/infinite-recursive-type.rs:1:1 + | +LL | enum E { + | ^^^^^^ recursive type has infinite size +LL | +LL | V(E), + | - recursive without indirection + | +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `E` representable + | +LL | V(Box), + | ^^^^ ^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0072`. From 4c88070c87b81c3cf6c8409a78c35ebdf67a67c3 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 11 Jul 2020 08:48:25 -0400 Subject: [PATCH 17/23] Use mem::replace instead of rewriting it --- src/librustc_resolve/late.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index 528444b0e9894..63b238faff615 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -504,10 +504,9 @@ impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> { visit::walk_fn_ret_ty(this, &declaration.output); - let previous_state = this.in_func_body; // Ignore errors in function bodies if this is rustdoc // Be sure not to set this until the function signature has been resolved. - this.in_func_body = true; + let previous_state = replace(&mut this.in_func_body, true); // Resolve the function body, potentially inside the body of an async closure match fn_kind { FnKind::Fn(.., body) => walk_list!(this, visit_block, body), @@ -1175,9 +1174,8 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { impl_items: &'ast [P], ) { debug!("resolve_implementation"); - let old_ignore = self.in_func_body; // Never ignore errors in trait implementations. - self.in_func_body = false; + let old_ignore = replace(&mut self.in_func_body, false); // 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. From b2ff0e703eef715737ebb2afab04ec3f73cbf4bf Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 11 Jul 2020 08:52:36 -0400 Subject: [PATCH 18/23] Fix comment --- src/librustc_resolve/late.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index 63b238faff615..2df68624a369b 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -1879,7 +1879,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { /// A wrapper around [`Resolver::report_error`]. /// - /// This doesn't emit errors for function bodies if this is r + /// This doesn't emit errors for function bodies if this is rustdoc. fn report_error(&self, span: Span, resolution_error: ResolutionError<'_>) { if self.should_report_errs() { self.r.report_error(span, resolution_error); From ac9157b482e916c09e2ec35bb7e514ae7b6b9c03 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 11 Jul 2020 20:54:47 -0400 Subject: [PATCH 19/23] EMPTY_MAP -> EMPTY_SET --- src/librustdoc/core.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index a77b177bd2864..f433c78890ffa 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -364,9 +364,6 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt ..Options::default() }; - lazy_static! { - static ref EMPTY_MAP: FxHashSet = FxHashSet::default(); - } let config = interface::Config { opts: sessopts, crate_cfg: interface::parse_cfgspecs(cfgs), @@ -381,12 +378,15 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt lint_caps, register_lints: None, override_queries: Some(|_sess, local_providers, external_providers| { + lazy_static! { + static ref EMPTY_SET: FxHashSet = FxHashSet::default(); + } // Most lints will require typechecking, so just don't run them. local_providers.lint_mod = |_, _| {}; external_providers.lint_mod = |_, _| {}; local_providers.typeck_item_bodies = |_, _| {}; // hack so that `used_trait_imports` won't try to call typeck_tables_of - local_providers.used_trait_imports = |_, _| &EMPTY_MAP; + local_providers.used_trait_imports = |_, _| &EMPTY_SET; // In case typeck does end up being called, don't ICE in case there were name resolution errors local_providers.typeck_tables_of = move |tcx, def_id| { // Closures' tables come from their outermost function, From 6eec9fb5d15d2bb2025398f5cae12aebe03d87e8 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 11 Jul 2020 21:50:25 -0400 Subject: [PATCH 20/23] Address review comments - Move static variables into the innermost scope in which they are used - Clean up comments - Remove external_providers; rename local_providers -> providers --- src/librustdoc/core.rs | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index f433c78890ffa..c2d0bd103eca9 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -377,18 +377,26 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt crate_name, lint_caps, register_lints: None, - override_queries: Some(|_sess, local_providers, external_providers| { - lazy_static! { - static ref EMPTY_SET: FxHashSet = FxHashSet::default(); - } + override_queries: Some(|_sess, providers, _external_providers| { // Most lints will require typechecking, so just don't run them. - local_providers.lint_mod = |_, _| {}; - external_providers.lint_mod = |_, _| {}; - local_providers.typeck_item_bodies = |_, _| {}; + providers.lint_mod = |_, _| {}; + // Prevent `rustc_typeck::check_crate` from calling `typeck_tables_of` on all bodies. + providers.typeck_item_bodies = |_, _| {}; // hack so that `used_trait_imports` won't try to call typeck_tables_of - local_providers.used_trait_imports = |_, _| &EMPTY_SET; + providers.used_trait_imports = |_, _| { + lazy_static! { + static ref EMPTY_SET: FxHashSet = FxHashSet::default(); + } + &EMPTY_SET + }; // In case typeck does end up being called, don't ICE in case there were name resolution errors - local_providers.typeck_tables_of = move |tcx, def_id| { + providers.typeck_tables_of = move |tcx, def_id| { + thread_local!(static DEFAULT_TYPECK: for<'tcx> fn(TyCtxt<'tcx>, LocalDefId) -> &'tcx ty::TypeckTables<'tcx> = { + let mut providers = ty::query::Providers::default(); + rustc_typeck::provide(&mut providers); + providers.typeck_tables_of + }); + // Closures' tables come from their outermost function, // as they are part of the same "inference environment". // This avoids emitting errors for the parent twice (see similar code in `typeck_tables_of_with_fallback`) @@ -447,10 +455,11 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt let mut global_ctxt = abort_on_err(queries.global_ctxt(), sess).take(); global_ctxt.enter(|tcx| { - // Some queries require that they only run on valid types: - // https://github.com/rust-lang/rust/pull/73566#issuecomment-656954425 - // Therefore typecheck this crate before running lints. - // NOTE: this does not typeck item bodies or run the default rustc lints + // Certain queries assume that some checks were run elsewhere + // (see https://github.com/rust-lang/rust/pull/73566#issuecomment-656954425), + // so type-check everything other than function bodies in this crate before running lints. + // NOTE: this does not call `tcx.analysis()` so that we won't + // typeck function bodies or run the default rustc lints. // (see `override_queries` in the `config`) let _ = rustc_typeck::check_crate(tcx); tcx.sess.abort_if_errors(); @@ -607,12 +616,6 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt }) } -thread_local!(static DEFAULT_TYPECK: for<'tcx> fn(TyCtxt<'tcx>, LocalDefId) -> &'tcx ty::TypeckTables<'tcx> = { - let mut providers = ty::query::Providers::default(); - rustc_typeck::provide(&mut providers); - providers.typeck_tables_of -}); - /// Due to https://github.com/rust-lang/rust/pull/73566, /// the name resolution pass may find errors that are never emitted. /// If typeck is called after this happens, then we'll get an ICE: From e117b47f759e93679192256043db67f8f8a68675 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 14 Jul 2020 21:14:09 -0400 Subject: [PATCH 21/23] Catch errors for any new item, not just trait implementations This matches the previous behavior of everybody_loops and is also more consistent than special-casing impls. --- src/librustc_resolve/late.rs | 6 +++--- src/test/rustdoc-ui/impl-fn-nesting.rs | 4 ++-- src/test/rustdoc-ui/impl-fn-nesting.stderr | 8 +++++++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index 2df68624a369b..274f93dd50b0d 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -407,7 +407,10 @@ struct LateResolutionVisitor<'a, 'b, 'ast> { impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> { fn visit_item(&mut self, item: &'ast Item) { let prev = replace(&mut self.diagnostic_metadata.current_item, Some(item)); + // Always report errors in items we just entered. + let old_ignore = replace(&mut self.in_func_body, false); self.resolve_item(item); + self.in_func_body = old_ignore; self.diagnostic_metadata.current_item = prev; } fn visit_arm(&mut self, arm: &'ast Arm) { @@ -1174,8 +1177,6 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { impl_items: &'ast [P], ) { debug!("resolve_implementation"); - // Never ignore errors in trait implementations. - let old_ignore = replace(&mut self.in_func_body, false); // 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. @@ -1271,7 +1272,6 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { }); }); }); - self.in_func_body = old_ignore; } fn check_trait_item(&mut self, ident: Ident, ns: Namespace, span: Span, err: F) diff --git a/src/test/rustdoc-ui/impl-fn-nesting.rs b/src/test/rustdoc-ui/impl-fn-nesting.rs index d2dd8d835fd79..a927f6bd79976 100644 --- a/src/test/rustdoc-ui/impl-fn-nesting.rs +++ b/src/test/rustdoc-ui/impl-fn-nesting.rs @@ -41,8 +41,8 @@ pub fn f(a: UnknownType, b: B) { can_define_macros_here_too!(); /// This also is documented. - pub fn doubly_nested(c: UnknownTypeShouldBeIgnored) { - + pub fn doubly_nested(c: UnknownType) { + //~^ ERROR cannot find type `UnknownType` in this scope } } } diff --git a/src/test/rustdoc-ui/impl-fn-nesting.stderr b/src/test/rustdoc-ui/impl-fn-nesting.stderr index f8629964c0701..608749af895ed 100644 --- a/src/test/rustdoc-ui/impl-fn-nesting.stderr +++ b/src/test/rustdoc-ui/impl-fn-nesting.stderr @@ -52,9 +52,15 @@ error[E0412]: cannot find type `UnknownType` in this scope LL | type Item = UnknownType; | ^^^^^^^^^^^ not found in this scope +error[E0412]: cannot find type `UnknownType` in this scope + --> $DIR/impl-fn-nesting.rs:44:37 + | +LL | pub fn doubly_nested(c: UnknownType) { + | ^^^^^^^^^^^ not found in this scope + error: Compilation failed, aborting rustdoc -error: aborting due to 10 previous errors +error: aborting due to 11 previous errors Some errors have detailed explanations: E0405, E0412. For more information about an error, try `rustc --explain E0405`. From 281ca139161fd6a208f2d531f683a706e8286826 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 15 Jul 2020 10:42:18 -0400 Subject: [PATCH 22/23] Use the default providers in rustc_interface instead of adding our own This avoids duplicating the same struct twice. --- src/librustdoc/core.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index c2d0bd103eca9..00315675fafe3 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -16,7 +16,7 @@ use rustc_interface::interface; use rustc_middle::hir::map::Map; use rustc_middle::middle::cstore::CrateStore; use rustc_middle::middle::privacy::AccessLevels; -use rustc_middle::ty::{self, Ty, TyCtxt}; +use rustc_middle::ty::{Ty, TyCtxt}; use rustc_resolve as resolve; use rustc_session::config::{self, CrateType, ErrorOutputType}; use rustc_session::lint; @@ -391,12 +391,6 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt }; // In case typeck does end up being called, don't ICE in case there were name resolution errors providers.typeck_tables_of = move |tcx, def_id| { - thread_local!(static DEFAULT_TYPECK: for<'tcx> fn(TyCtxt<'tcx>, LocalDefId) -> &'tcx ty::TypeckTables<'tcx> = { - let mut providers = ty::query::Providers::default(); - rustc_typeck::provide(&mut providers); - providers.typeck_tables_of - }); - // Closures' tables come from their outermost function, // as they are part of the same "inference environment". // This avoids emitting errors for the parent twice (see similar code in `typeck_tables_of_with_fallback`) @@ -409,7 +403,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt let body = hir.body(hir.body_owned_by(hir.as_local_hir_id(def_id))); debug!("visiting body for {:?}", def_id); EmitIgnoredResolutionErrors::new(tcx).visit_body(body); - DEFAULT_TYPECK.with(|typeck| typeck(tcx, def_id)) + (rustc_interface::DEFAULT_QUERY_PROVIDERS.typeck_tables_of)(tcx, def_id) }; }), registry: rustc_driver::diagnostics_registry(), From 631b2b9b722a3333aa5931fbbfa9df8846d48380 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 16 Jul 2020 09:03:46 -0400 Subject: [PATCH 23/23] Remove unused lazy_static --- Cargo.lock | 1 - src/librustdoc/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 992421dcd7abb..5309c03ee23ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4027,7 +4027,6 @@ name = "rustdoc" version = "0.0.0" dependencies = [ "itertools 0.8.0", - "lazy_static", "minifier", "pulldown-cmark", "rustc-rayon", diff --git a/src/librustdoc/Cargo.toml b/src/librustdoc/Cargo.toml index baceb13cc6141..4af13e4cd587a 100644 --- a/src/librustdoc/Cargo.toml +++ b/src/librustdoc/Cargo.toml @@ -16,4 +16,3 @@ serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" tempfile = "3" itertools = "0.8" -lazy_static = "1"