Skip to content

Commit

Permalink
Auto merge of #45384 - mikhail-m1:mir_add_false_edges_terminator_kind…
Browse files Browse the repository at this point in the history
…, r=arielb1

add TerminatorKind::FalseEdges and use it in matches

impl #45184 and fixes #45043 right way.

False edges unexpectedly affects uninitialized variables analysis in MIR borrowck.
  • Loading branch information
bors committed Nov 4, 2017
2 parents 2278506 + 7d87054 commit 95a4016
Show file tree
Hide file tree
Showing 21 changed files with 447 additions and 48 deletions.
9 changes: 8 additions & 1 deletion src/librustc/ich/impls_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ for mir::Terminator<'gcx> {
mir::TerminatorKind::Drop { .. } |
mir::TerminatorKind::DropAndReplace { .. } |
mir::TerminatorKind::Yield { .. } |
mir::TerminatorKind::Call { .. } => false,
mir::TerminatorKind::Call { .. } |
mir::TerminatorKind::FalseEdges { .. } => false,
};

if hash_spans_unconditionally {
Expand Down Expand Up @@ -210,6 +211,12 @@ for mir::TerminatorKind<'gcx> {
target.hash_stable(hcx, hasher);
cleanup.hash_stable(hcx, hasher);
}
mir::TerminatorKind::FalseEdges { ref real_target, ref imaginary_targets } => {
real_target.hash_stable(hcx, hasher);
for target in imaginary_targets {
target.hash_stable(hcx, hasher);
}
}
}
}
}
Expand Down
32 changes: 28 additions & 4 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,11 @@ pub enum TerminatorKind<'tcx> {

/// Indicates the end of the dropping of a generator
GeneratorDrop,

FalseEdges {
real_target: BasicBlock,
imaginary_targets: Vec<BasicBlock>
},
}

impl<'tcx> Terminator<'tcx> {
Expand Down Expand Up @@ -731,6 +736,11 @@ impl<'tcx> TerminatorKind<'tcx> {
}
Assert { target, cleanup: Some(unwind), .. } => vec![target, unwind].into_cow(),
Assert { ref target, .. } => slice::ref_slice(target).into_cow(),
FalseEdges { ref real_target, ref imaginary_targets } => {
let mut s = vec![*real_target];
s.extend_from_slice(imaginary_targets);
s.into_cow()
}
}
}

Expand All @@ -757,7 +767,12 @@ impl<'tcx> TerminatorKind<'tcx> {
vec![target]
}
Assert { ref mut target, cleanup: Some(ref mut unwind), .. } => vec![target, unwind],
Assert { ref mut target, .. } => vec![target]
Assert { ref mut target, .. } => vec![target],
FalseEdges { ref mut real_target, ref mut imaginary_targets } => {
let mut s = vec![real_target];
s.extend(imaginary_targets.iter_mut());
s
}
}
}
}
Expand Down Expand Up @@ -874,7 +889,8 @@ impl<'tcx> TerminatorKind<'tcx> {
}

write!(fmt, ")")
}
},
FalseEdges { .. } => write!(fmt, "falseEdges")
}
}

