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 match arm scopes and other scope fixes #60174

Merged
merged 11 commits into from
May 23, 2019
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);
Centril marked this conversation as resolved.
Show resolved Hide resolved

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 @@ -1314,13 +1314,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 @@ -5024,9 +5026,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 @@ -852,6 +852,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_size!(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