From 1879d17f082e58500c84529815a2a8ba955af999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Thu, 31 Jan 2019 23:11:29 +0100 Subject: [PATCH 1/2] Only insert nodes which changes lint levels in the LintLevelMap --- src/librustc/hir/map/mod.rs | 1 + src/librustc/lint/levels.rs | 7 ++-- src/librustc/lint/mod.rs | 21 ++++++++--- src/librustc/ty/context.rs | 60 +++++++++++++++++++------------ src/librustc_mir/build/scope.rs | 30 ++++++++++------ src/librustc_mir/hair/cx/block.rs | 2 +- src/librustc_mir/hair/cx/expr.rs | 2 +- src/librustc_mir/hair/cx/mod.rs | 41 ++------------------- 8 files changed, 80 insertions(+), 84 deletions(-) diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 21a9ed5ebe03a..da1c541019c5d 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -1033,6 +1033,7 @@ impl<'hir> Map<'hir> { pub fn attrs(&self, id: NodeId) -> &'hir [ast::Attribute] { self.read(id); // reveals attributes on the node let attrs = match self.find(id) { + Some(Node::Local(l)) => Some(&l.attrs[..]), Some(Node::Item(i)) => Some(&i.attrs[..]), Some(Node::ForeignItem(fi)) => Some(&fi.attrs[..]), Some(Node::TraitItem(ref ti)) => Some(&ti.attrs[..]), diff --git a/src/librustc/lint/levels.rs b/src/librustc/lint/levels.rs index 924aa3fde0a08..3c6635c034131 100644 --- a/src/librustc/lint/levels.rs +++ b/src/librustc/lint/levels.rs @@ -157,6 +157,7 @@ pub struct LintLevelsBuilder<'a> { pub struct BuilderPush { prev: u32, + pub(super) changed: bool, } impl<'a> LintLevelsBuilder<'a> { @@ -454,6 +455,7 @@ impl<'a> LintLevelsBuilder<'a> { BuilderPush { prev: prev, + changed: prev != self.cur, } } @@ -512,11 +514,6 @@ impl LintLevelMap { self.sets.get_lint_level(lint, *idx, None, session) }) } - - /// Returns if this `id` has lint level information. - pub fn lint_level_set(&self, id: HirId) -> Option { - self.id_to_set.get(&id).cloned() - } } impl<'a> HashStable> for LintLevelMap { diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 496ff568b31b4..b3fc5612ba547 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -721,6 +721,16 @@ pub fn struct_lint_level<'a>(sess: &'a Session, return err } +pub fn maybe_lint_level_root(tcx: TyCtxt<'_, '_, '_>, id: hir::HirId) -> bool { + let attrs = tcx.hir().attrs_by_hir_id(id); + for attr in attrs { + if Level::from_str(&attr.name().as_str()).is_some() { + true; + } + } + false +} + fn lint_levels<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, cnum: CrateNum) -> Lrc { @@ -731,9 +741,10 @@ fn lint_levels<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, cnum: CrateNum) }; let krate = tcx.hir().krate(); - builder.with_lint_attrs(hir::CRATE_HIR_ID, &krate.attrs, |builder| { - intravisit::walk_crate(builder, krate); - }); + let push = builder.levels.push(&krate.attrs); + builder.levels.register_id(hir::CRATE_HIR_ID); + intravisit::walk_crate(&mut builder, krate); + builder.levels.pop(push); Lrc::new(builder.levels.build_map()) } @@ -751,7 +762,9 @@ impl<'a, 'tcx> LintLevelMapBuilder<'a, 'tcx> { where F: FnOnce(&mut Self) { let push = self.levels.push(attrs); - self.levels.register_id(id); + if push.changed { + self.levels.register_id(id); + } f(self); self.levels.pop(push); } diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 3c63dcb9ef307..3f7018ed919cd 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -2856,30 +2856,44 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { err.emit() } - pub fn lint_level_at_node(self, lint: &'static Lint, mut id: hir::HirId) - -> (lint::Level, lint::LintSource) - { - // Right now we insert a `with_ignore` node in the dep graph here to - // ignore the fact that `lint_levels` below depends on the entire crate. - // For now this'll prevent false positives of recompiling too much when - // anything changes. - // - // Once red/green incremental compilation lands we should be able to - // remove this because while the crate changes often the lint level map - // will change rarely. - self.dep_graph.with_ignore(|| { - let sets = self.lint_levels(LOCAL_CRATE); - loop { - if let Some(pair) = sets.level_and_source(lint, id, self.sess) { - return pair - } - let next = self.hir().get_parent_node_by_hir_id(id); - if next == id { - bug!("lint traversal reached the root of the crate"); - } - id = next; + /// Walks upwards from `id` to find a node which might change lint levels with attributes. + /// It stops at `bound` and just returns it if reached. + pub fn maybe_lint_level_root_bounded( + self, + mut id: hir::HirId, + bound: hir::HirId, + ) -> hir::HirId { + loop { + if id == bound { + return bound; } - }) + if lint::maybe_lint_level_root(self, id) { + return id; + } + let next = self.hir().get_parent_node_by_hir_id(id); + if next == id { + bug!("lint traversal reached the root of the crate"); + } + id = next; + } + } + + pub fn lint_level_at_node( + self, + lint: &'static Lint, + mut id: hir::HirId + ) -> (lint::Level, lint::LintSource) { + let sets = self.lint_levels(LOCAL_CRATE); + loop { + if let Some(pair) = sets.level_and_source(lint, id, self.sess) { + return pair + } + let next = self.hir().get_parent_node_by_hir_id(id); + if next == id { + bug!("lint traversal reached the root of the crate"); + } + id = next; + } } pub fn struct_span_lint_hir>(self, diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 4189e3e7ddbb6..4aa463b37ab77 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -82,7 +82,6 @@ use crate::hair::LintLevel; use rustc::middle::region; use rustc::ty::Ty; use rustc::hir; -use rustc::hir::def_id::LOCAL_CRATE; use rustc::mir::*; use syntax_pos::{Span}; use rustc_data_structures::fx::FxHashMap; @@ -309,16 +308,25 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let source_scope = self.source_scope; let tcx = self.hir.tcx(); if let LintLevel::Explicit(current_hir_id) = lint_level { - let same_lint_scopes = tcx.dep_graph.with_ignore(|| { - let sets = tcx.lint_levels(LOCAL_CRATE); - let parent_hir_id = self.source_scope_local_data[source_scope].lint_root; - sets.lint_level_set(parent_hir_id) == sets.lint_level_set(current_hir_id) - }); - - if !same_lint_scopes { - self.source_scope = - self.new_source_scope(region_scope.1.span, lint_level, - None); + // Use `maybe_lint_level_root_bounded` with `root_lint_level` as a bound + // to avoid adding Hir dependences on our parents. + // We estimate the true lint roots here to avoid creating a lot of source scopes. + + let parent_root = tcx.maybe_lint_level_root_bounded( + self.source_scope_local_data[source_scope].lint_root, + self.hir.root_lint_level, + ); + let current_root = tcx.maybe_lint_level_root_bounded( + current_hir_id, + self.hir.root_lint_level + ); + + if parent_root != current_root { + self.source_scope = self.new_source_scope( + region_scope.1.span, + LintLevel::Explicit(current_root), + None + ); } } self.push_scope(region_scope); diff --git a/src/librustc_mir/hair/cx/block.rs b/src/librustc_mir/hair/cx/block.rs index f58e61915e8c9..17fab6c5ddcff 100644 --- a/src/librustc_mir/hair/cx/block.rs +++ b/src/librustc_mir/hair/cx/block.rs @@ -103,7 +103,7 @@ fn mirror_stmts<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, }, pattern, initializer: local.init.to_ref(), - lint_level: cx.lint_level_of(local.hir_id), + lint_level: LintLevel::Explicit(local.hir_id), }, opt_destruction_scope: opt_dxn_ext, span: stmt_span, diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs index 65cd9f7103d68..1591fa318cae8 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/src/librustc_mir/hair/cx/expr.rs @@ -45,7 +45,7 @@ impl<'tcx> Mirror<'tcx> for &'tcx hir::Expr { kind: ExprKind::Scope { region_scope: expr_scope, value: expr.to_ref(), - lint_level: cx.lint_level_of(self.hir_id), + lint_level: LintLevel::Explicit(self.hir_id), }, }; diff --git a/src/librustc_mir/hair/cx/mod.rs b/src/librustc_mir/hair/cx/mod.rs index 8b16eeeea23c0..c0f3989b4ba97 100644 --- a/src/librustc_mir/hair/cx/mod.rs +++ b/src/librustc_mir/hair/cx/mod.rs @@ -6,7 +6,7 @@ use crate::hair::*; use crate::hair::util::UserAnnotatedTyHelpers; use rustc_data_structures::indexed_vec::Idx; -use rustc::hir::def_id::{DefId, LOCAL_CRATE}; +use rustc::hir::def_id::DefId; use rustc::hir::Node; use rustc::middle::region; use rustc::infer::InferCtxt; @@ -76,11 +76,10 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> { // Constants always need overflow checks. check_overflow |= constness == hir::Constness::Const; - let lint_level = lint_level_for_hir_id(tcx, src_id); Cx { tcx, infcx, - root_lint_level: lint_level, + root_lint_level: src_id, param_env: tcx.param_env(src_def_id), identity_substs: InternalSubsts::identity_for_item(tcx.global_tcx(), src_def_id), region_scope_tree: tcx.region_scope_tree(src_def_id), @@ -197,18 +196,6 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> { ty.needs_drop(self.tcx.global_tcx(), param_env) } - fn lint_level_of(&self, hir_id: hir::HirId) -> LintLevel { - let has_lint_level = self.tcx.dep_graph.with_ignore(|| { - self.tcx.lint_levels(LOCAL_CRATE).lint_level_set(hir_id).is_some() - }); - - if has_lint_level { - LintLevel::Explicit(hir_id) - } else { - LintLevel::Inherited - } - } - pub fn tcx(&self) -> TyCtxt<'a, 'gcx, 'tcx> { self.tcx } @@ -236,30 +223,6 @@ impl UserAnnotatedTyHelpers<'gcx, 'tcx> for Cx<'_, 'gcx, 'tcx> { } } -fn lint_level_for_hir_id(tcx: TyCtxt<'_, '_, '_>, mut id: hir::HirId) -> hir::HirId { - // Right now we insert a `with_ignore` node in the dep graph here to - // ignore the fact that `lint_levels` below depends on the entire crate. - // For now this'll prevent false positives of recompiling too much when - // anything changes. - // - // Once red/green incremental compilation lands we should be able to - // remove this because while the crate changes often the lint level map - // will change rarely. - tcx.dep_graph.with_ignore(|| { - let sets = tcx.lint_levels(LOCAL_CRATE); - loop { - if sets.lint_level_set(id).is_some() { - return id - } - let next = tcx.hir().get_parent_node_by_hir_id(id); - if next == id { - bug!("lint traversal reached the root of the crate"); - } - id = next; - } - }) -} - mod block; mod expr; mod to_ref; From f07ce55d0f75d7a76c20e58e3639cdffbe770aeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Tue, 5 Mar 2019 19:39:34 +0100 Subject: [PATCH 2/2] Add `return` --- src/librustc/lint/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index b3fc5612ba547..c01b0ae2ccc1d 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -725,7 +725,7 @@ pub fn maybe_lint_level_root(tcx: TyCtxt<'_, '_, '_>, id: hir::HirId) -> bool { let attrs = tcx.hir().attrs_by_hir_id(id); for attr in attrs { if Level::from_str(&attr.name().as_str()).is_some() { - true; + return true; } } false