Skip to content

Commit

Permalink
Rollup merge of rust-lang#66789 - eddyb:mir-source-scope-local-data, …
Browse files Browse the repository at this point in the history
…r=oli-obk

rustc: move mir::SourceScopeLocalData to a field of SourceScopeData.

By having one `ClearCrossCrate<SourceScopeLocalData>` for each scope, as opposed to a single `ClearCrossCrate` for all the `SourceScopeLocalData`s, we can represent the fact that some scopes have `SourceScopeLocalData` associated with them, and some don't.

This is useful when doing MIR inlining across crates, because the `ClearCrossCrate` will be `Clear` for the cross-crate MIR scopes and `Set` for the local ones.

Also see rust-lang#66203 (comment) for some context around this approach.

Fixes rust-lang#51314.
  • Loading branch information
RalfJung authored Nov 29, 2019
2 parents 535a09a + 3aead85 commit b3d835f
Show file tree
Hide file tree
Showing 11 changed files with 110 additions and 122 deletions.
17 changes: 11 additions & 6 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ pub struct Body<'tcx> {
/// and used for debuginfo. Indexed by a `SourceScope`.
pub source_scopes: IndexVec<SourceScope, SourceScopeData>,

/// Crate-local information for each source scope, that can't (and
/// needn't) be tracked across crates.
pub source_scope_local_data: ClearCrossCrate<IndexVec<SourceScope, SourceScopeLocalData>>,

/// The yield type of the function, if it is a generator.
pub yield_ty: Option<Ty<'tcx>>,

