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.

Sign up for GitHub

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

Give temporaries in if let guards correct scopes #119122

Merged
merged 2 commits into from
Dec 25, 2023
Merged

Give temporaries in if let guards correct scopes #119122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
68d684c
Pass THIR ExprIds in MIR building
matthewjasper Dec 15, 2023
d437a11
Give temporaries in if let guards correct scopes
matthewjasper Dec 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions compiler/rustc_hir_analysis/src/check/region.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,10 @@ fn resolve_block<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, blk: &'tcx h
fn resolve_arm<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, arm: &'tcx hir::Arm<'tcx>) {
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.hir_id.local_id);

visitor.terminating_scopes.insert(arm.body.hir_id.local_id);
visitor.enter_node_scope_with_dtor(arm.hir_id.local_id);
visitor.cx.var_parent = visitor.cx.parent;

if let Some(hir::Guard::If(expr)) = arm.guard {
visitor.terminating_scopes.insert(expr.hir_id.local_id);
Expand Down
20 changes: 9 additions & 11 deletions compiler/rustc_mir_build/src/build/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
) -> BlockAnd<()> {
let Block { region_scope, span, ref stmts, expr, targeted_by_break, safety_mode } =
self.thir[ast_block];
let expr = expr.map(|expr| &self.thir[expr]);
self.in_scope((region_scope, source_info), LintLevel::Inherited, move |this| {
if targeted_by_break {
this.in_breakable_scope(None, destination, span, |this| {
Expand Down Expand Up @@ -49,7 +48,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
mut block: BasicBlock,
span: Span,
stmts: &[StmtId],
expr: Option<&Expr<'tcx>>,
expr: Option<ExprId>,
safety_mode: BlockSafety,
region_scope: Scope,
) -> BlockAnd<()> {
Expand Down Expand Up @@ -90,7 +89,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let si = (*scope, source_info);
unpack!(
block = this.in_scope(si, LintLevel::Inherited, |this| {
this.stmt_expr(block, &this.thir[*expr], Some(*scope))
this.stmt_expr(block, *expr, Some(*scope))
})
);
}
Expand Down Expand Up @@ -205,8 +204,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let visibility_scope =
Some(this.new_source_scope(remainder_span, LintLevel::Inherited, None));

let init = &this.thir[*initializer];
let initializer_span = init.span;
let initializer_span = this.thir[*initializer].span;
let scope = (*init_scope, source_info);
let failure = unpack!(
block = this.in_scope(scope, *lint_level, |this| {
Expand All @@ -232,7 +230,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
);
this.ast_let_else(
block,
init,
*initializer,
initializer_span,
*else_block,
&last_remainder_scope,
Expand Down Expand Up @@ -276,9 +274,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Some(this.new_source_scope(remainder_span, LintLevel::Inherited, None));

// Evaluate the initializer, if present.
if let Some(init) = initializer {
let init = &this.thir[*init];
let initializer_span = init.span;
if let Some(init) = *initializer {
let initializer_span = this.thir[init].span;
let scope = (*init_scope, source_info);

unpack!(
Expand Down Expand Up @@ -334,13 +331,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// of the block, which is stored into `destination`.
let tcx = this.tcx;
let destination_ty = destination.ty(&this.local_decls, tcx).ty;
if let Some(expr) = expr {
if let Some(expr_id) = expr {
let expr = &this.thir[expr_id];
let tail_result_is_ignored =
destination_ty.is_unit() || this.block_context.currently_ignores_tail_results();
this.block_context
.push(BlockFrame::TailExpr { tail_result_is_ignored, span: expr.span });

unpack!(block = this.expr_into_dest(destination, block, expr));
unpack!(block = this.expr_into_dest(destination, block, expr_id));
let popped = this.block_context.pop();

assert!(popped.is_some_and(|bf| bf.is_tail_expr()));
Expand Down
26 changes: 13 additions & 13 deletions compiler/rustc_mir_build/src/build/expr/as_operand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pub(crate) fn as_local_operand(
&mut self,
block: BasicBlock,
expr: &Expr<'tcx>,
expr_id: ExprId,
) -> BlockAnd<Operand<'tcx>> {
let local_scope = self.local_scope();
self.as_operand(block, Some(local_scope), expr, LocalInfo::Boring, NeedsTemporary::Maybe)
self.as_operand(block, Some(local_scope), expr_id, LocalInfo::Boring, NeedsTemporary::Maybe)
}

/// Returns an operand suitable for use until the end of the current scope expression and
Expand Down Expand Up @@ -76,7 +76,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pub(crate) fn as_local_call_operand(
&mut self,
block: BasicBlock,
expr: &Expr<'tcx>,
expr: ExprId,
) -> BlockAnd<Operand<'tcx>> {
let local_scope = self.local_scope();
self.as_call_operand(block, Some(local_scope), expr)
Expand All @@ -101,17 +101,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
&mut self,
mut block: BasicBlock,
scope: Option<region::Scope>,
expr: &Expr<'tcx>,
expr_id: ExprId,
local_info: LocalInfo<'tcx>,
needs_temporary: NeedsTemporary,
) -> BlockAnd<Operand<'tcx>> {
let this = self;

let expr = &this.thir[expr_id];
if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind {
let source_info = this.source_info(expr.span);
let region_scope = (region_scope, source_info);
return this.in_scope(region_scope, lint_level, |this| {
this.as_operand(block, scope, &this.thir[value], local_info, needs_temporary)
this.as_operand(block, scope, value, local_info, needs_temporary)
});
}

Expand All @@ -126,7 +127,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block.and(Operand::Constant(Box::new(constant)))
}
Category::Constant | Category::Place | Category::Rvalue(..) => {
let operand = unpack!(block = this.as_temp(block, scope, expr, Mutability::Mut));
let operand = unpack!(block = this.as_temp(block, scope, expr_id, Mutability::Mut));
// Overwrite temp local info if we have something more interesting to record.
if !matches!(local_info, LocalInfo::Boring) {
let decl_info =
Expand All @@ -144,16 +145,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
&mut self,
mut block: BasicBlock,
scope: Option<region::Scope>,
expr: &Expr<'tcx>,
expr_id: ExprId,
) -> BlockAnd<Operand<'tcx>> {
debug!("as_call_operand(block={:?}, expr={:?})", block, expr);
let this = self;
let expr = &this.thir[expr_id];
debug!("as_call_operand(block={:?}, expr={:?})", block, expr);

if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind {
let source_info = this.source_info(expr.span);
let region_scope = (region_scope, source_info);
return this.in_scope(region_scope, lint_level, |this| {
this.as_call_operand(block, scope, &this.thir[value])
this.as_call_operand(block, scope, value)
});
}

Expand All @@ -171,9 +173,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// type, and that value is coming from the deref of a box.
if let ExprKind::Deref { arg } = expr.kind {
// Generate let tmp0 = arg0
let operand = unpack!(
block = this.as_temp(block, scope, &this.thir[arg], Mutability::Mut)
);
let operand = unpack!(block = this.as_temp(block, scope, arg, Mutability::Mut));

// Return the operand *tmp0 to be used as the call argument
let place = Place {
Expand All @@ -186,6 +186,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

this.as_operand(block, scope, expr, LocalInfo::Boring, NeedsTemporary::Maybe)
this.as_operand(block, scope, expr_id, LocalInfo::Boring, NeedsTemporary::Maybe)
}
}
57 changes: 26 additions & 31 deletions compiler/rustc_mir_build/src/build/expr/as_place.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,9 +354,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pub(crate) fn as_place(
&mut self,
mut block: BasicBlock,
expr: &Expr<'tcx>,
expr_id: ExprId,
) -> BlockAnd<Place<'tcx>> {
let place_builder = unpack!(block = self.as_place_builder(block, expr));
let place_builder = unpack!(block = self.as_place_builder(block, expr_id));
block.and(place_builder.to_place(self))
}

Expand All @@ -365,9 +365,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pub(crate) fn as_place_builder(
&mut self,
block: BasicBlock,
expr: &Expr<'tcx>,
expr_id: ExprId,
) -> BlockAnd<PlaceBuilder<'tcx>> {
self.expr_as_place(block, expr, Mutability::Mut, None)
self.expr_as_place(block, expr_id, Mutability::Mut, None)
}

/// Compile `expr`, yielding a place that we can move from etc.
Expand All @@ -378,9 +378,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pub(crate) fn as_read_only_place(
&mut self,
mut block: BasicBlock,
expr: &Expr<'tcx>,
expr_id: ExprId,
) -> BlockAnd<Place<'tcx>> {
let place_builder = unpack!(block = self.as_read_only_place_builder(block, expr));
let place_builder = unpack!(block = self.as_read_only_place_builder(block, expr_id));
block.and(place_builder.to_place(self))
}

Expand All @@ -393,18 +393,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn as_read_only_place_builder(
&mut self,
block: BasicBlock,
expr: &Expr<'tcx>,
expr_id: ExprId,
) -> BlockAnd<PlaceBuilder<'tcx>> {
self.expr_as_place(block, expr, Mutability::Not, None)
self.expr_as_place(block, expr_id, Mutability::Not, None)
}

fn expr_as_place(
&mut self,
mut block: BasicBlock,
expr: &Expr<'tcx>,
expr_id: ExprId,
mutability: Mutability,
fake_borrow_temps: Option<&mut Vec<Local>>,
) -> BlockAnd<PlaceBuilder<'tcx>> {
let expr = &self.thir[expr_id];
debug!("expr_as_place(block={:?}, expr={:?}, mutability={:?})", block, expr, mutability);

let this = self;
Expand All @@ -413,31 +414,29 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
match expr.kind {
ExprKind::Scope { region_scope, lint_level, value } => {
this.in_scope((region_scope, source_info), lint_level, |this| {
this.expr_as_place(block, &this.thir[value], mutability, fake_borrow_temps)
this.expr_as_place(block, value, mutability, fake_borrow_temps)
})
}
ExprKind::Field { lhs, variant_index, name } => {
let lhs = &this.thir[lhs];
let lhs_expr = &this.thir[lhs];
let mut place_builder =
unpack!(block = this.expr_as_place(block, lhs, mutability, fake_borrow_temps,));
if let ty::Adt(adt_def, _) = lhs.ty.kind() {
if let ty::Adt(adt_def, _) = lhs_expr.ty.kind() {
if adt_def.is_enum() {
place_builder = place_builder.downcast(*adt_def, variant_index);
}
}
block.and(place_builder.field(name, expr.ty))
}
ExprKind::Deref { arg } => {
let place_builder = unpack!(
block =
this.expr_as_place(block, &this.thir[arg], mutability, fake_borrow_temps,)
);
let place_builder =
unpack!(block = this.expr_as_place(block, arg, mutability, fake_borrow_temps,));
block.and(place_builder.deref())
}
ExprKind::Index { lhs, index } => this.lower_index_expression(
block,
&this.thir[lhs],
&this.thir[index],
lhs,
index,
mutability,
fake_borrow_temps,
expr.temp_lifetime,
Expand All @@ -461,12 +460,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

ExprKind::PlaceTypeAscription { source, ref user_ty } => {
let place_builder = unpack!(
block = this.expr_as_place(
block,
&this.thir[source],
mutability,
fake_borrow_temps,
)
block = this.expr_as_place(block, source, mutability, fake_borrow_temps,)
);
if let Some(user_ty) = user_ty {
let annotation_index =
Expand Down Expand Up @@ -494,9 +488,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block.and(place_builder)
}
ExprKind::ValueTypeAscription { source, ref user_ty } => {
let source = &this.thir[source];
let temp =
unpack!(block = this.as_temp(block, source.temp_lifetime, source, mutability));
let source_expr = &this.thir[source];
let temp = unpack!(
block = this.as_temp(block, source_expr.temp_lifetime, source, mutability)
);
if let Some(user_ty) = user_ty {
let annotation_index =
this.canonical_user_type_annotations.push(CanonicalUserTypeAnnotation {
Expand Down Expand Up @@ -562,7 +557,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// these are not places, so we need to make a temporary.
debug_assert!(!matches!(Category::of(&expr.kind), Some(Category::Place)));
let temp =
unpack!(block = this.as_temp(block, expr.temp_lifetime, expr, mutability));
unpack!(block = this.as_temp(block, expr.temp_lifetime, expr_id, mutability));
block.and(PlaceBuilder::from(temp))
}
}
Expand Down Expand Up @@ -591,8 +586,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn lower_index_expression(
&mut self,
mut block: BasicBlock,
base: &Expr<'tcx>,
index: &Expr<'tcx>,
base: ExprId,
index: ExprId,
mutability: Mutability,
fake_borrow_temps: Option<&mut Vec<Local>>,
temp_lifetime: Option<region::Scope>,
Expand All @@ -609,7 +604,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// Making this a *fresh* temporary means we do not have to worry about
// the index changing later: Nothing will ever change this temporary.
// The "retagging" transformation (for Stacked Borrows) relies on this.
let idx = unpack!(block = self.as_temp(block, temp_lifetime, index, Mutability::Not,));
let idx = unpack!(block = self.as_temp(block, temp_lifetime, index, Mutability::Not));

block = self.bounds_check(block, &base_place, idx, expr_span, source_info);

Expand Down
Loading
Loading