From c3ded2204a722dd15ed433ca09341f4de5eb5edc 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] Only insert nodes which changes lint levels in the LintLevelMap --- src/librustc/dep_graph/dep_node.rs | 1 + src/librustc/lint/levels.rs | 5 +++ src/librustc/lint/mod.rs | 39 +++++++++++++++++---- src/librustc/ty/context.rs | 54 +++++++++++++++++------------- src/librustc/ty/query/mod.rs | 3 +- src/librustc/ty/query/plumbing.rs | 1 + src/librustc_mir/build/scope.rs | 23 +++++-------- src/librustc_mir/hair/cx/block.rs | 2 +- src/librustc_mir/hair/cx/expr.rs | 2 +- src/librustc_mir/hair/cx/mod.rs | 43 ++---------------------- 10 files changed, 85 insertions(+), 88 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index cda469657ed87..87bd822c518b1 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -574,6 +574,7 @@ define_dep_nodes!( <'tcx> [] HasPanicHandler(CrateNum), [input] ExternCrate(DefId), [eval_always] LintLevels, + [eval_always] LintLevelRootForDefId(DefId), [] Specializes { impl1: DefId, impl2: DefId }, [input] InScopeTraits(DefIndex), [input] ModuleExports(DefId), diff --git a/src/librustc/lint/levels.rs b/src/librustc/lint/levels.rs index 616915769435d..ff3afbd7a0e23 100644 --- a/src/librustc/lint/levels.rs +++ b/src/librustc/lint/levels.rs @@ -16,11 +16,13 @@ use syntax::source_map::MultiSpan; use syntax::symbol::Symbol; use util::nodemap::FxHashMap; +#[derive(Debug)] pub struct LintLevelSets { list: Vec, lint_cap: Level, } +#[derive(Debug)] enum LintSet { CommandLine { // -A,-W,-D flags, a `Symbol` for the flag itself and `Level` for which @@ -157,6 +159,7 @@ pub struct LintLevelsBuilder<'a> { pub struct BuilderPush { prev: u32, + pub(super) changed: bool, } impl<'a> LintLevelsBuilder<'a> { @@ -454,6 +457,7 @@ impl<'a> LintLevelsBuilder<'a> { BuilderPush { prev: prev, + changed: prev != self.cur, } } @@ -492,6 +496,7 @@ impl<'a> LintLevelsBuilder<'a> { } } +#[derive(Debug)] pub struct LintLevelMap { sets: LintLevelSets, id_to_set: FxHashMap, diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index a95fa350bf1c3..a12695334f893 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -24,9 +24,10 @@ pub use self::LintSource::*; use rustc_data_structures::sync::{self, Lrc}; use errors::{DiagnosticBuilder, DiagnosticId}; -use hir::def_id::{CrateNum, LOCAL_CRATE}; +use hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; use hir::intravisit; use hir; +use hir::CRATE_HIR_ID; use lint::builtin::BuiltinLintDiagnostics; use lint::builtin::parser::{QUESTION_MARK_MACRO_SEP, ILL_FORMED_ATTRIBUTE_INPUT}; use session::{Session, DiagnosticMessageId}; @@ -540,7 +541,7 @@ impl Level { } /// How a lint level was set. -#[derive(Clone, Copy, PartialEq, Eq)] +#[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum LintSource { /// Lint is at the default level as declared /// in rustc or a plugin. @@ -722,11 +723,32 @@ fn lint_levels<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, cnum: CrateNum) }; let krate = tcx.hir().krate(); - builder.with_lint_attrs(ast::CRATE_NODE_ID, &krate.attrs, |builder| { - intravisit::walk_crate(builder, krate); - }); + let push = builder.levels.push(&krate.attrs); + builder.levels.register_id(CRATE_HIR_ID); + intravisit::walk_crate(&mut builder, krate); + builder.levels.pop(push); - Lrc::new(builder.levels.build_map()) + let r = Lrc::new(builder.levels.build_map()); + if tcx.sess.verbose() { + eprintln!("lint level map: {:#?}", r); + } + r +} + +fn lint_level_root_for_def_id(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> hir::HirId { + let mut id = tcx.hir().as_local_hir_id(def_id).unwrap(); + let sets = tcx.lint_levels(LOCAL_CRATE); + loop { + if sets.lint_level_set(id).is_some() { + return id + } + let node_id = tcx.hir().hir_to_node_id(id); + let next = tcx.hir().node_to_hir_id(tcx.hir().get_parent_node(node_id)); + if next == id { + bug!("lint traversal reached the root of the crate"); + } + id = next; + } } struct LintLevelMapBuilder<'a, 'tcx: 'a> { @@ -742,7 +764,9 @@ impl<'a, 'tcx> LintLevelMapBuilder<'a, 'tcx> { where F: FnOnce(&mut Self) { let push = self.levels.push(attrs); - self.levels.register_id(self.tcx.hir().definitions().node_to_hir_id(id)); + if push.changed { + self.levels.register_id(self.tcx.hir().definitions().node_to_hir_id(id)); + } f(self); self.levels.pop(push); } @@ -807,6 +831,7 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for LintLevelMapBuilder<'a, 'tcx> { pub fn provide(providers: &mut Providers<'_>) { providers.lint_levels = lint_levels; + providers.lint_level_root_for_def_id = lint_level_root_for_def_id; } /// Returns whether `span` originates in a foreign crate's external macro. diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index b379b5ba02494..1ac842f1f062a 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -2857,31 +2857,39 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { err.emit() } - pub fn lint_level_at_node(self, lint: &'static Lint, mut id: NodeId) + pub fn lint_level_root(self, mut id: ast::NodeId) -> ast::NodeId { + // If the `id` has a DefId, just use the query to avoid + // adding dependencies to its parents + if let Some(def_id) = self.hir().opt_local_def_id(id) { + return self.hir().hir_to_node_id(self.lint_level_root_for_def_id(def_id)); + } + + let sets = self.lint_levels(LOCAL_CRATE); + loop { + let hir_id = self.hir().definitions().node_to_hir_id(id); + if sets.lint_level_set(hir_id).is_some() { + return id + } + let next = self.hir().get_parent_node(id); + if next == id { + bug!("lint traversal reached the root of the crate"); + } + // If we find a node with a DefId, stop adding dependencies + // to the parents of `id` and just use the query + if let Some(def_id) = self.hir().opt_local_def_id(next) { + return self.hir().hir_to_node_id(self.lint_level_root_for_def_id(def_id)); + } + id = next; + } + } + + pub fn lint_level_at_node(self, lint: &'static Lint, id: NodeId) -> (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 { - let hir_id = self.hir().definitions().node_to_hir_id(id); - if let Some(pair) = sets.level_and_source(lint, hir_id, self.sess) { - return pair - } - let next = self.hir().get_parent_node(id); - if next == id { - bug!("lint traversal reached the root of the crate"); - } - id = next; - } - }) + let sets = self.lint_levels(LOCAL_CRATE); + let lint_root = self.lint_level_root(id); + let hir_id = self.hir().definitions().node_to_hir_id(lint_root); + sets.level_and_source(lint, hir_id, self.sess).unwrap() } pub fn struct_span_lint_hir>(self, diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index d4884e712b860..89a7a21241a71 100644 --- a/src/librustc/ty/query/mod.rs +++ b/src/librustc/ty/query/mod.rs @@ -2,7 +2,7 @@ use dep_graph::{DepConstructor, DepNode}; use errors::DiagnosticBuilder; use hir::def_id::{CrateNum, DefId, DefIndex}; use hir::def::{Def, Export}; -use hir::{self, TraitCandidate, ItemLocalId, CodegenFnAttrs}; +use hir::{self, HirId, TraitCandidate, ItemLocalId, CodegenFnAttrs}; use rustc_data_structures::svh::Svh; use infer::canonical::{self, Canonical}; use lint; @@ -443,6 +443,7 @@ define_queries! { <'tcx> Other { [] fn module_exports: ModuleExports(DefId) -> Option>>, [] fn lint_levels: lint_levels_node(CrateNum) -> Lrc, + [] fn lint_level_root_for_def_id: LintLevelRootForDefId(DefId) -> HirId, }, TypeChecking { diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 69bff8d25b024..d3837495175d1 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -1330,6 +1330,7 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>, DepKind::HasPanicHandler => { force!(has_panic_handler, krate!()); } DepKind::ExternCrate => { force!(extern_crate, def_id!()); } DepKind::LintLevels => { force!(lint_levels, LOCAL_CRATE); } + DepKind::LintLevelRootForDefId => { force!(lint_level_root_for_def_id, def_id!()); } DepKind::InScopeTraits => { force!(in_scope_traits_map, def_id!().index); } DepKind::ModuleExports => { force!(module_exports, def_id!()); } DepKind::IsSanitizerRuntime => { force!(is_sanitizer_runtime, krate!()); } diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 78abba5f885b2..7d4e695e13e73 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -82,7 +82,6 @@ use 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,19 +308,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let source_scope = self.source_scope; let tcx = self.hir.tcx(); if let LintLevel::Explicit(node_id) = lint_level { - let same_lint_scopes = tcx.dep_graph.with_ignore(|| { - let sets = tcx.lint_levels(LOCAL_CRATE); - let parent_hir_id = - tcx.hir().definitions().node_to_hir_id( - self.source_scope_local_data[source_scope].lint_root - ); - let current_hir_id = - tcx.hir().definitions().node_to_hir_id(node_id); - sets.lint_level_set(parent_hir_id) == - sets.lint_level_set(current_hir_id) - }); - - if !same_lint_scopes { + let parent_root = tcx.lint_level_root( + self.source_scope_local_data[source_scope].lint_root + ); + let current_root = tcx.lint_level_root(node_id); + + if parent_root != current_root { + // FIXME: The new source scope should use `current_root` + // but that causes bindings to be created in the wrong + // source scope which might affect debug info self.source_scope = self.new_source_scope(region_scope.1.span, lint_level, None); diff --git a/src/librustc_mir/hair/cx/block.rs b/src/librustc_mir/hair/cx/block.rs index 518ae978ae17a..d4253ed302bcc 100644 --- a/src/librustc_mir/hair/cx/block.rs +++ b/src/librustc_mir/hair/cx/block.rs @@ -101,7 +101,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.id), + lint_level: LintLevel::Explicit(local.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 8d64c9e9ada89..7082ce068dd14 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/src/librustc_mir/hair/cx/expr.rs @@ -44,7 +44,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.id), + lint_level: LintLevel::Explicit(self.id), }, }; diff --git a/src/librustc_mir/hair/cx/mod.rs b/src/librustc_mir/hair/cx/mod.rs index f514cac6326be..6781914e9dc2d 100644 --- a/src/librustc_mir/hair/cx/mod.rs +++ b/src/librustc_mir/hair/cx/mod.rs @@ -8,7 +8,7 @@ use hair::*; use 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; @@ -78,11 +78,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: tcx.lint_level_root(src_id), param_env: tcx.param_env(src_def_id), identity_substs: Substs::identity_for_item(tcx.global_tcx(), src_def_id), region_scope_tree: tcx.region_scope_tree(src_def_id), @@ -199,19 +198,6 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> { ty.needs_drop(self.tcx.global_tcx(), param_env) } - fn lint_level_of(&self, node_id: ast::NodeId) -> LintLevel { - let hir_id = self.tcx.hir().definitions().node_to_hir_id(node_id); - 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(node_id) - } else { - LintLevel::Inherited - } - } - pub fn tcx(&self) -> TyCtxt<'a, 'gcx, 'tcx> { self.tcx } @@ -239,31 +225,6 @@ impl UserAnnotatedTyHelpers<'gcx, 'tcx> for Cx<'_, 'gcx, 'tcx> { } } -fn lint_level_for_hir_id(tcx: TyCtxt, mut id: ast::NodeId) -> ast::NodeId { - // 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 { - let hir_id = tcx.hir().definitions().node_to_hir_id(id); - if sets.lint_level_set(hir_id).is_some() { - return id - } - let next = tcx.hir().get_parent_node(id); - if next == id { - bug!("lint traversal reached the root of the crate"); - } - id = next; - } - }) -} - mod block; mod expr; mod to_ref;