Expand Down Expand Up @@ -163,7 +159,6 @@ impl<'tcx> Body<'tcx> {
pub fn new(
basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>,
source_scopes: IndexVec<SourceScope, SourceScopeData>,
source_scope_local_data: ClearCrossCrate<IndexVec<SourceScope, SourceScopeLocalData>>,
local_decls: LocalDecls<'tcx>,
user_type_annotations: CanonicalUserTypeAnnotations<'tcx>,
arg_count: usize,
Expand All @@ -183,7 +178,6 @@ impl<'tcx> Body<'tcx> {
phase: MirPhase::Build,
basic_blocks,
source_scopes,
source_scope_local_data,
yield_ty: None,
generator_drop: None,
generator_layout: None,
Expand Down Expand Up @@ -429,6 +423,13 @@ pub enum ClearCrossCrate<T> {
}

impl<T> ClearCrossCrate<T> {
pub fn as_ref(&'a self) -> ClearCrossCrate<&'a T> {
match self {
ClearCrossCrate::Clear => ClearCrossCrate::Clear,
ClearCrossCrate::Set(v) => ClearCrossCrate::Set(v),
}
}

pub fn assert_crate_local(self) -> T {
match self {
ClearCrossCrate::Clear => bug!("unwrapping cross-crate data"),
Expand Down Expand Up @@ -2021,6 +2022,10 @@ rustc_index::newtype_index! {
pub struct SourceScopeData {
pub span: Span,
pub parent_scope: Option<SourceScope>,

/// Crate-local information for this source scope, that can't (and
/// needn't) be tracked across crates.
pub local_data: ClearCrossCrate<SourceScopeLocalData>,
}

#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable)]
Expand Down
1 change: 1 addition & 0 deletions src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ macro_rules! make_mir_visitor {
let SourceScopeData {
span,
parent_scope,
local_data: _,
} = scope_data;

self.visit_span(span);
Expand Down
67 changes: 34 additions & 33 deletions src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,11 +300,10 @@ fn do_mir_borrowck<'a, 'tcx>(
let mut initial_diag =
mbcx.report_conflicting_borrow(location, (&place, span), bk, &borrow);

let lint_root = if let ClearCrossCrate::Set(ref vsi) = mbcx.body.source_scope_local_data {
let scope = mbcx.body.source_info(location).scope;
vsi[scope].lint_root
} else {
id
let scope = mbcx.body.source_info(location).scope;
let lint_root = match &mbcx.body.source_scopes[scope].local_data {
ClearCrossCrate::Set(data) => data.lint_root,
_ => id,
};

// Span and message don't matter; we overwrite them below anyway
Expand Down Expand Up @@ -338,38 +337,40 @@ fn do_mir_borrowck<'a, 'tcx>(
debug!("mbcx.used_mut: {:?}", mbcx.used_mut);
let used_mut = mbcx.used_mut;
for local in mbcx.body.mut_vars_and_args_iter().filter(|local| !used_mut.contains(local)) {
if let ClearCrossCrate::Set(ref vsi) = mbcx.body.source_scope_local_data {
let local_decl = &mbcx.body.local_decls[local];

// Skip over locals that begin with an underscore or have no name
match mbcx.local_names[local] {
Some(name) => if name.as_str().starts_with("_") {
continue;
},
None => continue,
}
let local_decl = &mbcx.body.local_decls[local];
let lint_root = match &mbcx.body.source_scopes[local_decl.source_info.scope].local_data {
ClearCrossCrate::Set(data) => data.lint_root,
_ => continue,
};

let span = local_decl.source_info.span;
if span.desugaring_kind().is_some() {
// If the `mut` arises as part of a desugaring, we should ignore it.
// Skip over locals that begin with an underscore or have no name
match mbcx.local_names[local] {
Some(name) => if name.as_str().starts_with("_") {
continue;
}
},
None => continue,
}

let mut_span = tcx.sess.source_map().span_until_non_whitespace(span);
tcx.struct_span_lint_hir(
UNUSED_MUT,
vsi[local_decl.source_info.scope].lint_root,
span,
"variable does not need to be mutable",
)
.span_suggestion_short(
mut_span,
"remove this `mut`",
String::new(),
Applicability::MachineApplicable,
)
.emit();
let span = local_decl.source_info.span;
if span.desugaring_kind().is_some() {
// If the `mut` arises as part of a desugaring, we should ignore it.
continue;
}

let mut_span = tcx.sess.source_map().span_until_non_whitespace(span);
tcx.struct_span_lint_hir(
UNUSED_MUT,
lint_root,
span,
"variable does not need to be mutable",
)
.span_suggestion_short(
mut_span,
"remove this `mut`",
String::new(),
Applicability::MachineApplicable,
)
.emit();
}

// Buffer any move errors that we collected and de-duplicated.
Expand Down
9 changes: 5 additions & 4 deletions src/librustc_mir/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,6 @@ struct Builder<'a, 'tcx> {
/// The vector of all scopes that we have created thus far;
/// we track this for debuginfo later.
source_scopes: IndexVec<SourceScope, SourceScopeData>,
source_scope_local_data: IndexVec<SourceScope, SourceScopeLocalData>,
source_scope: SourceScope,

/// The guard-context: each time we build the guard expression for
Expand Down Expand Up @@ -704,7 +703,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block_context: BlockContext::new(),
source_scopes: IndexVec::new(),
source_scope: OUTERMOST_SOURCE_SCOPE,
source_scope_local_data: IndexVec::new(),
guard_context: vec![],
push_unsafe_count: 0,
unpushed_unsafe: safety,
Expand Down Expand Up @@ -741,7 +739,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Body::new(
self.cfg.basic_blocks,
self.source_scopes,
ClearCrossCrate::Set(self.source_scope_local_data),
self.local_decls,
self.canonical_user_type_annotations,
self.arg_count,
Expand Down Expand Up @@ -941,7 +938,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.hir.root_lint_level
);
let parent_root = tcx.maybe_lint_level_root_bounded(
self.source_scope_local_data[original_source_scope].lint_root,
self.source_scopes[original_source_scope]
.local_data
.as_ref()
.assert_crate_local()
.lint_root,
self.hir.root_lint_level,
);
if current_root != parent_root {
Expand Down
23 changes: 13 additions & 10 deletions src/librustc_mir/build/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// 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.source_scopes[source_scope]
.local_data
.as_ref()
.assert_crate_local()
.lint_root,
self.hir.root_lint_level,
);
let current_root = tcx.maybe_lint_level_root_bounded(
Expand Down Expand Up @@ -648,23 +652,22 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let parent = self.source_scope;
debug!("new_source_scope({:?}, {:?}, {:?}) - parent({:?})={:?}",
span, lint_level, safety,
parent, self.source_scope_local_data.get(parent));
let scope = self.source_scopes.push(SourceScopeData {
span,
parent_scope: Some(parent),
});
parent, self.source_scopes.get(parent));
let scope_local_data = SourceScopeLocalData {
lint_root: if let LintLevel::Explicit(lint_root) = lint_level {
lint_root
} else {
self.source_scope_local_data[parent].lint_root
self.source_scopes[parent].local_data.as_ref().assert_crate_local().lint_root
},
safety: safety.unwrap_or_else(|| {
self.source_scope_local_data[parent].safety
self.source_scopes[parent].local_data.as_ref().assert_crate_local().safety
})
};
self.source_scope_local_data.push(scope_local_data);
scope
self.source_scopes.push(SourceScopeData {
span,
parent_scope: Some(parent),
local_data: ClearCrossCrate::Set(scope_local_data),
})
}

/// Given a span and the current source scope, make a SourceInfo.
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -849,8 +849,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
} else {
block.terminator().source_info
};
match body.source_scope_local_data {
mir::ClearCrossCrate::Set(ref ivs) => Some(ivs[source_info.scope].lint_root),
match &body.source_scopes[source_info.scope].local_data {
mir::ClearCrossCrate::Set(data) => Some(data.lint_root),
mir::ClearCrossCrate::Clear => None,
}
});
Expand Down
20 changes: 12 additions & 8 deletions src/librustc_mir/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,9 @@ fn build_drop_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, ty: Option<Ty<'tcx>>)
let mut body = Body::new(
blocks,
IndexVec::from_elem_n(
SourceScopeData { span: span, parent_scope: None }, 1
SourceScopeData { span, parent_scope: None, local_data: ClearCrossCrate::Clear },
1,
),
ClearCrossCrate::Clear,
local_decls_for_sig(&sig, span),
IndexVec::new(),
sig.inputs().len(),
Expand Down Expand Up @@ -365,9 +365,13 @@ impl CloneShimBuilder<'tcx> {
Body::new(
self.blocks,
IndexVec::from_elem_n(
SourceScopeData { span: self.span, parent_scope: None }, 1
SourceScopeData {
span: self.span,
parent_scope: None,
local_data: ClearCrossCrate::Clear,
},
1,
),
ClearCrossCrate::Clear,
self.local_decls,
IndexVec::new(),
self.sig.inputs().len(),
Expand Down Expand Up @@ -825,9 +829,9 @@ fn build_call_shim<'tcx>(
let mut body = Body::new(
blocks,
IndexVec::from_elem_n(
SourceScopeData { span: span, parent_scope: None }, 1
SourceScopeData { span, parent_scope: None, local_data: ClearCrossCrate::Clear },
1,
),
ClearCrossCrate::Clear,
local_decls,
IndexVec::new(),
sig.inputs().len(),
Expand Down Expand Up @@ -911,9 +915,9 @@ pub fn build_adt_ctor(tcx: TyCtxt<'_>, ctor_id: DefId) -> &Body<'_> {
let body = Body::new(
IndexVec::from_elem_n(start_block, 1),
IndexVec::from_elem_n(
SourceScopeData { span: span, parent_scope: None }, 1
SourceScopeData { span, parent_scope: None, local_data: ClearCrossCrate::Clear },
1,
),
ClearCrossCrate::Clear,
local_decls,
IndexVec::new(),
sig.inputs().len(),
Expand Down
33 changes: 11 additions & 22 deletions src/librustc_mir/transform/check_unsafety.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use rustc_data_structures::fx::FxHashSet;
use rustc_index::vec::IndexVec;
use rustc_data_structures::sync::Lrc;

use rustc::ty::query::Providers;
use rustc::ty::{self, TyCtxt};
Expand All @@ -24,7 +22,6 @@ pub struct UnsafetyChecker<'a, 'tcx> {
body: &'a Body<'tcx>,
const_context: bool,
min_const_fn: bool,
source_scope_local_data: &'a IndexVec<SourceScope, SourceScopeLocalData>,
violations: Vec<UnsafetyViolation>,
source_info: SourceInfo,
tcx: TyCtxt<'tcx>,
Expand All @@ -39,7 +36,6 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
const_context: bool,
min_const_fn: bool,
body: &'a Body<'tcx>,
source_scope_local_data: &'a IndexVec<SourceScope, SourceScopeLocalData>,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
) -> Self {
Expand All @@ -51,7 +47,6 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
body,
const_context,
min_const_fn,
source_scope_local_data,
violations: vec![],
source_info: SourceInfo {
span: body.span,
Expand Down Expand Up @@ -219,8 +214,11 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
if context.is_borrow() {
if util::is_disaligned(self.tcx, self.body, self.param_env, place) {
let source_info = self.source_info;
let lint_root =
self.source_scope_local_data[source_info.scope].lint_root;
let lint_root = self.body.source_scopes[source_info.scope]
.local_data
.as_ref()
.assert_crate_local()
.lint_root;
self.register_violations(&[UnsafetyViolation {
source_info,
description: Symbol::intern("borrow of packed field"),
Expand Down Expand Up @@ -346,7 +344,11 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
fn register_violations(&mut self,
violations: &[UnsafetyViolation],
unsafe_blocks: &[(hir::HirId, bool)]) {
let safety = self.source_scope_local_data[self.source_info.scope].safety;
let safety = self.body.source_scopes[self.source_info.scope]
.local_data
.as_ref()
.assert_crate_local()
.safety;
let within_unsafe = match safety {
// `unsafe` blocks are required in safe code
Safety::Safe => {
Expand Down Expand Up @@ -516,17 +518,6 @@ fn unsafety_check_result(tcx: TyCtxt<'_>, def_id: DefId) -> UnsafetyCheckResult
// `mir_built` force this.
let body = &tcx.mir_built(def_id).borrow();

let source_scope_local_data = match body.source_scope_local_data {
ClearCrossCrate::Set(ref data) => data,
ClearCrossCrate::Clear => {
debug!("unsafety_violations: {:?} - remote, skipping", def_id);
return UnsafetyCheckResult {
violations: Lrc::new([]),
unsafe_blocks: Lrc::new([])
}
}
};

let param_env = tcx.param_env(def_id);

let id = tcx.hir().as_local_hir_id(def_id).unwrap();
Expand All @@ -536,9 +527,7 @@ fn unsafety_check_result(tcx: TyCtxt<'_>, def_id: DefId) -> UnsafetyCheckResult
hir::BodyOwnerKind::Const |
hir::BodyOwnerKind::Static(_) => (true, false),
};
let mut checker = UnsafetyChecker::new(
const_context, min_const_fn,
body, source_scope_local_data, tcx, param_env);
let mut checker = UnsafetyChecker::new(const_context, min_const_fn, body, tcx, param_env);
checker.visit_body(body);

check_unused_unsafe(tcx, def_id, &checker.used_unsafe, &mut checker.inherited_blocks);
Expand Down
Loading

0 comments on commit b3d835f

Please sign in to comment.