Auto merge of #58176 - Zoxc:lint-levels, r=<try>

bors · bors · commit e04391707576 · 2019-02-06T02:07:27.000Z
[WIP] Only insert nodes which changes lint levels in the LintLevelMap r? @eddyb
diff --git 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
@@ -16,11 +16,13 @@ use syntax::source_map::MultiSpan;
 use syntax::symbol::Symbol;
 use util::nodemap::FxHashMap;
 
+#[derive(Debug)]
 pub struct LintLevelSets {
     list: Vec<LintSet>,
     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<HirId, u32>,
diff --git 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
@@ -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<S: Into<MultiSpan>>(self,
diff --git 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<Lrc<Vec<Export>>>,
         [] fn lint_levels: lint_levels_node(CrateNum) -> Lrc<lint::LintLevelMap>,
+        [] 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
@@ -400,7 +400,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             return Ok(result);
         }
 
-        if !dep_node.kind.is_input() {
+        if !dep_node.kind.is_input() && !dep_node.kind.is_eval_always() {
             if let Some((prev_dep_node_index,
                          dep_node_index)) = self.dep_graph.try_mark_green_and_read(self,
                                                                                    &dep_node) {
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;
diff --git a/src/librustc_mir/util/pretty.rs b/src/librustc_mir/util/pretty.rs

Original file line number	Diff line number	Diff line change
`@@ -16,11 +16,13 @@ use syntax::source_map::MultiSpan;`
`16`	`16`	`use syntax::symbol::Symbol;`
`17`	`17`	`use util::nodemap::FxHashMap;`
`18`	`18`
	`19`	`+#[derive(Debug)]`
`19`	`20`	`pub struct LintLevelSets {`
`20`	`21`	`list: Vec<LintSet>,`
`21`	`22`	`lint_cap: Level,`
`22`	`23`	`}`
`23`	`24`
	`25`	`+#[derive(Debug)]`
`24`	`26`	`enum LintSet {`
`25`	`27`	`CommandLine {`
`26`	`28`	// -A,-W,-D flags, a `Symbol` for the flag itself and `Level` for which
`@@ -157,6 +159,7 @@ pub struct LintLevelsBuilder<'a> {`
`157`	`159`
`158`	`160`	`pub struct BuilderPush {`
`159`	`161`	`prev: u32,`
	`162`	`+ pub(super) changed: bool,`
`160`	`163`	`}`
`161`	`164`
`162`	`165`	`impl<'a> LintLevelsBuilder<'a> {`
`@@ -454,6 +457,7 @@ impl<'a> LintLevelsBuilder<'a> {`
`454`	`457`
`455`	`458`	`BuilderPush {`
`456`	`459`	`prev: prev,`
	`460`	`+ changed: prev != self.cur,`
`457`	`461`	`}`
`458`	`462`	`}`
`459`	`463`
`@@ -492,6 +496,7 @@ impl<'a> LintLevelsBuilder<'a> {`
`492`	`496`	`}`
`493`	`497`	`}`
`494`	`498`
	`499`	`+#[derive(Debug)]`
`495`	`500`	`pub struct LintLevelMap {`
`496`	`501`	`sets: LintLevelSets,`
`497`	`502`	`id_to_set: FxHashMap<HirId, u32>,`