Skip to content

Commit

Permalink
Rollup merge of rust-lang#60174 - matthewjasper:add-match-arm-scopes,…
Browse files Browse the repository at this point in the history
… r=pnkfelix

Add match arm scopes and other scope fixes

* Add drop and lint scopes for match arms.
* Lint attributes are now respected on match arms.
* Make sure we emit a StorageDead if we diverge when initializing a temporary.
* Adjust MIR pretty printing of scopes for locals.
* Don't generate duplicate lint scopes for `let statements`.
* Add some previously missing fake borrows for matches.

closes rust-lang#46525

cc @rust-lang/compiler
  • Loading branch information
Centril authored May 15, 2019
2 parents e7dd3eb + 5b5255d commit d4a5588
Show file tree
Hide file tree
Showing 41 changed files with 726 additions and 276 deletions.
10 changes: 6 additions & 4 deletions src/librustc/cfg/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
for arm in arms {
// Add an exit node for when we've visited all the
// patterns and the guard (if there is one) in the arm.
let arm_exit = self.add_dummy_node(&[]);
let bindings_exit = self.add_dummy_node(&[]);

for pat in &arm.pats {
// Visit the pattern, coming from the discriminant exit
Expand Down Expand Up @@ -453,14 +453,16 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {

// Add an edge from the exit of this pattern to the
// exit of the arm
self.add_contained_edge(pat_exit, arm_exit);
self.add_contained_edge(pat_exit, bindings_exit);
}

// Visit the body of this arm
let body_exit = self.expr(&arm.body, arm_exit);
let body_exit = self.expr(&arm.body, bindings_exit);

let arm_exit = self.add_ast_node(arm.hir_id.local_id, &[body_exit]);

// Link the body to the exit of the expression
self.add_contained_edge(body_exit, expr_exit);
self.add_contained_edge(arm_exit, expr_exit);
}

expr_exit
Expand Down
1 change: 1 addition & 0 deletions src/librustc/hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1102,6 +1102,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
}

