Skip to content

Commit 407cb24

Browse files
matthewjaspermatthewjasper
committed
Remove hir::Guard
Use Expr instead. Use `ExprKind::Let` to represent if let guards.
1 parent 5113ed2 commit 407cb24

File tree

17 files changed

+77
-109
lines changed

17 files changed

+77
-109
lines changed

‎compiler/rustc_ast_lowering/src/expr.rs

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -546,20 +546,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
546546

547547
fn lower_arm(&mut self, arm: &Arm) -> hir::Arm<'hir> {
548548
let pat = self.lower_pat(&arm.pat);
549-
let guard = arm.guard.as_ref().map(|cond| {
550-
if let ExprKind::Let(pat, scrutinee, span, is_recovered) = &cond.kind {
551-
hir::Guard::IfLet(self.arena.alloc(hir::Let {
552-
hir_id: self.next_id(),
553-
span: self.lower_span(*span),
554-
pat: self.lower_pat(pat),
555-
ty: None,
556-
init: self.lower_expr(scrutinee),
557-
is_recovered: *is_recovered,
558-
}))
559-
} else {
560-
hir::Guard::If(self.lower_expr(cond))
561-
}
562-
});
549+
let guard = arm.guard.as_ref().map(|cond| self.lower_expr(cond));
563550
let hir_id = self.next_id();
564551
let span = self.lower_span(arm.span);
565552
self.lower_attrs(hir_id, &arm.attrs);