Expand Down Expand Up @@ -910,7 +926,12 @@ impl<'tcx> TerminatorKind<'tcx> {
}
Assert { cleanup: None, .. } => vec!["".into()],
Assert { .. } =>
vec!["success".into_cow(), "unwind".into_cow()]
vec!["success".into_cow(), "unwind".into_cow()],
FalseEdges { ref imaginary_targets, .. } => {
let mut l = vec!["real".into()];
l.resize(imaginary_targets.len() + 1, "imaginary".into());
l
}
}
}
}
Expand Down Expand Up @@ -1878,6 +1899,8 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> {
Resume => Resume,
Return => Return,
Unreachable => Unreachable,
FalseEdges { real_target, ref imaginary_targets } =>
FalseEdges { real_target, imaginary_targets: imaginary_targets.clone() }
};
Terminator {
source_info: self.source_info,
Expand Down Expand Up @@ -1917,7 +1940,8 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> {
Resume |
Return |
GeneratorDrop |
Unreachable => false
Unreachable |
FalseEdges { .. } => false
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,8 +486,15 @@ macro_rules! make_mir_visitor {
self.visit_operand(value, source_location);
self.visit_branch(block, resume);
drop.map(|t| self.visit_branch(block, t));

}

TerminatorKind::FalseEdges { real_target, ref imaginary_targets } => {
self.visit_branch(block, real_target);
for target in imaginary_targets {
self.visit_branch(block, *target);
}
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/borrow_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,8 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> DataflowResultsConsumer<'b, 'tcx>
TerminatorKind::Resume |
TerminatorKind::Return |
TerminatorKind::GeneratorDrop |
TerminatorKind::Unreachable => {
TerminatorKind::Unreachable |
TerminatorKind::FalseEdges { .. } => {
// no data used, thus irrelevant to borrowck
}
}
Expand Down
118 changes: 83 additions & 35 deletions src/librustc_mir/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use rustc::mir::*;
use rustc::hir;
use hair::*;
use syntax::ast::{Name, NodeId};
use syntax_pos::{DUMMY_SP, Span};
use syntax_pos::Span;

// helper functions, broken out by category:
mod simplify;
Expand Down Expand Up @@ -54,29 +54,43 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
(body, scope.unwrap_or(self.visibility_scope))
}).collect();

// create binding start block for link them by false edges
let candidate_count = arms.iter().fold(0, |ac, c| ac + c.patterns.len());
let pre_binding_blocks: Vec<_> = (0..candidate_count + 1)
.map(|_| self.cfg.start_new_block()).collect();

// assemble a list of candidates: there is one candidate per
// pattern, which means there may be more than one candidate
// *per arm*. These candidates are kept sorted such that the
// highest priority candidate comes first in the list.
// (i.e. same order as in source)

let candidates: Vec<_> =
arms.iter()
.enumerate()
.flat_map(|(arm_index, arm)| {
arm.patterns.iter()
.map(move |pat| (arm_index, pat, arm.guard.clone()))
})
.map(|(arm_index, pattern, guard)| {
.zip(pre_binding_blocks.iter().zip(pre_binding_blocks.iter().skip(1)))
.map(|((arm_index, pattern, guard),
(pre_binding_block, next_candidate_pre_binding_block))| {
Candidate {
span: pattern.span,
match_pairs: vec![MatchPair::new(discriminant_lvalue.clone(), pattern)],
bindings: vec![],
guard,
arm_index,
pre_binding_block: *pre_binding_block,
next_candidate_pre_binding_block: *next_candidate_pre_binding_block,
}
})
.collect();

let outer_source_info = self.source_info(span);
self.cfg.terminate(*pre_binding_blocks.last().unwrap(),
outer_source_info, TerminatorKind::Unreachable);

// this will generate code to test discriminant_lvalue and
// branch to the appropriate arm block
let otherwise = self.match_candidates(span, &mut arm_blocks, candidates, block);
Expand Down Expand Up @@ -148,7 +162,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
match_pairs: vec![MatchPair::new(initializer.clone(), &irrefutable_pat)],
bindings: vec![],
guard: None,
arm_index: 0, // since we don't call `match_candidates`, this field is unused

// since we don't call `match_candidates`, next fields is unused
arm_index: 0,
pre_binding_block: block,
next_candidate_pre_binding_block: block
};

// Simplify the candidate. Since the pattern is irrefutable, this should
Expand Down Expand Up @@ -278,6 +296,10 @@ pub struct Candidate<'pat, 'tcx:'pat> {

// ...and then we branch to arm with this index.
arm_index: usize,

// ...and the blocks for add false edges between candidates
pre_binding_block: BasicBlock,
next_candidate_pre_binding_block: BasicBlock,
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -398,17 +420,43 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
candidates.iter().take_while(|c| c.match_pairs.is_empty()).count();
debug!("match_candidates: {:?} candidates fully matched", fully_matched);
let mut unmatched_candidates = candidates.split_off(fully_matched);
for (index, candidate) in candidates.into_iter().enumerate() {

let fully_matched_with_guard =
candidates.iter().take_while(|c| c.guard.is_some()).count();

let unreachable_candidates = if fully_matched_with_guard + 1 < candidates.len() {
candidates.split_off(fully_matched_with_guard + 1)
} else {
vec![]
};

for candidate in candidates {
// If so, apply any bindings, test the guard (if any), and
// branch to the arm.
let is_last = index == fully_matched - 1;
if let Some(b) = self.bind_and_guard_matched_candidate(block, arm_blocks,
candidate, is_last) {
if let Some(b) = self.bind_and_guard_matched_candidate(block, arm_blocks, candidate) {
block = b;
} else {
// if None is returned, then any remaining candidates
// are unreachable (at least not through this path).
return vec![];
// Link them with false edges.
debug!("match_candidates: add false edges for unreachable {:?} and unmatched {:?}",
unreachable_candidates, unmatched_candidates);
for candidate in unreachable_candidates {
let source_info = self.source_info(candidate.span);
let target = self.cfg.start_new_block();
if let Some(otherwise) = self.bind_and_guard_matched_candidate(target,
arm_blocks,
candidate) {
self.cfg.terminate(otherwise, source_info, TerminatorKind::Unreachable);
}
}

if unmatched_candidates.is_empty() {
return vec![]
} else {
let target = self.cfg.start_new_block();
return self.match_candidates(span, arm_blocks, unmatched_candidates, target);
}
}
}

Expand All @@ -423,9 +471,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
self.test_candidates(span, arm_blocks, &unmatched_candidates, block);

// If the target candidates were exhaustive, then we are done.
if otherwise.is_empty() {
return vec![];
}
// But for borrowck continue build decision tree.

// If all candidates were sorted into `target_candidates` somewhere, then
// the initial set was inexhaustive.
Expand Down Expand Up @@ -666,48 +712,50 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
fn bind_and_guard_matched_candidate<'pat>(&mut self,
mut block: BasicBlock,
arm_blocks: &mut ArmBlocks,
candidate: Candidate<'pat, 'tcx>,
is_last_arm: bool)
candidate: Candidate<'pat, 'tcx>)
-> Option<BasicBlock> {
debug!("bind_and_guard_matched_candidate(block={:?}, candidate={:?})",
block, candidate);

debug_assert!(candidate.match_pairs.is_empty());

self.bind_matched_candidate(block, candidate.bindings);

let arm_block = arm_blocks.blocks[candidate.arm_index];
let candidate_source_info = self.source_info(candidate.span);

self.cfg.terminate(block, candidate_source_info,
TerminatorKind::Goto { target: candidate.pre_binding_block });

block = self.cfg.start_new_block();
self.cfg.terminate(candidate.pre_binding_block, candidate_source_info,
TerminatorKind::FalseEdges {
real_target: block,
imaginary_targets:
vec![candidate.next_candidate_pre_binding_block]});

self.bind_matched_candidate(block, candidate.bindings);

if let Some(guard) = candidate.guard {
// the block to branch to if the guard fails; if there is no
// guard, this block is simply unreachable
let guard = self.hir.mirror(guard);
let source_info = self.source_info(guard.span);
let cond = unpack!(block = self.as_local_operand(block, guard));
let otherwise = self.cfg.start_new_block();

let false_edge_block = self.cfg.start_new_block();
self.cfg.terminate(block, source_info,
TerminatorKind::if_(self.hir.tcx(), cond, arm_block, otherwise));
Some(otherwise)
} else if !is_last_arm {
// Add always true guard in case of more than one arm
// it creates false edges and allow MIR borrowck detects errors
// FIXME(#45184) -- permit "false edges"
let source_info = self.source_info(candidate.span);
let true_expr = Expr {
temp_lifetime: None,
ty: self.hir.tcx().types.bool,
span: DUMMY_SP,
kind: ExprKind::Literal{literal: self.hir.true_literal()},
};
let cond = unpack!(block = self.as_local_operand(block, true_expr));
TerminatorKind::if_(self.hir.tcx(), cond, arm_block,
false_edge_block));

let otherwise = self.cfg.start_new_block();
self.cfg.terminate(block, source_info,
TerminatorKind::if_(self.hir.tcx(), cond, arm_block, otherwise));
self.cfg.terminate(false_edge_block, source_info,
TerminatorKind::FalseEdges {
real_target: otherwise,
imaginary_targets:
vec![candidate.next_candidate_pre_binding_block] });
Some(otherwise)
} else {
let source_info = self.source_info(candidate.span);
self.cfg.terminate(block, source_info,
TerminatorKind::Goto { target: arm_block });
self.cfg.terminate(block, candidate_source_info,
TerminatorKind::Goto { target: arm_block });
None
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_mir/build/matches/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
bindings: candidate.bindings.clone(),
guard: candidate.guard.clone(),
arm_index: candidate.arm_index,
pre_binding_block: candidate.pre_binding_block,
next_candidate_pre_binding_block: candidate.next_candidate_pre_binding_block,
}
}

Expand Down Expand Up @@ -659,6 +661,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
bindings: candidate.bindings.clone(),
guard: candidate.guard.clone(),
arm_index: candidate.arm_index,
pre_binding_block: candidate.pre_binding_block,
next_candidate_pre_binding_block: candidate.next_candidate_pre_binding_block,
}
}

Expand Down
6 changes: 6 additions & 0 deletions src/librustc_mir/dataflow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,12 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation
self.propagate_bits_into_entry_set_for(in_out, changed, dest_bb);
}
}
mir::TerminatorKind::FalseEdges { ref real_target, ref imaginary_targets } => {
self.propagate_bits_into_entry_set_for(in_out, changed, real_target);
for target in imaginary_targets {
self.propagate_bits_into_entry_set_for(in_out, changed, target);
}
}
}
}

Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/dataflow/move_paths/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> {
TerminatorKind::Goto { target: _ } |
TerminatorKind::Resume |
TerminatorKind::GeneratorDrop |
TerminatorKind::FalseEdges { .. } |
TerminatorKind::Unreachable => { }

TerminatorKind::Return => {
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/transform/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
TerminatorKind::GeneratorDrop |
TerminatorKind::Resume |
TerminatorKind::Return |
TerminatorKind::Unreachable => {
TerminatorKind::Unreachable |
TerminatorKind::FalseEdges { .. } => {
// safe (at least as emitted during MIR construction)
}

Expand Down
Loading

0 comments on commit 95a4016

Please sign in to comment.