From 6360cd5c44533e6384d1195c0d5eb8c7462e9ecc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Mon, 11 Jun 2018 08:48:15 +0200 Subject: [PATCH] Make some lints incremental --- src/librustc/dep_graph/dep_node.rs | 1 + src/librustc/lint/context.rs | 103 ++++++++++++++++-- src/librustc/lint/mod.rs | 1 + src/librustc/ty/context.rs | 32 ++---- src/librustc/ty/query/mod.rs | 2 + src/librustc/ty/query/plumbing.rs | 1 + src/librustc_driver/driver.rs | 2 +- src/librustc_lint/lib.rs | 53 +++++++-- .../lint/lint-group-nonstandard-style.stderr | 26 ++--- src/test/ui/lint/lint-impl-fn.stderr | 16 +-- src/test/ui/lint/suggestions.stderr | 36 +++--- 11 files changed, 196 insertions(+), 77 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 05f331145afe2..1f78895b76658 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -472,6 +472,7 @@ define_dep_nodes!( <'tcx> [] UnsafetyCheckResult(DefId), [] UnsafeDeriveOnReprPacked(DefId), + [] LintMod(DefId), [] CheckModAttrs(DefId), [] CheckModLoops(DefId), [] CheckModUnstableApiUsage(DefId), diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index 394e404fb881f..2275333171c88 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -27,6 +27,8 @@ use rustc_serialize::{Decoder, Decodable, Encoder, Encodable}; use session::{config, early_error, Session}; use ty::{self, TyCtxt, Ty}; use ty::layout::{LayoutError, LayoutOf, TyLayout}; +use ty::query::{Providers, queries}; + use util::nodemap::FxHashMap; use util::common::time; @@ -36,10 +38,12 @@ use syntax::edition; use syntax_pos::{MultiSpan, Span, symbol::{LocalInternedString, Symbol}}; use errors::DiagnosticBuilder; use hir; +use hir::def_id::DefId; use hir::def_id::LOCAL_CRATE; use hir::intravisit as hir_visit; use syntax::util::lev_distance::find_best_match_for_name; use syntax::visit as ast_visit; +use syntax::ast::{NodeId, CRATE_NODE_ID}; /// Information about the registered lints. /// @@ -55,6 +59,7 @@ pub struct LintStore { pre_expansion_passes: Option>, early_passes: Option>, late_passes: Option>, + late_module_passes: Option>, /// Lints indexed by name. by_name: FxHashMap, @@ -150,6 +155,7 @@ impl LintStore { pre_expansion_passes: Some(vec![]), early_passes: Some(vec![]), late_passes: Some(vec![]), + late_module_passes: Some(vec![]), by_name: Default::default(), future_incompatible: Default::default(), lint_groups: Default::default(), @@ -199,9 +205,14 @@ impl LintStore { pub fn register_late_pass(&mut self, sess: Option<&Session>, from_plugin: bool, + per_module: bool, pass: LateLintPassObject) { self.push_pass(sess, from_plugin, &pass); - self.late_passes.as_mut().unwrap().push(pass); + if per_module { + self.late_module_passes.as_mut().unwrap().push(pass); + } else { + self.late_passes.as_mut().unwrap().push(pass); + } } // Helper method for register_early/late_pass @@ -508,6 +519,7 @@ pub struct LateContext<'a, 'tcx: 'a> { pub tcx: TyCtxt<'a, 'tcx, 'tcx>, /// Side-tables for the body we are in. + // FIXME: Make this lazy to avoid running the TypeckTables query? pub tables: &'a ty::TypeckTables<'tcx>, /// Parameter environment for the item we are in. @@ -523,6 +535,9 @@ pub struct LateContext<'a, 'tcx: 'a> { /// Generic type parameters in scope for the item we are in. pub generics: Option<&'tcx hir::Generics>, + + /// We are only looking at one module + only_module: bool, } /// Context for lint checking of the AST, after expansion, before lowering to @@ -801,6 +816,12 @@ impl<'a, 'tcx> LateContext<'a, 'tcx> { pub fn current_lint_root(&self) -> ast::NodeId { self.last_ast_node_with_lint_attrs } + + fn process_mod(&mut self, m: &'tcx hir::Mod, s: Span, n: ast::NodeId) { + run_lints!(self, check_mod, m, s, n); + hir_visit::walk_mod(self, m, n); + run_lints!(self, check_mod_post, m, s, n); + } } impl<'a, 'tcx> LayoutOf for LateContext<'a, 'tcx> { @@ -932,9 +953,9 @@ impl<'a, 'tcx> hir_visit::Visitor<'tcx> for LateContext<'a, 'tcx> { } fn visit_mod(&mut self, m: &'tcx hir::Mod, s: Span, n: ast::NodeId) { - run_lints!(self, check_mod, m, s, n); - hir_visit::walk_mod(self, m, n); - run_lints!(self, check_mod_post, m, s, n); + if !self.only_module { + self.process_mod(m, s, n); + } } fn visit_local(&mut self, l: &'tcx hir::Local) { @@ -1199,11 +1220,65 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> } } +pub fn lint_mod<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) { + fn find_last_node_with_lint_attrs<'tcx>( + tcx: TyCtxt<'_, 'tcx, 'tcx>, + mut id: NodeId + ) -> NodeId { + let sets = tcx.lint_levels(LOCAL_CRATE); + loop { + let hir_id = tcx.hir().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 { + return CRATE_NODE_ID; + } + id = next; + } + } -/// Perform lint checking on a crate. -/// -/// Consumes the `lint_store` field of the `Session`. -pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { + // Restricts this to only items in this module + let access_levels = &tcx.privacy_access_levels(LOCAL_CRATE); + + let store = &tcx.sess.lint_store; + let passes = store.borrow_mut().late_module_passes.take(); + + let mut cx = LateContext { + tcx, + tables: &ty::TypeckTables::empty(None), + param_env: ty::ParamEnv::empty(), + access_levels, + lint_sess: LintSession { + lints: store.borrow(), + passes, + }, + last_ast_node_with_lint_attrs: find_last_node_with_lint_attrs( + tcx, + tcx.hir().as_local_node_id(module_def_id).unwrap(), + ), + generics: None, + only_module: true, + }; + + let (module, span, node_id) = tcx.hir().get_module(module_def_id); + cx.process_mod(module, span, node_id); + + // Put the lint store levels and passes back in the session. + let passes = cx.lint_sess.passes; + drop(cx.lint_sess.lints); + store.borrow_mut().late_module_passes = passes; +} + +pub(crate) fn provide(providers: &mut Providers<'_>) { + *providers = Providers { + lint_mod, + ..*providers + }; +} + +fn lint_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { let access_levels = &tcx.privacy_access_levels(LOCAL_CRATE); let krate = tcx.hir().krate(); @@ -1221,6 +1296,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { }, last_ast_node_with_lint_attrs: ast::CRATE_NODE_ID, generics: None, + only_module: false, }; // Visit the whole crate. @@ -1240,6 +1316,17 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { tcx.sess.lint_store.borrow_mut().late_passes = passes; } +/// Perform lint checking on a crate. +pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { + // Run per-module lints + for &module in tcx.hir().krate().modules.keys() { + queries::lint_mod::ensure(tcx, tcx.hir().local_def_id(module)); + } + + // Run whole crate non-incremental lints + lint_crate(tcx); +} + struct EarlyLintPassObjects<'a> { lints: &'a mut [EarlyLintPassObject], } diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index a95fa350bf1c3..41bc06f719611 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -807,6 +807,7 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for LintLevelMapBuilder<'a, 'tcx> { pub fn provide(providers: &mut Providers<'_>) { providers.lint_levels = lint_levels; + context::provide(providers); } /// 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 881c0d4e6d239..1a172c75f7c47 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -2854,28 +2854,18 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { pub fn lint_level_at_node(self, lint: &'static Lint, mut 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); + 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; + } } pub fn struct_span_lint_hir>(self, diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index 195bec11ee570..ff8d1df5f21ac 100644 --- a/src/librustc/ty/query/mod.rs +++ b/src/librustc/ty/query/mod.rs @@ -254,6 +254,8 @@ define_queries! { <'tcx> }, Other { + [] fn lint_mod: LintMod(DefId) -> (), + /// Checks the attributes in the module [] fn check_mod_attrs: CheckModAttrs(DefId) -> (), diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index e777c883c3787..3b880fc0f7c35 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -1244,6 +1244,7 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>, DepKind::MirBorrowCheck => { force!(mir_borrowck, def_id!()); } DepKind::UnsafetyCheckResult => { force!(unsafety_check_result, def_id!()); } DepKind::UnsafeDeriveOnReprPacked => { force!(unsafe_derive_on_repr_packed, def_id!()); } + DepKind::LintMod => { force!(lint_mod, def_id!()); } DepKind::CheckModAttrs => { force!(check_mod_attrs, def_id!()); } DepKind::CheckModLoops => { force!(check_mod_loops, def_id!()); } DepKind::CheckModUnstableApiUsage => { force!(check_mod_unstable_api_usage, def_id!()); } diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index c586c705676f4..24146755bd0fe 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -887,7 +887,7 @@ where ls.register_early_pass(Some(sess), true, false, pass); } for pass in late_lint_passes { - ls.register_late_pass(Some(sess), true, pass); + ls.register_late_pass(Some(sess), true, false, pass); } for (name, (to, deprecated_name)) in lint_groups { diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 6607951d2cd14..3a696d385cf66 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -133,37 +133,74 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { store.register_early_pass(sess, false, true, box BuiltinCombinedEarlyLintPass::new()); } - late_lint_methods!(declare_combined_late_lint_pass, [BuiltinCombinedLateLintPass, [ + // FIXME: Make a separate lint type which do not require typeck tables + + late_lint_methods!(declare_combined_late_lint_pass, [BuiltinCombinedModuleLateLintPass, [ HardwiredLints: HardwiredLints, WhileTrue: WhileTrue, ImproperCTypes: ImproperCTypes, VariantSizeDifferences: VariantSizeDifferences, BoxPointers: BoxPointers, - UnusedAttributes: UnusedAttributes, PathStatements: PathStatements, UnusedResults: UnusedResults, - NonSnakeCase: NonSnakeCase, NonUpperCaseGlobals: NonUpperCaseGlobals, NonShorthandFieldPatterns: NonShorthandFieldPatterns, UnusedAllocation: UnusedAllocation, + + // Depends on types used in type definitions MissingCopyImplementations: MissingCopyImplementations, - UnstableFeatures: UnstableFeatures, - InvalidNoMangleItems: InvalidNoMangleItems, + PluginAsLibrary: PluginAsLibrary, + + // Depends on referenced function signatures in expressions MutableTransmutes: MutableTransmutes, + + // Depends on types of fields, checks if they implement Drop UnionsWithDropFields: UnionsWithDropFields, - UnreachablePub: UnreachablePub, - UnnameableTestItems: UnnameableTestItems::new(), + TypeAliasBounds: TypeAliasBounds, + + // May Depend on constants elsewhere UnusedBrokenConst: UnusedBrokenConst, + TrivialConstraints: TrivialConstraints, TypeLimits: TypeLimits::new(), + ]], ['tcx]); + + store.register_late_pass(sess, false, true, box BuiltinCombinedModuleLateLintPass::new()); + + late_lint_methods!(declare_combined_late_lint_pass, [BuiltinCombinedLateLintPass, [ + + // Uses attr::is_used which is untracked, can't be an incremental module pass. + // Doesn't require type tables. Make a separate combined pass for that? + UnusedAttributes: UnusedAttributes, + + + // Checks crate attributes. Find out how that would work. + NonSnakeCase: NonSnakeCase, + + + // Needs to look at crate attributes. Make sure that works + UnstableFeatures: UnstableFeatures, + + // Depends on access levels + InvalidNoMangleItems: InvalidNoMangleItems, + + // Depends on access levels + UnreachablePub: UnreachablePub, + + UnnameableTestItems: UnnameableTestItems::new(), + + // Tracks attributes of parents MissingDoc: MissingDoc::new(), + + // Depends on access levels MissingDebugImplementations: MissingDebugImplementations::new(), + ExplicitOutlivesRequirements: ExplicitOutlivesRequirements, ]], ['tcx]); - store.register_late_pass(sess, false, box BuiltinCombinedLateLintPass::new()); + store.register_late_pass(sess, false, false, box BuiltinCombinedLateLintPass::new()); add_lint_group!(sess, "nonstandard_style", diff --git a/src/test/ui/lint/lint-group-nonstandard-style.stderr b/src/test/ui/lint/lint-group-nonstandard-style.stderr index f3c7d70054b77..aedf03755be29 100644 --- a/src/test/ui/lint/lint-group-nonstandard-style.stderr +++ b/src/test/ui/lint/lint-group-nonstandard-style.stderr @@ -11,6 +11,19 @@ LL | #![warn(nonstandard_style)] | ^^^^^^^^^^^^^^^^^ = note: #[warn(non_camel_case_types)] implied by #[warn(nonstandard_style)] +error: static variable `bad` should have an upper case name + --> $DIR/lint-group-nonstandard-style.rs:14:16 + | +LL | static bad: isize = 1; //~ ERROR should have an upper + | ^^^ help: convert the identifier to upper case: `BAD` + | +note: lint level defined here + --> $DIR/lint-group-nonstandard-style.rs:10:14 + | +LL | #[forbid(nonstandard_style)] + | ^^^^^^^^^^^^^^^^^ + = note: #[forbid(non_upper_case_globals)] implied by #[forbid(nonstandard_style)] + error: function `CamelCase` should have a snake case name --> $DIR/lint-group-nonstandard-style.rs:4:4 | @@ -37,19 +50,6 @@ LL | #[forbid(nonstandard_style)] | ^^^^^^^^^^^^^^^^^ = note: #[forbid(non_snake_case)] implied by #[forbid(nonstandard_style)] -error: static variable `bad` should have an upper case name - --> $DIR/lint-group-nonstandard-style.rs:14:16 - | -LL | static bad: isize = 1; //~ ERROR should have an upper - | ^^^ help: convert the identifier to upper case: `BAD` - | -note: lint level defined here - --> $DIR/lint-group-nonstandard-style.rs:10:14 - | -LL | #[forbid(nonstandard_style)] - | ^^^^^^^^^^^^^^^^^ - = note: #[forbid(non_upper_case_globals)] implied by #[forbid(nonstandard_style)] - warning: function `CamelCase` should have a snake case name --> $DIR/lint-group-nonstandard-style.rs:20:12 | diff --git a/src/test/ui/lint/lint-impl-fn.stderr b/src/test/ui/lint/lint-impl-fn.stderr index b0a3f61978435..3e548abd5e6fc 100644 --- a/src/test/ui/lint/lint-impl-fn.stderr +++ b/src/test/ui/lint/lint-impl-fn.stderr @@ -11,25 +11,25 @@ LL | #[deny(while_true)] | ^^^^^^^^^^ error: denote infinite loops with `loop { ... }` - --> $DIR/lint-impl-fn.rs:18:25 + --> $DIR/lint-impl-fn.rs:27:5 | -LL | fn foo(&self) { while true {} } //~ ERROR: infinite loops - | ^^^^^^^^^^ help: use `loop` +LL | while true {} //~ ERROR: infinite loops + | ^^^^^^^^^^ help: use `loop` | note: lint level defined here - --> $DIR/lint-impl-fn.rs:13:8 + --> $DIR/lint-impl-fn.rs:25:8 | LL | #[deny(while_true)] | ^^^^^^^^^^ error: denote infinite loops with `loop { ... }` - --> $DIR/lint-impl-fn.rs:27:5 + --> $DIR/lint-impl-fn.rs:18:25 | -LL | while true {} //~ ERROR: infinite loops - | ^^^^^^^^^^ help: use `loop` +LL | fn foo(&self) { while true {} } //~ ERROR: infinite loops + | ^^^^^^^^^^ help: use `loop` | note: lint level defined here - --> $DIR/lint-impl-fn.rs:25:8 + --> $DIR/lint-impl-fn.rs:13:8 | LL | #[deny(while_true)] | ^^^^^^^^^^ diff --git a/src/test/ui/lint/suggestions.stderr b/src/test/ui/lint/suggestions.stderr index c70f17f95d028..cf27fa0199f64 100644 --- a/src/test/ui/lint/suggestions.stderr +++ b/src/test/ui/lint/suggestions.stderr @@ -44,6 +44,24 @@ LL | || b = 1; | |____________| | help: remove this `mut` +warning: denote infinite loops with `loop { ... }` + --> $DIR/suggestions.rs:46:5 + | +LL | while true { + | ^^^^^^^^^^ help: use `loop` + | + = note: #[warn(while_true)] on by default + +warning: the `warp_factor:` in this pattern is redundant + --> $DIR/suggestions.rs:61:23 + | +LL | Equinox { warp_factor: warp_factor } => {} + | ------------^^^^^^^^^^^^ + | | + | help: remove this + | + = note: #[warn(non_shorthand_field_patterns)] on by default + error: const items should never be #[no_mangle] --> $DIR/suggestions.rs:6:14 | @@ -97,23 +115,5 @@ LL | #[no_mangle] pub(crate) fn crossfield() {} | | | help: remove this attribute -warning: denote infinite loops with `loop { ... }` - --> $DIR/suggestions.rs:46:5 - | -LL | while true { - | ^^^^^^^^^^ help: use `loop` - | - = note: #[warn(while_true)] on by default - -warning: the `warp_factor:` in this pattern is redundant - --> $DIR/suggestions.rs:61:23 - | -LL | Equinox { warp_factor: warp_factor } => {} - | ------------^^^^^^^^^^^^ - | | - | help: remove this - | - = note: #[warn(non_shorthand_field_patterns)] on by default - error: aborting due to 3 previous errors