From db0c30bcc1840f64791648e237636dcb6f6306f8 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 1/2] Make some lints incremental --- src/librustc/dep_graph/dep_node.rs | 1 + src/librustc/lint/context.rs | 78 ++++++++++++++++++++++++++++-- src/librustc/lint/mod.rs | 1 + src/librustc/ty/context.rs | 2 + 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 +++++++++++++++++--- 8 files changed, 127 insertions(+), 13 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index d1067b70778ee..618987a803591 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 f5a7919ef09c8..0130cc58fae24 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 std::default::Default as StdDefault; @@ -35,6 +37,7 @@ 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; @@ -54,6 +57,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 +154,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(), @@ -192,9 +197,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 @@ -501,6 +511,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. @@ -516,6 +527,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 @@ -787,6 +801,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> { @@ -918,9 +938,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) { @@ -1191,6 +1211,50 @@ impl<'a> ast_visit::Visitor<'a> for EarlyContext<'a> { } +pub fn lint_mod<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) { + // Look for the parents of the module and for attributes on them + // to populate last_ast_node_with_lint_attrs + + // 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, + }, + // Invalid for modules. + // FIXME: Have the modules require the parent module's attribute + last_ast_node_with_lint_attrs: ast::CRATE_NODE_ID, + + 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 + }; +} + /// Perform lint checking on a crate. /// /// Consumes the `lint_store` field of the `Session`. @@ -1212,6 +1276,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. @@ -1229,6 +1294,11 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { // Put the lint store levels and passes back in the session. tcx.sess.lint_store.borrow_mut().late_passes = passes; + + // Run per-module lints + for &module in tcx.hir().krate().modules.keys() { + queries::lint_mod::ensure(tcx, tcx.hir().local_def_id(module)); + } } pub fn check_ast_crate( diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 730ce919bd295..46c43586dada8 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -721,6 +721,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 d69219efbd884..1c810b48dcd64 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -2835,6 +2835,8 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { // 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. + // FIXME: How is this correct? + // Just remove it. self.dep_graph.with_ignore(|| { let sets = self.lint_levels(LOCAL_CRATE); loop { diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index 88c20547a2108..f0fda8d87f014 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 5b7cb55b7bc3a..da0546620a33a 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 15cb5b032ade4..9c469637f8bdb 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -901,7 +901,7 @@ where ls.register_early_pass(Some(sess), true, 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 2d764d11c66aa..570a0d9b4dc56 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -123,37 +123,74 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { DeprecatedAttr, ); - 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", From 2f518af7189825955903c5c1e4e9516f1d3e3131 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sun, 6 Jan 2019 16:54:53 +0100 Subject: [PATCH 2/2] Update tests --- .../lint/lint-group-nonstandard-style.stderr | 26 +++++++++---------- src/test/ui/lint/lint-impl-fn.stderr | 16 ++++++------ 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/test/ui/lint/lint-group-nonstandard-style.stderr b/src/test/ui/lint/lint-group-nonstandard-style.stderr index f3c7d70054b77..fa793726fe661 100644 --- a/src/test/ui/lint/lint-group-nonstandard-style.stderr +++ b/src/test/ui/lint/lint-group-nonstandard-style.stderr @@ -37,19 +37,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 | @@ -63,5 +50,18 @@ LL | #![warn(nonstandard_style)] | ^^^^^^^^^^^^^^^^^ = note: #[warn(non_snake_case)] 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: aborting due to 3 previous errors 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)] | ^^^^^^^^^^