Skip to content

Commit

Permalink
Only declare bindings for if-let guards once per arm
Browse files Browse the repository at this point in the history
  • Loading branch information
LeSeulArtichaut committed Nov 21, 2022
1 parent 911cbf8 commit baa59d1
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 17 deletions.
7 changes: 3 additions & 4 deletions compiler/rustc_mir_build/src/build/block.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::build::matches::ArmHasGuard;
use crate::build::ForGuard::OutsideGuard;
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
use rustc_middle::middle::region::Scope;
Expand Down Expand Up @@ -231,7 +230,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
visibility_scope,
remainder_span,
pattern,
ArmHasGuard(false),
None,
Some((None, initializer_span)),
);
this.visit_primary_bindings(
Expand Down Expand Up @@ -308,7 +307,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
visibility_scope,
remainder_span,
pattern,
ArmHasGuard(false),
None,
Some((None, initializer_span)),
);
this.expr_into_pattern(block, &pattern, init)
Expand All @@ -324,7 +323,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
visibility_scope,
remainder_span,
pattern,
ArmHasGuard(false),
None,
None,
);
block.unit()
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
ExprKind::Let { expr, ref pat } => {
let scope = this.local_scope();
let (true_block, false_block) = this.in_if_then_scope(scope, expr_span, |this| {
this.lower_let_expr(block, &this.thir[expr], pat, scope, None, expr_span)
this.lower_let_expr(block, &this.thir[expr], pat, scope, None, expr_span, true)
});

this.cfg.push_assign_constant(
Expand Down
26 changes: 15 additions & 11 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
break_scope,
Some(variable_source_info.scope),
variable_source_info.span,
true,
),
_ => {
let temp_scope = temp_scope_override.unwrap_or_else(|| this.local_scope());
Expand Down Expand Up @@ -357,7 +358,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
None,
arm.span,
&arm.pattern,
ArmHasGuard(arm.guard.is_some()),
arm.guard.as_ref(),
opt_scrutinee_place,
);

Expand Down Expand Up @@ -645,7 +646,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
mut visibility_scope: Option<SourceScope>,
scope_span: Span,
pattern: &Pat<'tcx>,
has_guard: ArmHasGuard,
guard: Option<&Guard<'tcx>>,
opt_match_place: Option<(Option<&Place<'tcx>>, Span)>,
) -> Option<SourceScope> {
self.visit_primary_bindings(
Expand All @@ -667,12 +668,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
var,
ty,
user_ty,
has_guard,
ArmHasGuard(guard.is_some()),
opt_match_place.map(|(x, y)| (x.cloned(), y)),
pattern.span,
);
},
);
if let Some(Guard::IfLet(guard_pat, _)) = guard {
// FIXME: pass a proper `opt_match_place`
self.declare_bindings(visibility_scope, scope_span, guard_pat, None, None);
}
visibility_scope
}

Expand Down Expand Up @@ -1766,6 +1771,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// Pat binding - used for `let` and function parameters as well.

impl<'a, 'tcx> Builder<'a, 'tcx> {
/// If the bindings have already been declared, set `declare_bindings` to
/// `false` to avoid duplicated bindings declaration. Used for if-let guards.
pub(crate) fn lower_let_expr(
&mut self,
mut block: BasicBlock,
Expand All @@ -1774,6 +1781,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
else_target: region::Scope,
source_scope: Option<SourceScope>,
span: Span,
declare_bindings: bool,
) -> BlockAnd<()> {
let expr_span = expr.span;
let expr_place_builder = unpack!(block = self.lower_scrutinee(block, expr, expr_span));
Expand All @@ -1797,13 +1805,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let otherwise_post_guard_block = otherwise_candidate.pre_binding_block.unwrap();
self.break_for_else(otherwise_post_guard_block, else_target, self.source_info(expr_span));

self.declare_bindings(
source_scope,
pat.span.to(span),
pat,
ArmHasGuard(false),
opt_expr_place,
);
if declare_bindings {
self.declare_bindings(source_scope, pat.span.to(span), pat, None, opt_expr_place);
}

let post_guard_block = self.bind_pattern(
self.source_info(pat.span),
Expand Down Expand Up @@ -1984,7 +1988,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Guard::IfLet(ref pat, scrutinee) => {
let s = &this.thir[scrutinee];
guard_span = s.span;
this.lower_let_expr(block, s, pat, match_scope, None, arm.span)
this.lower_let_expr(block, s, pat, match_scope, None, arm.span, false)
}
});

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -924,7 +924,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
scope,
expr.span,
&pat,
matches::ArmHasGuard(false),
None,
Some((Some(&place), span)),
);
let place_builder = PlaceBuilder::from(local);
Expand Down
7 changes: 7 additions & 0 deletions src/test/ui/rfc-2294-if-let-guard/run-pass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,11 @@ fn main() {
Some(x) if let Foo::Qux(y) = qux(x) => assert_eq!(y, 84),
_ => panic!(),
}

// issue #88015
#[allow(irrefutable_let_patterns)]
match () {
() | () if let x = 42 => assert_eq!(x, 42),
_ => panic!()
}
}

0 comments on commit baa59d1

Please sign in to comment.