pub fn walk_arm<'v, V: Visitor<'v>>(visitor: &mut V, arm: &'v Arm) {
visitor.visit_id(arm.hir_id);
walk_list!(visitor, visit_pat, &arm.pats);
if let Some(ref g) = arm.guard {
match g {
Expand Down
4 changes: 4 additions & 0 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1313,13 +1313,15 @@ impl<'a> LoweringContext<'a> {

fn lower_arm(&mut self, arm: &Arm) -> hir::Arm {
hir::Arm {
hir_id: self.next_id(),
attrs: self.lower_attrs(&arm.attrs),
pats: arm.pats.iter().map(|x| self.lower_pat(x)).collect(),
guard: match arm.guard {
Some(Guard::If(ref x)) => Some(hir::Guard::If(P(self.lower_expr(x)))),
_ => None,
},
body: P(self.lower_expr(&arm.body)),
span: arm.span,
}
}

Expand Down Expand Up @@ -5023,9 +5025,11 @@ impl<'a> LoweringContext<'a> {

fn arm(&mut self, pats: hir::HirVec<P<hir::Pat>>, expr: P<hir::Expr>) -> hir::Arm {
hir::Arm {
hir_id: self.next_id(),
attrs: hir_vec![],
pats,
guard: None,
span: expr.span,
body: expr,
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/librustc/hir/map/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,16 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
});
}

fn visit_arm(&mut self, arm: &'hir Arm) {
let node = Node::Arm(arm);

self.insert(arm.span, arm.hir_id, node);

self.with_parent(arm.hir_id, |this| {
intravisit::walk_arm(this, arm);
});
}

fn visit_anon_const(&mut self, constant: &'hir AnonConst) {
self.insert(DUMMY_SP, constant.hir_id, Node::AnonConst(constant));

Expand Down
7 changes: 7 additions & 0 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ impl<'hir> Map<'hir> {
Node::Pat(_) |
Node::Binding(_) |
Node::Local(_) |
Node::Arm(_) |
Node::Lifetime(_) |
Node::Visibility(_) |
Node::Block(_) |
Expand Down Expand Up @@ -1000,6 +1001,7 @@ impl<'hir> Map<'hir> {
Some(Node::Field(ref f)) => Some(&f.attrs[..]),
Some(Node::Expr(ref e)) => Some(&*e.attrs),
Some(Node::Stmt(ref s)) => Some(s.node.attrs()),
Some(Node::Arm(ref a)) => Some(&*a.attrs),
Some(Node::GenericParam(param)) => Some(&param.attrs[..]),
// Unit/tuple structs/variants take the attributes straight from
// the struct/variant definition.
Expand Down Expand Up @@ -1073,6 +1075,7 @@ impl<'hir> Map<'hir> {
Some(Node::TraitRef(tr)) => tr.path.span,
Some(Node::Binding(pat)) => pat.span,
Some(Node::Pat(pat)) => pat.span,
Some(Node::Arm(arm)) => arm.span,
Some(Node::Block(block)) => block.span,
Some(Node::Ctor(..)) => match self.find_by_hir_id(
self.get_parent_node_by_hir_id(hir_id))
Expand Down Expand Up @@ -1288,6 +1291,7 @@ impl<'a> print::State<'a> {
Node::TraitRef(a) => self.print_trait_ref(&a),
Node::Binding(a) |
Node::Pat(a) => self.print_pat(&a),
Node::Arm(a) => self.print_arm(&a),
Node::Block(a) => {
use syntax::print::pprust::PrintState;

Expand Down Expand Up @@ -1417,6 +1421,9 @@ fn hir_id_to_string(map: &Map<'_>, id: HirId, include_id: bool) -> String {
Some(Node::Pat(_)) => {
format!("pat {}{}", map.hir_to_pretty_string(id), id_str)
}
Some(Node::Arm(_)) => {
format!("arm {}{}", map.hir_to_pretty_string(id), id_str)
}
Some(Node::Block(_)) => {
format!("block {}{}", map.hir_to_pretty_string(id), id_str)
}
Expand Down
4 changes: 4 additions & 0 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,9 @@ pub struct Local {
/// `<pats> (if <guard>) => <body>`.
#[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)]
pub struct Arm {
#[stable_hasher(ignore)]
pub hir_id: HirId,
pub span: Span,
pub attrs: HirVec<Attribute>,
/// Multiple patterns can be combined with `|`
pub pats: HirVec<P<Pat>>,
Expand Down Expand Up @@ -2656,6 +2659,7 @@ pub enum Node<'hir> {
TraitRef(&'hir TraitRef),
Binding(&'hir Pat),
Pat(&'hir Pat),
Arm(&'hir Arm),
Block(&'hir Block),
Local(&'hir Local),
MacroDef(&'hir MacroDef),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/hir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1862,7 +1862,7 @@ impl<'a> State<'a> {
self.ann.post(self, AnnNode::Pat(pat))
}

fn print_arm(&mut self, arm: &hir::Arm) -> io::Result<()> {
pub fn print_arm(&mut self, arm: &hir::Arm) -> io::Result<()> {
// I have no idea why this check is necessary, but here it
// is :(
if arm.attrs.is_empty() {
Expand Down
6 changes: 6 additions & 0 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -849,6 +849,12 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for LintLevelMapBuilder<'a, 'tcx> {
})
}

fn visit_arm(&mut self, a: &'tcx hir::Arm) {
self.with_lint_attrs(a.hir_id, &a.attrs, |builder| {
intravisit::walk_arm(builder, a);
})
}

fn visit_trait_item(&mut self, trait_item: &'tcx hir::TraitItem) {
self.with_lint_attrs(trait_item.hir_id, &trait_item.attrs, |builder| {
intravisit::walk_trait_item(builder, trait_item);
Expand Down
34 changes: 21 additions & 13 deletions src/librustc/middle/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,18 +119,18 @@ impl fmt::Debug for Scope {
pub enum ScopeData {
Node,

// Scope of the call-site for a function or closure
// (outlives the arguments as well as the body).
/// Scope of the call-site for a function or closure
/// (outlives the arguments as well as the body).
CallSite,

// Scope of arguments passed to a function or closure
// (they outlive its body).
/// Scope of arguments passed to a function or closure
/// (they outlive its body).
Arguments,

// Scope of destructors for temporaries of node-id.
/// Scope of destructors for temporaries of node-id.
Destruction,

// Scope following a `let id = expr;` binding in a block.
/// Scope following a `let id = expr;` binding in a block.
Remainder(FirstStatementIndex)
}

Expand All @@ -152,11 +152,11 @@ newtype_index! {
///
/// * The subscope with `first_statement_index == 1` is scope of `c`,
/// and thus does not include EXPR_2, but covers the `...`.
pub struct FirstStatementIndex { .. }
pub struct FirstStatementIndex {
derive [HashStable]
}
}

impl_stable_hash_for!(struct crate::middle::region::FirstStatementIndex { private });

// compilation error if size of `ScopeData` is not the same as a `u32`
static_assert!(ASSERT_SCOPE_DATA: mem::size_of::<ScopeData>() == 4);

Expand Down Expand Up @@ -814,13 +814,25 @@ fn resolve_block<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, blk:
}

fn resolve_arm<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, arm: &'tcx hir::Arm) {
let prev_cx = visitor.cx;

visitor.enter_scope(
Scope {
id: arm.hir_id.local_id,
data: ScopeData::Node,
}
);
visitor.cx.var_parent = visitor.cx.parent;

visitor.terminating_scopes.insert(arm.body.hir_id.local_id);

if let Some(hir::Guard::If(ref expr)) = arm.guard {
visitor.terminating_scopes.insert(expr.hir_id.local_id);
}

intravisit::walk_arm(visitor, arm);

visitor.cx = prev_cx;
}

fn resolve_pat<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, pat: &'tcx hir::Pat) {
Expand Down Expand Up @@ -893,10 +905,6 @@ fn resolve_expr<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, expr:
terminating(body.hir_id.local_id);
}

hir::ExprKind::Match(..) => {
visitor.cx.var_parent = visitor.cx.parent;
}

hir::ExprKind::DropTemps(ref expr) => {
// `DropTemps(expr)` does not denote a conditional scope.
// Rather, we want to achieve the same behavior as `{ let _t = expr; _t }`.
Expand Down
48 changes: 28 additions & 20 deletions src/librustc_mir/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
safety_mode
} =
self.hir.mirror(ast_block);
self.in_opt_scope(opt_destruction_scope.map(|de|(de, source_info)), block, move |this| {
this.in_scope((region_scope, source_info), LintLevel::Inherited, block, move |this| {
self.in_opt_scope(opt_destruction_scope.map(|de|(de, source_info)), move |this| {
this.in_scope((region_scope, source_info), LintLevel::Inherited, move |this| {
if targeted_by_break {
// This is a `break`-able block
let exit_block = this.cfg.start_new_block();
Expand Down Expand Up @@ -83,9 +83,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
StmtKind::Expr { scope, expr } => {
this.block_context.push(BlockFrame::Statement { ignores_expr_result: true });
unpack!(block = this.in_opt_scope(
opt_destruction_scope.map(|de|(de, source_info)), block, |this| {
opt_destruction_scope.map(|de|(de, source_info)), |this| {
let si = (scope, source_info);
this.in_scope(si, LintLevel::Inherited, block, |this| {
this.in_scope(si, LintLevel::Inherited, |this| {
let expr = this.hir.mirror(expr);
this.stmt_expr(block, expr, Some(stmt_span))
})
Expand Down Expand Up @@ -113,31 +113,39 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
let remainder_span = remainder_scope.span(this.hir.tcx(),
&this.hir.region_scope_tree);

let scope;
let visibility_scope =
Some(this.new_source_scope(remainder_span, LintLevel::Inherited, None));

// Evaluate the initializer, if present.
if let Some(init) = initializer {
let initializer_span = init.span();

scope = this.declare_bindings(
None,
remainder_span,
lint_level,
&pattern,
ArmHasGuard(false),
Some((None, initializer_span)),
);
unpack!(block = this.in_opt_scope(
opt_destruction_scope.map(|de|(de, source_info)), block, |this| {
opt_destruction_scope.map(|de|(de, source_info)), |this| {
let scope = (init_scope, source_info);
this.in_scope(scope, lint_level, block, |this| {
this.in_scope(scope, lint_level, |this| {
this.declare_bindings(
visibility_scope,
remainder_span,
&pattern,
ArmHasGuard(false),
Some((None, initializer_span)),
);
this.expr_into_pattern(block, pattern, init)
})
}));
} else {
scope = this.declare_bindings(
None, remainder_span, lint_level, &pattern,
ArmHasGuard(false), None);
let scope = (init_scope, source_info);
unpack!(this.in_scope(scope, lint_level, |this| {
this.declare_bindings(
visibility_scope,
remainder_span,
&pattern,
ArmHasGuard(false),
None,
);
block.unit()
}));

debug!("ast_block_stmts: pattern={:?}", pattern);
this.visit_bindings(
Expand All @@ -149,8 +157,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
})
}

// Enter the source scope, after evaluating the initializer.
if let Some(source_scope) = scope {
// Enter the visibility scope, after evaluating the initializer.
if let Some(source_scope) = visibility_scope {
this.source_scope = source_scope;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/build/expr/as_operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
{
let source_info = this.source_info(expr.span);
let region_scope = (region_scope, source_info);
return this.in_scope(region_scope, lint_level, block, |this| {
return this.in_scope(region_scope, lint_level, |this| {
this.as_operand(block, scope, value)
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/build/expr/as_place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
region_scope,
lint_level,
value,
} => this.in_scope((region_scope, source_info), lint_level, block, |this| {
} => this.in_scope((region_scope, source_info), lint_level, |this| {
if mutability == Mutability::Not {
this.as_read_only_place(block, value)
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/build/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
value,
} => {
let region_scope = (region_scope, source_info);
this.in_scope(region_scope, lint_level, block, |this| {
this.in_scope(region_scope, lint_level, |this| {
this.as_rvalue(block, scope, value)
})
}
Expand Down
Loading

0 comments on commit d4a5588

Please sign in to comment.