‎compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3590,7 +3590,7 @@ impl<'b, 'v> Visitor<'v> for ConditionVisitor<'b> {
35903590
));
35913591
} else if let Some(guard) = &arm.guard {
35923592
self.errors.push((
3593-
arm.pat.span.to(guard.body().span),
3593+
arm.pat.span.to(guard.span),
35943594
format!(
35953595
"if this pattern and condition are matched, {} is not \
35963596
initialized",

‎compiler/rustc_hir/src/hir.rs

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1258,7 +1258,7 @@ pub struct Arm<'hir> {
12581258
/// If this pattern and the optional guard matches, then `body` is evaluated.
12591259
pub pat: &'hir Pat<'hir>,
12601260
/// Optional guard clause.
1261-
pub guard: Option<Guard<'hir>>,
1261+
pub guard: Option<&'hir Expr<'hir>>,
12621262
/// The expression the arm evaluates to if this arm matches.
12631263
pub body: &'hir Expr<'hir>,
12641264
}
@@ -1280,26 +1280,6 @@ pub struct Let<'hir> {
12801280
pub is_recovered: Option<ErrorGuaranteed>,
12811281
}
12821282

1283-
#[derive(Debug, Clone, Copy, HashStable_Generic)]
1284-
pub enum Guard<'hir> {
1285-
If(&'hir Expr<'hir>),
1286-
IfLet(&'hir Let<'hir>),
1287-
}
1288-
1289-
impl<'hir> Guard<'hir> {
1290-
/// Returns the body of the guard
1291-
///
1292-
/// In other words, returns the e in either of the following:
1293-
///
1294-
/// - `if e`
1295-
/// - `if let x = e`
1296-
pub fn body(&self) -> &'hir Expr<'hir> {
1297-
match self {
1298-
Guard::If(e) | Guard::IfLet(Let { init: e, .. }) => e,
1299-
}
1300-
}
1301-
}
1302-
13031283
#[derive(Debug, Clone, Copy, HashStable_Generic)]
13041284
pub struct ExprField<'hir> {
13051285
#[stable_hasher(ignore)]

‎compiler/rustc_hir/src/intravisit.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -619,13 +619,8 @@ pub fn walk_stmt<'v, V: Visitor<'v>>(visitor: &mut V, statement: &'v Stmt<'v>) {
619619
pub fn walk_arm<'v, V: Visitor<'v>>(visitor: &mut V, arm: &'v Arm<'v>) {
620620
visitor.visit_id(arm.hir_id);
621621
visitor.visit_pat(arm.pat);
622-
if let Some(ref g) = arm.guard {
623-
match g {
624-
Guard::If(ref e) => visitor.visit_expr(e),
625-
Guard::IfLet(ref l) => {
626-
visitor.visit_let_expr(l);
627-
}
628-
}
622+
if let Some(ref e) = arm.guard {
623+
visitor.visit_expr(e);
629624
}
630625
visitor.visit_expr(arm.body);
631626
}

‎compiler/rustc_hir_analysis/src/check/region.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ fn resolve_arm<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, arm: &'tcx hir
184184
visitor.enter_node_scope_with_dtor(arm.hir_id.local_id);
185185
visitor.cx.var_parent = visitor.cx.parent;
186186

187-
if let Some(hir::Guard::If(expr)) = arm.guard {
187+
if let Some(expr) = arm.guard {
188+
// Check for if??
188189
visitor.terminating_scopes.insert(expr.hir_id.local_id);
189190
}
190191

‎compiler/rustc_hir_pretty/src/lib.rs

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1874,17 +1874,9 @@ impl<'a> State<'a> {
18741874
self.print_pat(arm.pat);
18751875
self.space();
18761876
if let Some(ref g) = arm.guard {
1877-
match *g {
1878-
hir::Guard::If(e) => {
1879-
self.word_space("if");
1880-
self.print_expr(e);
1881-
self.space();
1882-
}
1883-
hir::Guard::IfLet(&hir::Let { pat, ty, init, .. }) => {
1884-
self.word_nbsp("if");
1885-
self.print_let(pat, ty, init);
1886-
}
1887-
}
1877+
self.word_space("if");
1878+
self.print_expr(g);
1879+
self.space();
18881880
}
18891881
self.word_space("=>");
18901882

‎compiler/rustc_hir_typeck/src/_match.rs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,16 +78,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
7878
let mut other_arms = vec![]; // Used only for diagnostics.
7979
let mut prior_arm = None;
8080
for arm in arms {
81-
if let Some(g) = &arm.guard {
81+
if let Some(e) = &arm.guard {
8282
self.diverges.set(Diverges::Maybe);
83-
match g {
84-
hir::Guard::If(e) => {
85-
self.check_expr_has_type_or_error(e, tcx.types.bool, |_| {});
86-
}
87-
hir::Guard::IfLet(l) => {
88-
self.check_expr_let(l);
89-
}
90-
};
83+
self.check_expr_has_type_or_error(e, tcx.types.bool, |_| {});
9184
}
9285

9386
self.diverges.set(Diverges::Maybe);

‎compiler/rustc_hir_typeck/src/expr_use_visitor.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -669,12 +669,8 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
669669
);
670670
self.walk_pat(discr_place, arm.pat, arm.guard.is_some());
671671

672-
match arm.guard {
673-
Some(hir::Guard::If(e)) => self.consume_expr(e),
674-
Some(hir::Guard::IfLet(l)) => {
675-
self.walk_local(l.init, l.pat, None, |t| t.borrow_expr(l.init, ty::ImmBorrow))
676-
}
677-
None => {}
672+
if let Some(ref e) = arm.guard {
673+
self.consume_expr(e)
678674
}
679675

680676
self.consume_expr(arm.body);

‎compiler/rustc_mir_build/src/build/expr/into.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
8282
cond,
8383
Some(condition_scope),
8484
condition_scope,
85-
source_info
85+
source_info,
86+
true,
8687
));
8788

8889
this.expr_into_dest(destination, then_blk, then)
@@ -173,6 +174,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
173174
Some(condition_scope),
174175
condition_scope,
175176
source_info,
177+
true,
176178
)
177179
});
178180
let (short_circuit, continuation, constant) = match op {

‎compiler/rustc_mir_build/src/build/matches/mod.rs

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
4040
temp_scope_override: Option<region::Scope>,
4141
break_scope: region::Scope,
4242
variable_source_info: SourceInfo,
43+
declare_bindings: bool,
4344
) -> BlockAnd<()> {
4445
let this = self;
4546
let expr = &this.thir[expr_id];
@@ -53,6 +54,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
5354
temp_scope_override,
5455
break_scope,
5556
variable_source_info,
57+
declare_bindings,
5658
));
5759

5860
let rhs_then_block = unpack!(this.then_else_break(
@@ -61,6 +63,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
6163
temp_scope_override,
6264
break_scope,
6365
variable_source_info,
66+
declare_bindings,
6467
));
6568

6669
rhs_then_block.unit()
@@ -75,6 +78,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
7578
temp_scope_override,
7679
local_scope,
7780
variable_source_info,
81+
true,
7882
)
7983
});
8084
let rhs_success_block = unpack!(this.then_else_break(
@@ -83,6 +87,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
8387
temp_scope_override,
8488
break_scope,
8589
variable_source_info,
90+
true,
8691
));
8792
this.cfg.goto(lhs_success_block, variable_source_info, rhs_success_block);
8893
rhs_success_block.unit()
@@ -102,6 +107,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
102107
temp_scope_override,
103108
local_scope,
104109
variable_source_info,
110+
true,
105111
)
106112
});
107113
this.break_for_else(success_block, break_scope, variable_source_info);
@@ -116,6 +122,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
116122
temp_scope_override,
117123
break_scope,
118124
variable_source_info,
125+
declare_bindings,
119126
)
120127
})
121128
}
@@ -125,6 +132,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
125132
temp_scope_override,
126133
break_scope,
127134
variable_source_info,
135+
declare_bindings,
128136
),
129137
ExprKind::Let { expr, ref pat } => this.lower_let_expr(
130138
block,
@@ -133,7 +141,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
133141
break_scope,
134142
Some(variable_source_info.scope),
135143
variable_source_info.span,
136-
true,
144+
declare_bindings,
137145
),
138146
_ => {
139147
let temp_scope = temp_scope_override.unwrap_or_else(|| this.local_scope());
@@ -737,13 +745,40 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
737745
);
738746
},
739747
);
740-
if let Some(Guard::IfLet(guard_pat, _)) = guard {
741-
// FIXME: pass a proper `opt_match_place`
742-
self.declare_bindings(visibility_scope, scope_span, guard_pat, None, None);
748+
if let Some(&Guard::If(guard_expr)) = guard {
749+
self.declare_guard_bindings(guard_expr, scope_span, visibility_scope);
743750
}
744751
visibility_scope
745752
}
746753

