bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #119554 - matthewjasper:remove-guard-distinction, r=compiler-errors

Browse source 
Fix scoping for let chains in match guards

If let guards were previously represented as a different type of guard in HIR and THIR. This meant that let chains in match guards were not handled correctly because they were treated exactly like normal guards.

- Remove `hir::Guard` and `thir::Guard`.
- Make the scoping different between normal guards and if let guards also check for let chains.

closes #118593
This commit is contained in:
Matthias Krüger 2024-01-05 20:39:52 +01:00 • committed by GitHub
commit 958417fba1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
34 changed files with 272 additions and 292 deletions
Download patch file Download diff file Expand all files Collapse all files

4
compiler/rustc_mir_build/src/build/expr/into.rs
View file

@ -82,7 +82,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
cond,
Some(condition_scope),
condition_scope,
source_info
source_info,
true,
));
this.expr_into_dest(destination, then_blk, then)
@ -173,6 +174,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Some(condition_scope),
condition_scope,
source_info,
true,
)
});
let (short_circuit, continuation, constant) = match op {

80
compiler/rustc_mir_build/src/build/matches/mod.rs
View file

@ -33,6 +33,12 @@ use std::borrow::Borrow;
use std::mem;
impl<'a, 'tcx> Builder<'a, 'tcx> {
/// Lowers a condition in a way that ensures that variables bound in any let
/// expressions are definitely initialized in the if body.
///
/// If `declare_bindings` is false then variables created in `let`
/// expressions will not be declared. This is for if let guards on arms with
/// an or pattern, where the guard is lowered multiple times.
pub(crate) fn then_else_break(
&mut self,
mut block: BasicBlock,
@ -40,6 +46,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
temp_scope_override: Option<region::Scope>,
break_scope: region::Scope,
variable_source_info: SourceInfo,
declare_bindings: bool,
) -> BlockAnd<()> {
let this = self;
let expr = &this.thir[expr_id];
@ -53,6 +60,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
temp_scope_override,
break_scope,
variable_source_info,
declare_bindings,
));
let rhs_then_block = unpack!(this.then_else_break(
@ -61,6 +69,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
temp_scope_override,
break_scope,
variable_source_info,
declare_bindings,
));
rhs_then_block.unit()
@ -75,6 +84,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
temp_scope_override,
local_scope,
variable_source_info,
true,
)
});
let rhs_success_block = unpack!(this.then_else_break(
@ -83,6 +93,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
temp_scope_override,
break_scope,
variable_source_info,
true,
));
this.cfg.goto(lhs_success_block, variable_source_info, rhs_success_block);
rhs_success_block.unit()
@ -102,6 +113,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
temp_scope_override,
local_scope,
variable_source_info,
true,
)
});
this.break_for_else(success_block, break_scope, variable_source_info);
@ -116,6 +128,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
temp_scope_override,
break_scope,
variable_source_info,
declare_bindings,
)
})
}
@ -125,6 +138,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
temp_scope_override,
break_scope,
variable_source_info,
declare_bindings,
),
ExprKind::Let { expr, ref pat } => this.lower_let_expr(
block,
@ -133,7 +147,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
break_scope,
Some(variable_source_info.scope),
variable_source_info.span,
true,
declare_bindings,
),
_ => {
let temp_scope = temp_scope_override.unwrap_or_else(|| this.local_scope());
@ -417,7 +431,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
None,
arm.span,
&arm.pattern,
arm.guard.as_ref(),
arm.guard,
opt_scrutinee_place,
);
@ -709,7 +723,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
mut visibility_scope: Option<SourceScope>,
scope_span: Span,
pattern: &Pat<'tcx>,
guard: Option<&Guard<'tcx>>,
guard: Option<ExprId>,
opt_match_place: Option<(Option<&Place<'tcx>>, Span)>,
) -> Option<SourceScope> {
self.visit_primary_bindings(
@ -737,13 +751,40 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
);
},
);
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);
if let Some(guard_expr) = guard {
self.declare_guard_bindings(guard_expr, scope_span, visibility_scope);
}
visibility_scope
}
/// Declare bindings in a guard. This has to be done when declaring bindings
/// for an arm to ensure that or patterns only have one version of each
/// variable.
pub(crate) fn declare_guard_bindings(
&mut self,
guard_expr: ExprId,
scope_span: Span,
visibility_scope: Option<SourceScope>,
) {
match self.thir.exprs[guard_expr].kind {
ExprKind::Let { expr: _, pat: ref guard_pat } => {
// FIXME: pass a proper `opt_match_place`
self.declare_bindings(visibility_scope, scope_span, guard_pat, None, None);
}
ExprKind::Scope { value, .. } => {
self.declare_guard_bindings(value, scope_span, visibility_scope);
}
ExprKind::Use { source } => {
self.declare_guard_bindings(source, scope_span, visibility_scope);
}
ExprKind::LogicalOp { op: LogicalOp::And, lhs, rhs } => {
self.declare_guard_bindings(lhs, scope_span, visibility_scope);
self.declare_guard_bindings(rhs, scope_span, visibility_scope);
}
_ => {}
}
}
pub(crate) fn storage_live_binding(
&mut self,
block: BasicBlock,
@ -2009,7 +2050,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// * So we eagerly create the reference for the arm and then take a
// reference to that.
if let Some((arm, match_scope)) = arm_match_scope
&& let Some(guard) = &arm.guard
&& let Some(guard) = arm.guard
{
let tcx = self.tcx;
let bindings = parent_bindings
@ -2034,21 +2075,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let mut guard_span = rustc_span::DUMMY_SP;
let (post_guard_block, otherwise_post_guard_block) =
self.in_if_then_scope(match_scope, guard_span, |this| match *guard {
Guard::If(e) => {
guard_span = this.thir[e].span;
this.then_else_break(
block,
e,
None,
match_scope,
this.source_info(arm.span),
)
}
Guard::IfLet(ref pat, s) => {
guard_span = this.thir[s].span;
this.lower_let_expr(block, s, pat, match_scope, None, arm.span, false)
}
self.in_if_then_scope(match_scope, guard_span, |this| {
guard_span = this.thir[guard].span;
this.then_else_break(
block,
guard,
None,
match_scope,
this.source_info(arm.span),
false,
)
});
let source_info = self.source_info(guard_span);

9
compiler/rustc_mir_build/src/thir/cx/expr.rs
View file

@ -855,13 +855,8 @@ impl<'tcx> Cx<'tcx> {
fn convert_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) -> ArmId {
let arm = Arm {
pattern: self.pattern_from_hir(arm.pat),
guard: arm.guard.as_ref().map(|g| match g {
hir::Guard::If(e) => Guard::If(self.mirror_expr(e)),
hir::Guard::IfLet(l) => {
Guard::IfLet(self.pattern_from_hir(l.pat), self.mirror_expr(l.init))
}
}),
pattern: self.pattern_from_hir(&arm.pat),
guard: arm.guard.as_ref().map(|g| self.mirror_expr(g)),
body: self.mirror_expr(arm.body),
lint_level: LintLevel::Explicit(arm.hir_id),
scope: region::Scope { id: arm.hir_id.local_id, data: region::ScopeData::Node },

18
compiler/rustc_mir_build/src/thir/pattern/check_match.rs
View file

@ -100,20 +100,10 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> {
#[instrument(level = "trace", skip(self))]
fn visit_arm(&mut self, arm: &'p Arm<'tcx>) {
self.with_lint_level(arm.lint_level, |this| {
match arm.guard {
Some(Guard::If(expr)) => {
this.with_let_source(LetSource::IfLetGuard, |this| {
this.visit_expr(&this.thir[expr])
});
}
Some(Guard::IfLet(ref pat, expr)) => {
this.with_let_source(LetSource::IfLetGuard, |this| {
this.check_let(pat, Some(expr), pat.span);
this.visit_pat(pat);
this.visit_expr(&this.thir[expr]);
});
}
None => {}
if let Some(expr) = arm.guard {
this.with_let_source(LetSource::IfLetGuard, |this| {
this.visit_expr(&this.thir[expr])
});
}
this.visit_pat(&arm.pattern);
this.visit_expr(&self.thir[arm.body]);

25
compiler/rustc_mir_build/src/thir/print.rs
View file

@ -593,9 +593,9 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> {
print_indented!(self, "pattern: ", depth_lvl + 1);
self.print_pat(pattern, depth_lvl + 2);
if let Some(guard) = guard {
if let Some(guard) = *guard {
print_indented!(self, "guard: ", depth_lvl + 1);
self.print_guard(guard, depth_lvl + 2);
self.print_expr(guard, depth_lvl + 2);
} else {
print_indented!(self, "guard: None", depth_lvl + 1);
}
@ -764,27 +764,6 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> {
print_indented!(self, "}", depth_lvl);
}
fn print_guard(&mut self, guard: &Guard<'tcx>, depth_lvl: usize) {
print_indented!(self, "Guard {", depth_lvl);
match guard {
Guard::If(expr_id) => {
print_indented!(self, "If (", depth_lvl + 1);
self.print_expr(*expr_id, depth_lvl + 2);
print_indented!(self, ")", depth_lvl + 1);
}
Guard::IfLet(pat, expr_id) => {
print_indented!(self, "IfLet (", depth_lvl + 1);
self.print_pat(pat, depth_lvl + 2);
print_indented!(self, ",", depth_lvl + 1);
self.print_expr(*expr_id, depth_lvl + 2);
print_indented!(self, ")", depth_lvl + 1);
}
}
print_indented!(self, "}", depth_lvl);
}
fn print_closure_expr(&mut self, expr: &ClosureExpr<'tcx>, depth_lvl: usize) {
let ClosureExpr { closure_id, args, upvars, movability, fake_reads } = expr;