1
Fork 0

Replace a magic boolean with enum DeclareLetBindings

The new enum `DeclareLetBindings` has three variants:
- `Yes`: Declare `let` bindings as normal, for `if` conditions.
- `No`: Don't declare bindings, for match guards and let-else.
- `LetNotPermitted`: Assert that `let` expressions should not occur.
This commit is contained in:
Zalathar 2024-06-26 11:34:31 +10:00
parent 716752ebe6
commit ad575b093b
3 changed files with 64 additions and 17 deletions

View file

@ -1,3 +1,4 @@
use crate::build::matches::DeclareLetBindings;
use crate::build::ForGuard::OutsideGuard; use crate::build::ForGuard::OutsideGuard;
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder}; use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
use rustc_middle::middle::region::Scope; use rustc_middle::middle::region::Scope;
@ -213,7 +214,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pattern, pattern,
None, None,
initializer_span, initializer_span,
false, DeclareLetBindings::No,
true, true,
) )
}); });

View file

@ -1,6 +1,7 @@
//! See docs in build/expr/mod.rs //! See docs in build/expr/mod.rs
use crate::build::expr::category::{Category, RvalueFunc}; use crate::build::expr::category::{Category, RvalueFunc};
use crate::build::matches::DeclareLetBindings;
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder, NeedsTemporary}; use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder, NeedsTemporary};
use rustc_ast::InlineAsmOptions; use rustc_ast::InlineAsmOptions;
use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::fx::FxHashMap;
@ -86,7 +87,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
cond, cond,
Some(condition_scope), // Temp scope Some(condition_scope), // Temp scope
source_info, source_info,
true, // Declare `let` bindings normally DeclareLetBindings::Yes, // Declare `let` bindings normally
)); ));
// Lower the `then` arm into its block. // Lower the `then` arm into its block.
@ -163,7 +164,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
source_info, source_info,
// This flag controls how inner `let` expressions are lowered, // This flag controls how inner `let` expressions are lowered,
// but either way there shouldn't be any of those in here. // but either way there shouldn't be any of those in here.
true, DeclareLetBindings::LetNotPermitted,
) )
}); });
let (short_circuit, continuation, constant) = match op { let (short_circuit, continuation, constant) = match op {

View file

@ -39,9 +39,27 @@ struct ThenElseArgs {
/// `self.local_scope()` is used. /// `self.local_scope()` is used.
temp_scope_override: Option<region::Scope>, temp_scope_override: Option<region::Scope>,
variable_source_info: SourceInfo, variable_source_info: SourceInfo,
/// Determines how bindings should be handled when lowering `let` expressions.
///
/// Forwarded to [`Builder::lower_let_expr`] when lowering [`ExprKind::Let`]. /// Forwarded to [`Builder::lower_let_expr`] when lowering [`ExprKind::Let`].
/// When false (for match guards), `let` bindings won't be declared. declare_let_bindings: DeclareLetBindings,
declare_let_bindings: bool, }
/// Should lowering a `let` expression also declare its bindings?
///
/// Used by [`Builder::lower_let_expr`] when lowering [`ExprKind::Let`].
#[derive(Clone, Copy)]
pub(crate) enum DeclareLetBindings {
/// Yes, declare `let` bindings as normal for `if` conditions.
Yes,
/// No, don't declare `let` bindings, because the caller declares them
/// separately due to special requirements.
///
/// Used for match guards and let-else.
No,
/// Let expressions are not permitted in this context, so it is a bug to
/// try to lower one (e.g inside lazy-boolean-or or boolean-not).
LetNotPermitted,
} }
impl<'a, 'tcx> Builder<'a, 'tcx> { impl<'a, 'tcx> Builder<'a, 'tcx> {
@ -57,7 +75,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
expr_id: ExprId, expr_id: ExprId,
temp_scope_override: Option<region::Scope>, temp_scope_override: Option<region::Scope>,
variable_source_info: SourceInfo, variable_source_info: SourceInfo,
declare_let_bindings: bool, declare_let_bindings: DeclareLetBindings,
) -> BlockAnd<()> { ) -> BlockAnd<()> {
self.then_else_break_inner( self.then_else_break_inner(
block, block,
@ -91,13 +109,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
this.then_else_break_inner( this.then_else_break_inner(
block, block,
lhs, lhs,
ThenElseArgs { declare_let_bindings: true, ..args }, ThenElseArgs {
declare_let_bindings: DeclareLetBindings::LetNotPermitted,
..args
},
) )
}); });
let rhs_success_block = unpack!(this.then_else_break_inner( let rhs_success_block = unpack!(this.then_else_break_inner(
failure_block, failure_block,
rhs, rhs,
ThenElseArgs { declare_let_bindings: true, ..args }, ThenElseArgs {
declare_let_bindings: DeclareLetBindings::LetNotPermitted,
..args
},
)); ));
// Make the LHS and RHS success arms converge to a common block. // Make the LHS and RHS success arms converge to a common block.
@ -127,7 +151,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
this.then_else_break_inner( this.then_else_break_inner(
block, block,
arg, arg,
ThenElseArgs { declare_let_bindings: true, ..args }, ThenElseArgs {
declare_let_bindings: DeclareLetBindings::LetNotPermitted,
..args
},
) )
}); });
this.break_for_else(success_block, args.variable_source_info); this.break_for_else(success_block, args.variable_source_info);
@ -1991,8 +2018,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// Pat binding - used for `let` and function parameters as well. // Pat binding - used for `let` and function parameters as well.
impl<'a, 'tcx> Builder<'a, 'tcx> { impl<'a, 'tcx> Builder<'a, 'tcx> {
/// If the bindings have already been declared, set `declare_bindings` to /// Lowers a `let` expression that appears in a suitable context
/// `false` to avoid duplicated bindings declaration; used for if-let guards. /// (e.g. an `if` condition or match guard).
///
/// Also used for lowering let-else statements, since they have similar
/// needs despite not actually using `let` expressions.
///
/// Use [`DeclareLetBindings`] to control whether the `let` bindings are
/// declared or not.
pub(crate) fn lower_let_expr( pub(crate) fn lower_let_expr(
&mut self, &mut self,
mut block: BasicBlock, mut block: BasicBlock,
@ -2000,7 +2033,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pat: &Pat<'tcx>, pat: &Pat<'tcx>,
source_scope: Option<SourceScope>, source_scope: Option<SourceScope>,
scope_span: Span, scope_span: Span,
declare_bindings: bool, declare_let_bindings: DeclareLetBindings,
storages_alive: bool, storages_alive: bool,
) -> BlockAnd<()> { ) -> BlockAnd<()> {
let expr_span = self.thir[expr_id].span; let expr_span = self.thir[expr_id].span;
@ -2017,10 +2050,22 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.break_for_else(otherwise_block, self.source_info(expr_span)); self.break_for_else(otherwise_block, self.source_info(expr_span));
if declare_bindings { match declare_let_bindings {
let expr_place = scrutinee.try_to_place(self); DeclareLetBindings::Yes => {
let opt_expr_place = expr_place.as_ref().map(|place| (Some(place), expr_span)); let expr_place = scrutinee.try_to_place(self);
self.declare_bindings(source_scope, pat.span.to(scope_span), pat, None, opt_expr_place); let opt_expr_place = expr_place.as_ref().map(|place| (Some(place), expr_span));
self.declare_bindings(
source_scope,
pat.span.to(scope_span),
pat,
None,
opt_expr_place,
);
}
DeclareLetBindings::No => {} // Caller is responsible for bindings.
DeclareLetBindings::LetNotPermitted => {
self.tcx.dcx().span_bug(expr_span, "let expression not expected in this context")
}
} }
let success = self.bind_pattern( let success = self.bind_pattern(
@ -2203,7 +2248,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
guard, guard,
None, // Use `self.local_scope()` as the temp scope None, // Use `self.local_scope()` as the temp scope
this.source_info(arm.span), this.source_info(arm.span),
false, // For guards, `let` bindings are declared separately DeclareLetBindings::No, // For guards, `let` bindings are declared separately
) )
}); });