Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only insert nodes which changes lint levels in the LintLevelMap #58176

Merged
merged 2 commits into from
Mar 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1033,6 +1033,7 @@ impl<'hir> Map<'hir> {
pub fn attrs(&self, id: NodeId) -> &'hir [ast::Attribute] {
self.read(id); // reveals attributes on the node
let attrs = match self.find(id) {
Some(Node::Local(l)) => Some(&l.attrs[..]),
Some(Node::Item(i)) => Some(&i.attrs[..]),
Some(Node::ForeignItem(fi)) => Some(&fi.attrs[..]),
Some(Node::TraitItem(ref ti)) => Some(&ti.attrs[..]),
Expand Down
7 changes: 2 additions & 5 deletions src/librustc/lint/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ pub struct LintLevelsBuilder<'a> {

pub struct BuilderPush {
prev: u32,
pub(super) changed: bool,
}

impl<'a> LintLevelsBuilder<'a> {
Expand Down Expand Up @@ -454,6 +455,7 @@ impl<'a> LintLevelsBuilder<'a> {

BuilderPush {
prev: prev,
changed: prev != self.cur,
}
}

Expand Down Expand Up @@ -512,11 +514,6 @@ impl LintLevelMap {
self.sets.get_lint_level(lint, *idx, None, session)
})
}

/// Returns if this `id` has lint level information.
pub fn lint_level_set(&self, id: HirId) -> Option<u32> {
self.id_to_set.get(&id).cloned()
}
}

impl<'a> HashStable<StableHashingContext<'a>> for LintLevelMap {
Expand Down
21 changes: 17 additions & 4 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,16 @@ pub fn struct_lint_level<'a>(sess: &'a Session,
return err
}

pub fn maybe_lint_level_root(tcx: TyCtxt<'_, '_, '_>, id: hir::HirId) -> bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rename this to is_lint_level_root?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just because this is true, doesn't mean it is an actual lint root (i.e. shows up in the lint level map), hence the maybe.

let attrs = tcx.hir().attrs_by_hir_id(id);
for attr in attrs {
if Level::from_str(&attr.name().as_str()).is_some() {
return true;
}
}
false
}

fn lint_levels<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, cnum: CrateNum)
-> Lrc<LintLevelMap>
{
Expand All @@ -731,9 +741,10 @@ fn lint_levels<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, cnum: CrateNum)
};
let krate = tcx.hir().krate();

builder.with_lint_attrs(hir::CRATE_HIR_ID, &krate.attrs, |builder| {
intravisit::walk_crate(builder, krate);
});
let push = builder.levels.push(&krate.attrs);
builder.levels.register_id(hir::CRATE_HIR_ID);
intravisit::walk_crate(&mut builder, krate);
builder.levels.pop(push);

Lrc::new(builder.levels.build_map())
}
Expand All @@ -751,7 +762,9 @@ impl<'a, 'tcx> LintLevelMapBuilder<'a, 'tcx> {
where F: FnOnce(&mut Self)
{
let push = self.levels.push(attrs);
self.levels.register_id(id);
if push.changed {
self.levels.register_id(id);
}
f(self);
self.levels.pop(push);
}
Expand Down
60 changes: 37 additions & 23 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2856,30 +2856,44 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
err.emit()
}

