Skip to content

Commit

Permalink
Auto merge of #62233 - matthewjasper:exit-arm-scopes, r=pnkfelix
Browse files Browse the repository at this point in the history
Exit arm scopes

Due to a bug in the HIR CFG construction, borrows for arm scopes were incorrectly leaking into other arms.
This PR also includes some drive-by improvements to `-Zunpretty=hir,identified` that would have been helpful while investigating this.

Closes #62107
  • Loading branch information
bors committed Jul 9, 2019
2 parents 5d8ffb5 + de5c6ec commit b8ec4c4
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 17 deletions.
13 changes: 7 additions & 6 deletions src/librustc/cfg/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,8 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
let expr_exit = self.add_ast_node(id, &[]);

// Keep track of the previous guard expressions
let mut prev_guards = Vec::new();
let mut prev_guard = None;
let match_scope = region::Scope { id, data: region::ScopeData::Node };

for arm in arms {
// Add an exit node for when we've visited all the
Expand All @@ -389,23 +390,23 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
let guard_start = self.add_dummy_node(&[pat_exit]);
// Visit the guard expression
let guard_exit = match guard {
hir::Guard::If(ref e) => self.expr(e, guard_start),
hir::Guard::If(ref e) => (&**e, self.expr(e, guard_start)),
};
// #47295: We used to have very special case code
// here for when a pair of arms are both formed
// solely from constants, and if so, not add these
// edges. But this was not actually sound without
// other constraints that we stopped enforcing at
// some point.
while let Some(prev) = prev_guards.pop() {
self.add_contained_edge(prev, guard_start);
if let Some((prev_guard, prev_index)) = prev_guard.take() {
self.add_exiting_edge(prev_guard, prev_index, match_scope, guard_start);
}

// Push the guard onto the list of previous guards
prev_guards.push(guard_exit);
prev_guard = Some(guard_exit);

// Update the exit node for the pattern
pat_exit = guard_exit;
pat_exit = guard_exit.1;
}

// Add an edge from the exit of this pattern to the
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/hir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub enum AnnNode<'a> {
SubItem(hir::HirId),
Expr(&'a hir::Expr),
Pat(&'a hir::Pat),
Arm(&'a hir::Arm),
}

pub enum Nested {
Expand Down Expand Up @@ -1821,6 +1822,7 @@ impl<'a> State<'a> {
self.s.space();
}
self.cbox(indent_unit);
self.ann.pre(self, AnnNode::Arm(arm));
self.ibox(0);
self.print_outer_attributes(&arm.attrs);
let mut first = true;
Expand Down Expand Up @@ -1865,6 +1867,7 @@ impl<'a> State<'a> {
self.s.word(",");
}
}
self.ann.post(self, AnnNode::Arm(arm));
self.end() // close enclosing cbox
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc_borrowck/dataflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ impl<'tcx, O: DataFlowOperator> pprust::PpAnn for DataFlowContext<'tcx, O> {
pprust::AnnNode::Block(blk) => blk.hir_id.local_id,
pprust::AnnNode::Item(_) |
pprust::AnnNode::SubItem(_) => return,
pprust::AnnNode::Pat(pat) => pat.hir_id.local_id
pprust::AnnNode::Pat(pat) => pat.hir_id.local_id,
pprust::AnnNode::Arm(arm) => arm.hir_id.local_id,
};

if !self.has_bitset_for_local_id(id) {
Expand Down
20 changes: 10 additions & 10 deletions src/librustc_driver/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,28 +387,28 @@ impl<'hir> pprust_hir::PpAnn for IdentifiedAnnotation<'hir> {
pprust_hir::AnnNode::Name(_) => {},
pprust_hir::AnnNode::Item(item) => {
s.s.space();
s.synth_comment(format!("hir_id: {} hir local_id: {}",
item.hir_id, item.hir_id.local_id.as_u32()))
s.synth_comment(format!("hir_id: {}", item.hir_id));
}
pprust_hir::AnnNode::SubItem(id) => {
s.s.space();
s.synth_comment(id.to_string())
s.synth_comment(id.to_string());
}
pprust_hir::AnnNode::Block(blk) => {
s.s.space();
s.synth_comment(format!("block hir_id: {} hir local_id: {}",
blk.hir_id, blk.hir_id.local_id.as_u32()))
s.synth_comment(format!("block hir_id: {}", blk.hir_id));
}
pprust_hir::AnnNode::Expr(expr) => {
s.s.space();
s.synth_comment(format!("expr hir_id: {} hir local_id: {}",
expr.hir_id, expr.hir_id.local_id.as_u32()));
s.pclose()
s.synth_comment(format!("expr hir_id: {}", expr.hir_id));
s.pclose();
}
pprust_hir::AnnNode::Pat(pat) => {
s.s.space();
s.synth_comment(format!("pat hir_id: {} hir local_id: {}",
pat.hir_id, pat.hir_id.local_id.as_u32()))
s.synth_comment(format!("pat hir_id: {}", pat.hir_id));
}
pprust_hir::AnnNode::Arm(arm) => {
s.s.space();
s.synth_comment(format!("arm hir_id: {}", arm.hir_id));
}
}
}
Expand Down
12 changes: 12 additions & 0 deletions src/test/ui/borrowck/issue-62107-match-arm-scopes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
fn main() {
let e: i32;
match e {
//~^ ERROR use of possibly uninitialized variable
ref u if true => {}
ref v if true => {
let tx = 0;
&tx;
}
_ => (),
}
}
9 changes: 9 additions & 0 deletions src/test/ui/borrowck/issue-62107-match-arm-scopes.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0381]: use of possibly uninitialized variable: `e`
--> $DIR/issue-62107-match-arm-scopes.rs:3:11
|
LL | match e {
| ^ use of possibly uninitialized `e`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0381`.

0 comments on commit b8ec4c4

Please sign in to comment.