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

add TerminatorKind::FalseEdges and use it in matches #45384

Merged
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
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,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't like it, but add Option and unwraps it's worse, may be create sub struct for simplify

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,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

may be we don't need it

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't - we only need 1 false edge.

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