754+
/// Declare bindings in a guard. This has to be done when declaring bindings
755+
/// for an arm to ensure that or patterns only have one version of each
756+
/// variable.
757+
pub(crate) fn declare_guard_bindings(
758+
&mut self,
759+
guard_expr: ExprId,
760+
scope_span: Span,
761+
visibility_scope: Option<SourceScope>,
762+
) {
763+
match self.thir.exprs[guard_expr].kind {
764+
ExprKind::Let { expr: _, pat: ref guard_pat } => {
765+
// FIXME: pass a proper `opt_match_place`
766+
self.declare_bindings(visibility_scope, scope_span, guard_pat, None, None);
767+
}
768+
ExprKind::Scope { value, .. } => {
769+
self.declare_guard_bindings(value, scope_span, visibility_scope);
770+
}
771+
ExprKind::Use { source } => {
772+
self.declare_guard_bindings(source, scope_span, visibility_scope);
773+
}
774+
ExprKind::LogicalOp { op: LogicalOp::And, lhs, rhs } => {
775+
self.declare_guard_bindings(lhs, scope_span, visibility_scope);
776+
self.declare_guard_bindings(rhs, scope_span, visibility_scope);
777+
}
778+
_ => {}
779+
}
780+
}
781+
747782
pub(crate) fn storage_live_binding(
748783
&mut self,
749784
block: BasicBlock,
@@ -2043,6 +2078,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
20432078
None,
20442079
match_scope,
20452080
this.source_info(arm.span),
2081+
false,
20462082
)
20472083
}
20482084
Guard::IfLet(ref pat, s) => {

‎compiler/rustc_mir_build/src/thir/cx/expr.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -855,13 +855,8 @@ impl<'tcx> Cx<'tcx> {
855855

856856
fn convert_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) -> ArmId {
857857
let arm = Arm {
858-
pattern: self.pattern_from_hir(arm.pat),
859-
guard: arm.guard.as_ref().map(|g| match g {
860-
hir::Guard::If(e) => Guard::If(self.mirror_expr(e)),
861-
hir::Guard::IfLet(l) => {
862-
Guard::IfLet(self.pattern_from_hir(l.pat), self.mirror_expr(l.init))
863-
}
864-
}),
858+
pattern: self.pattern_from_hir(&arm.pat),
859+
guard: arm.guard.as_ref().map(|g| Guard::If(self.mirror_expr(g))),
865860
body: self.mirror_expr(arm.body),
866861
lint_level: LintLevel::Explicit(arm.hir_id),
867862
scope: region::Scope { id: arm.hir_id.local_id, data: region::ScopeData::Node },

‎compiler/rustc_passes/src/liveness.rs

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -351,10 +351,7 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> {
351351
}
352352

353353
fn visit_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) {
354-
self.add_from_pat(arm.pat);
355-
if let Some(hir::Guard::IfLet(let_expr)) = arm.guard {
356-
self.add_from_pat(let_expr.pat);
357-
}
354+
self.add_from_pat(&arm.pat);
358355
intravisit::walk_arm(self, arm);
359356
}
360357

@@ -921,14 +918,11 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
921918
for arm in arms {
922919
let body_succ = self.propagate_through_expr(arm.body, succ);
923920

924-
let guard_succ = arm.guard.as_ref().map_or(body_succ, |g| match g {
925-
hir::Guard::If(e) => self.propagate_through_expr(e, body_succ),
926-
hir::Guard::IfLet(let_expr) => {
927-
let let_bind = self.define_bindings_in_pat(let_expr.pat, body_succ);
928-
self.propagate_through_expr(let_expr.init, let_bind)
929-
}
930-
});
931-
let arm_succ = self.define_bindings_in_pat(arm.pat, guard_succ);
921+
let guard_succ = arm
922+
.guard
923+
.as_ref()
924+
.map_or(body_succ, |g| self.propagate_through_expr(g, body_succ));
925+
let arm_succ = self.define_bindings_in_pat(&arm.pat, guard_succ);
932926
self.merge_from_succ(ln, arm_succ);
933927
}
934928
self.propagate_through_expr(e, ln)
@@ -1328,9 +1322,6 @@ impl<'a, 'tcx> Visitor<'tcx> for Liveness<'a, 'tcx> {
13281322

13291323
fn visit_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) {
13301324
self.check_unused_vars_in_pat(arm.pat, None, None, |_, _, _, _| {});
1331-
if let Some(hir::Guard::IfLet(let_expr)) = arm.guard {
1332-
self.check_unused_vars_in_pat(let_expr.pat, None, None, |_, _, _, _| {});
1333-
}
13341325
intravisit::walk_arm(self, arm);
13351326
}
13361327
}

