Skip to content

Commit

Permalink
Auto merge of #57293 - Zoxc:incr-passes3, r=<try>
Browse files Browse the repository at this point in the history
[WIP] Make some lints incremental

Blocked on #57253

r? @michaelwoerister
  • Loading branch information
bors committed Jan 19, 2019
2 parents 1bc6bae + 2f518af commit bc1de3e
Show file tree
Hide file tree
Showing 10 changed files with 148 additions and 34 deletions.
1 change: 1 addition & 0 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ define_dep_nodes!( <'tcx>
[] UnsafetyCheckResult(DefId),
[] UnsafeDeriveOnReprPacked(DefId),

[] LintMod(DefId),
[] CheckModAttrs(DefId),
[] CheckModLoops(DefId),
[] CheckModUnstableApiUsage(DefId),
Expand Down
78 changes: 74 additions & 4 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -54,6 +57,7 @@ pub struct LintStore {
pre_expansion_passes: Option<Vec<EarlyLintPassObject>>,
early_passes: Option<Vec<EarlyLintPassObject>>,
late_passes: Option<Vec<LateLintPassObject>>,
late_module_passes: Option<Vec<LateLintPassObject>>,

/// Lints indexed by name.
by_name: FxHashMap<String, TargetLint>,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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`.
Expand All @@ -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.
Expand All @@ -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(
Expand Down
1 change: 1 addition & 0 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/ty/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) -> (),

Expand Down
1 change: 1 addition & 0 deletions src/librustc/ty/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!()); }
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
53 changes: 45 additions & 8 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
26 changes: 13 additions & 13 deletions src/test/ui/lint/lint-group-nonstandard-style.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand All @@ -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

16 changes: 8 additions & 8 deletions src/test/ui/lint/lint-impl-fn.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
| ^^^^^^^^^^
Expand Down

0 comments on commit bc1de3e

Please sign in to comment.