pub fn lint_level_at_node(self, lint: &'static Lint, mut id: hir::HirId)
-> (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 {
if let Some(pair) = sets.level_and_source(lint, id, self.sess) {
return pair
}
let next = self.hir().get_parent_node_by_hir_id(id);
if next == id {
bug!("lint traversal reached the root of the crate");
}
id = next;
/// Walks upwards from `id` to find a node which might change lint levels with attributes.
/// It stops at `bound` and just returns it if reached.
pub fn maybe_lint_level_root_bounded(
self,
mut id: hir::HirId,
bound: hir::HirId,
) -> hir::HirId {
loop {
if id == bound {
return bound;
}
})
if lint::maybe_lint_level_root(self, id) {
return id;
}
let next = self.hir().get_parent_node_by_hir_id(id);
if next == id {
bug!("lint traversal reached the root of the crate");
}
id = next;
}
}

pub fn lint_level_at_node(
self,
lint: &'static Lint,
mut id: hir::HirId
) -> (lint::Level, lint::LintSource) {
let sets = self.lint_levels(LOCAL_CRATE);
loop {
if let Some(pair) = sets.level_and_source(lint, id, self.sess) {
return pair
}
let next = self.hir().get_parent_node_by_hir_id(id);
if next == id {
bug!("lint traversal reached the root of the crate");
}
id = next;
}
}

pub fn struct_span_lint_hir<S: Into<MultiSpan>>(self,
Expand Down
30 changes: 19 additions & 11 deletions src/librustc_mir/build/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ use crate::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;
Expand Down Expand Up @@ -309,16 +308,25 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
let source_scope = self.source_scope;
let tcx = self.hir.tcx();
if let LintLevel::Explicit(current_hir_id) = lint_level {
let same_lint_scopes = tcx.dep_graph.with_ignore(|| {
let sets = tcx.lint_levels(LOCAL_CRATE);
let parent_hir_id = self.source_scope_local_data[source_scope].lint_root;
sets.lint_level_set(parent_hir_id) == sets.lint_level_set(current_hir_id)
});

if !same_lint_scopes {
self.source_scope =
self.new_source_scope(region_scope.1.span, lint_level,
None);
// Use `maybe_lint_level_root_bounded` with `root_lint_level` as a bound
// to avoid adding Hir dependences on our parents.
// We estimate the true lint roots here to avoid creating a lot of source scopes.

let parent_root = tcx.maybe_lint_level_root_bounded(
self.source_scope_local_data[source_scope].lint_root,
self.hir.root_lint_level,
);
let current_root = tcx.maybe_lint_level_root_bounded(
current_hir_id,
self.hir.root_lint_level
);

if parent_root != current_root {
self.source_scope = self.new_source_scope(
region_scope.1.span,
LintLevel::Explicit(current_root),
None
);
}
}
self.push_scope(region_scope);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/hair/cx/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,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.hir_id),
lint_level: LintLevel::Explicit(local.hir_id),
},
opt_destruction_scope: opt_dxn_ext,
span: stmt_span,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/hair/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,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.hir_id),
lint_level: LintLevel::Explicit(self.hir_id),
},
};

Expand Down
41 changes: 2 additions & 39 deletions src/librustc_mir/hair/cx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::hair::*;
use crate::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;
Expand Down Expand Up @@ -76,11 +76,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: src_id,
param_env: tcx.param_env(src_def_id),
identity_substs: InternalSubsts::identity_for_item(tcx.global_tcx(), src_def_id),
region_scope_tree: tcx.region_scope_tree(src_def_id),
Expand Down Expand Up @@ -197,18 +196,6 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> {
ty.needs_drop(self.tcx.global_tcx(), param_env)
}

fn lint_level_of(&self, hir_id: hir::HirId) -> LintLevel {
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(hir_id)
} else {
LintLevel::Inherited
}
}

pub fn tcx(&self) -> TyCtxt<'a, 'gcx, 'tcx> {
self.tcx
}
Expand Down Expand Up @@ -236,30 +223,6 @@ impl UserAnnotatedTyHelpers<'gcx, 'tcx> for Cx<'_, 'gcx, 'tcx> {
}
}

fn lint_level_for_hir_id(tcx: TyCtxt<'_, '_, '_>, mut id: hir::HirId) -> hir::HirId {
// 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 {
if sets.lint_level_set(id).is_some() {
return id
}
let next = tcx.hir().get_parent_node_by_hir_id(id);
if next == id {
bug!("lint traversal reached the root of the crate");
}
id = next;
}
})
}

mod block;
mod expr;
mod to_ref;