‎tests/ui/closures/2229_closure_analysis/issue-88118-2.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
warning: irrefutable `if let` guard pattern
2-
--> $DIR/issue-88118-2.rs:10:29
2+
--> $DIR/issue-88118-2.rs:10:25
33
|
44
LL | Registry if let _ = registry.try_find_description() => { }
5-
| ^
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: this pattern will always match, so the guard is useless
88
= help: consider removing the guard and adding a `let` inside the match arm

‎tests/ui/lint/lint-match-arms-2.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ LL | #[deny(bindings_with_variant_name)]
1111
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
1212

1313
error: irrefutable `if let` guard pattern
14-
--> $DIR/lint-match-arms-2.rs:18:18
14+
--> $DIR/lint-match-arms-2.rs:18:14
1515
|
1616
LL | a if let b = a => {}
17-
| ^
17+
| ^^^^^^^^^
1818
|
1919
= note: this pattern will always match, so the guard is useless
2020
= help: consider removing the guard and adding a `let` inside the match arm

‎tests/ui/pattern/usefulness/deny-irrefutable-let-patterns.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ LL | while let _ = 5 {
2222
= help: consider instead using a `loop { ... }` with a `let` inside it
2323

2424
error: irrefutable `if let` guard pattern
25-
--> $DIR/deny-irrefutable-let-patterns.rs:13:18
25+
--> $DIR/deny-irrefutable-let-patterns.rs:13:14
2626
|
2727
LL | _ if let _ = 2 => {}
28-
| ^
28+
| ^^^^^^^^^
2929
|
3030
= note: this pattern will always match, so the guard is useless
3131
= help: consider removing the guard and adding a `let` inside the match arm

‎tests/ui/rfcs/rfc-2294-if-let-guard/warns.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: irrefutable `if let` guard pattern
2-
--> $DIR/warns.rs:6:24
2+
--> $DIR/warns.rs:6:20
33
|
44
LL | Some(x) if let () = x => {}
5-
| ^^
5+
| ^^^^^^^^^^
66
|
77
= note: this pattern will always match, so the guard is useless
88
= help: consider removing the guard and adding a `let` inside the match arm

0 commit comments

Comments
 (0)