From a5275ff41521bb8e5a70f49f8ed420eaac7ed7de Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 20 Jun 2020 15:57:23 -0400 Subject: [PATCH] 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 241ba869d3e..1862b47b9ad 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 c165a601408..ddce82494e1 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 @@ fn visit_generic_arg(&mut self, arg: &'ast GenericArg) { } 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 @@ fn new(resolver: &'b mut Resolver<'a>) -> LateResolutionVisitor<'a, 'b, 'ast> { label_ribs: Vec::new(), current_trait_ref: None, diagnostic_metadata: DiagnosticMetadata::default(), + ignore_bodies, } } @@ -757,7 +766,7 @@ fn resolve_label(&self, mut label: Ident) -> Option { 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 @@ fn resolve_label(&self, mut label: Ident) -> Option { 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 @@ fn with_generic_param_rib<'c, F>(&'c mut self, generics: &'c Generics, kind: Rib 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 @@ fn check_trait_item(&mut self, ident: Ident, ns: Namespace, span: Span, err: .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 @@ fn check_consistent_bindings(&mut self, pats: &[P]) -> Vec { 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 @@ fn check_consistent_bindings(&mut self, pats: &[P]) -> Vec { 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 @@ fn fresh_binding( // `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 @@ fn try_resolve_as_non_binding( // 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 @@ fn smart_resolve_path_fragment( 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 @@ fn self_value_is_available(&mut self, self_span: Span, path_span: Span) -> bool 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 @@ fn find_transitive_imports( } 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 a265c15c18b..786dc28ba0e 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1441,13 +1441,13 @@ fn macro_def(&self, mut ctxt: SyntaxContext) -> DefId { } /// 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 aa407b7e926..8664930bc94 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(); + } }