From 969a2cc8c1e83ce93a13ab126c4519a72405bb69 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 15 Jun 2022 17:44:53 +1000 Subject: [PATCH 1/3] Fix quadratic behaviour in the `MissingDoc` lint. The `MissingDoc` lint has quadratic behaviour when processing doc comments. This is a problem for large doc comments (e.g. 1000+ lines) when `deny(missing_code)` is enabled. A 1000-line doc comment using `//!` comments is represented as 1000 attributes on an item. The lint machinery iterates over each attribute with `visit_attribute`. `MissingDoc`'s impl of that function calls `with_lint_attrs`, which calls `enter_attrs`, which iterates over all 1000 attributes looking for a `doc(hidden)` attribute. I.e. for every attribute we iterate over all the other attributes. The fix is simple: don't call `with_lint_attrs` on attributes. This makes sense: `with_lint_attrs` is intended to iterate over the attributes on a language fragment like a statement or expression, but it doesn't need to be called on attributes themselves. --- compiler/rustc_lint/src/late.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_lint/src/late.rs b/compiler/rustc_lint/src/late.rs index 772ab7fe22608..9ca80334e0531 100644 --- a/compiler/rustc_lint/src/late.rs +++ b/compiler/rustc_lint/src/late.rs @@ -337,10 +337,8 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas hir_visit::walk_path(self, p); } - fn visit_attribute(&mut self, hir_id: hir::HirId, attr: &'tcx ast::Attribute) { - self.with_lint_attrs(hir_id, |cx| { - lint_callback!(cx, check_attribute, attr); - }) + fn visit_attribute(&mut self, _hir_id: hir::HirId, attr: &'tcx ast::Attribute) { + lint_callback!(self, check_attribute, attr); } } From c9e97251adf538be8f7f34bfd3e02a4bed164cbd Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 16 Jun 2022 07:54:03 +1000 Subject: [PATCH 2/3] Remove unused `hir_id` arg from `visit_attribute`. --- compiler/rustc_hir/src/intravisit.rs | 2 +- compiler/rustc_incremental/src/persist/dirty_clean.rs | 3 +-- compiler/rustc_lint/src/late.rs | 4 ++-- compiler/rustc_middle/src/hir/map/mod.rs | 7 +++---- compiler/rustc_passes/src/hir_stats.rs | 2 +- compiler/rustc_passes/src/lib_features.rs | 2 +- 6 files changed, 9 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index d1da2519badce..cce5aa9f7324d 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -466,7 +466,7 @@ pub trait Visitor<'v>: Sized { fn visit_assoc_type_binding(&mut self, type_binding: &'v TypeBinding<'v>) { walk_assoc_type_binding(self, type_binding) } - fn visit_attribute(&mut self, _id: HirId, _attr: &'v Attribute) {} + fn visit_attribute(&mut self, _attr: &'v Attribute) {} fn visit_associated_item_kind(&mut self, kind: &'v AssocItemKind) { walk_associated_item_kind(self, kind); } diff --git a/compiler/rustc_incremental/src/persist/dirty_clean.rs b/compiler/rustc_incremental/src/persist/dirty_clean.rs index 424164d8760c4..94097357f8c10 100644 --- a/compiler/rustc_incremental/src/persist/dirty_clean.rs +++ b/compiler/rustc_incremental/src/persist/dirty_clean.rs @@ -21,7 +21,6 @@ use rustc_ast::{self as ast, Attribute, NestedMetaItem}; use rustc_data_structures::fx::FxHashSet; -use rustc_hir as hir; use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit; use rustc_hir::Node as HirNode; @@ -473,7 +472,7 @@ impl<'tcx> intravisit::Visitor<'tcx> for FindAllAttrs<'tcx> { self.tcx.hir() } - fn visit_attribute(&mut self, _: hir::HirId, attr: &'tcx Attribute) { + fn visit_attribute(&mut self, attr: &'tcx Attribute) { if self.is_active_attr(attr) { self.found_attrs.push(attr); } diff --git a/compiler/rustc_lint/src/late.rs b/compiler/rustc_lint/src/late.rs index 9ca80334e0531..e9b14b2693ca7 100644 --- a/compiler/rustc_lint/src/late.rs +++ b/compiler/rustc_lint/src/late.rs @@ -337,7 +337,7 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas hir_visit::walk_path(self, p); } - fn visit_attribute(&mut self, _hir_id: hir::HirId, attr: &'tcx ast::Attribute) { + fn visit_attribute(&mut self, attr: &'tcx ast::Attribute) { lint_callback!(self, check_attribute, attr); } } @@ -400,7 +400,7 @@ fn late_lint_mod_pass<'tcx, T: LateLintPass<'tcx>>( // Visit the crate attributes if hir_id == hir::CRATE_HIR_ID { for attr in tcx.hir().attrs(hir::CRATE_HIR_ID).iter() { - cx.visit_attribute(hir_id, attr) + cx.visit_attribute(attr) } } } diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index 0826cd793755f..4c853ba0fa906 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -577,12 +577,11 @@ impl<'hir> Map<'hir> { /// Walks the attributes in a crate. pub fn walk_attributes(self, visitor: &mut impl Visitor<'hir>) { let krate = self.krate(); - for (owner, info) in krate.owners.iter_enumerated() { + for info in krate.owners.iter() { if let MaybeOwner::Owner(info) = info { - for (local_id, attrs) in info.attrs.map.iter() { - let id = HirId { owner, local_id: *local_id }; + for attrs in info.attrs.map.values() { for a in *attrs { - visitor.visit_attribute(id, a) + visitor.visit_attribute(a) } } } diff --git a/compiler/rustc_passes/src/hir_stats.rs b/compiler/rustc_passes/src/hir_stats.rs index 6a234294ed161..a3be827a7ccec 100644 --- a/compiler/rustc_passes/src/hir_stats.rs +++ b/compiler/rustc_passes/src/hir_stats.rs @@ -238,7 +238,7 @@ impl<'v> hir_visit::Visitor<'v> for StatCollector<'v> { hir_visit::walk_assoc_type_binding(self, type_binding) } - fn visit_attribute(&mut self, _: hir::HirId, attr: &'v ast::Attribute) { + fn visit_attribute(&mut self, attr: &'v ast::Attribute) { self.record("Attribute", Id::Attr(attr.id), attr); } } diff --git a/compiler/rustc_passes/src/lib_features.rs b/compiler/rustc_passes/src/lib_features.rs index c414d7c031c04..26bfa4737a752 100644 --- a/compiler/rustc_passes/src/lib_features.rs +++ b/compiler/rustc_passes/src/lib_features.rs @@ -120,7 +120,7 @@ impl<'tcx> Visitor<'tcx> for LibFeatureCollector<'tcx> { self.tcx.hir() } - fn visit_attribute(&mut self, _: rustc_hir::HirId, attr: &'tcx Attribute) { + fn visit_attribute(&mut self, attr: &'tcx Attribute) { if let Some((feature, stable, span)) = self.extract(attr) { self.collect_feature(feature, stable, span); } From be45f10a9c3b429eeef95dc24ebdd19cbccce0be Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 16 Jun 2022 09:47:07 +1000 Subject: [PATCH 3/3] Inline and remove `{enter,exit}_attrs` functions. They each have a single call site. --- compiler/rustc_lint/src/early.rs | 12 ++---------- compiler/rustc_lint/src/late.rs | 16 ++++------------ 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_lint/src/early.rs b/compiler/rustc_lint/src/early.rs index 2ab948958d047..5de35dc085601 100644 --- a/compiler/rustc_lint/src/early.rs +++ b/compiler/rustc_lint/src/early.rs @@ -63,20 +63,12 @@ impl<'a, T: EarlyLintPass> EarlyContextAndPass<'a, T> { let push = self.context.builder.push(attrs, is_crate_node, None); self.check_id(id); - self.enter_attrs(attrs); - f(self); - self.exit_attrs(attrs); - self.context.builder.pop(push); - } - - fn enter_attrs(&mut self, attrs: &'a [ast::Attribute]) { debug!("early context: enter_attrs({:?})", attrs); run_early_pass!(self, enter_lint_attrs, attrs); - } - - fn exit_attrs(&mut self, attrs: &'a [ast::Attribute]) { + f(self); debug!("early context: exit_attrs({:?})", attrs); run_early_pass!(self, exit_lint_attrs, attrs); + self.context.builder.pop(push); } } diff --git a/compiler/rustc_lint/src/late.rs b/compiler/rustc_lint/src/late.rs index e9b14b2693ca7..c1d8d76c975d8 100644 --- a/compiler/rustc_lint/src/late.rs +++ b/compiler/rustc_lint/src/late.rs @@ -59,9 +59,11 @@ impl<'tcx, T: LateLintPass<'tcx>> LateContextAndPass<'tcx, T> { let attrs = self.context.tcx.hir().attrs(id); let prev = self.context.last_node_with_lint_attrs; self.context.last_node_with_lint_attrs = id; - self.enter_attrs(attrs); + debug!("late context: enter_attrs({:?})", attrs); + lint_callback!(self, enter_lint_attrs, attrs); f(self); - self.exit_attrs(attrs); + debug!("late context: exit_attrs({:?})", attrs); + lint_callback!(self, exit_lint_attrs, attrs); self.context.last_node_with_lint_attrs = prev; } @@ -81,16 +83,6 @@ impl<'tcx, T: LateLintPass<'tcx>> LateContextAndPass<'tcx, T> { hir_visit::walk_mod(self, m, n); lint_callback!(self, check_mod_post, m, s, n); } - - fn enter_attrs(&mut self, attrs: &'tcx [ast::Attribute]) { - debug!("late context: enter_attrs({:?})", attrs); - lint_callback!(self, enter_lint_attrs, attrs); - } - - fn exit_attrs(&mut self, attrs: &'tcx [ast::Attribute]) { - debug!("late context: exit_attrs({:?})", attrs); - lint_callback!(self, exit_lint_attrs, attrs); - } } impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPass<'tcx, T> {