Skip to content

remove let this = self; in compiler #148955

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+347 −363
Open

remove let this = self; in compiler #148955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
86 changes: 42 additions & 44 deletions compiler/rustc_mir_build/src/builder/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
expr: Option<ExprId>,
region_scope: Scope,
) -> BlockAnd<()> {
let this = self;

// This convoluted structure is to avoid using recursion as we walk down a list
// of statements. Basically, the structure we get back is something like:
//
Expand All @@ -58,21 +56,21 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
//
// First we build all the statements in the block.
let mut let_scope_stack = Vec::with_capacity(8);
let outer_source_scope = this.source_scope;
let outer_source_scope = self.source_scope;
// This scope information is kept for breaking out of the parent remainder scope in case
// one let-else pattern matching fails.
// By doing so, we can be sure that even temporaries that receive extended lifetime
// assignments are dropped, too.
let mut last_remainder_scope = region_scope;

let source_info = this.source_info(span);
let source_info = self.source_info(span);
for stmt in stmts {
let Stmt { ref kind } = this.thir[*stmt];
let Stmt { ref kind } = self.thir[*stmt];
match kind {
StmtKind::Expr { scope, expr } => {
this.block_context.push(BlockFrame::Statement { ignores_expr_result: true });
self.block_context.push(BlockFrame::Statement { ignores_expr_result: true });
let si = (*scope, source_info);
block = this
block = self
.in_scope(si, LintLevel::Inherited, |this| {
this.stmt_expr(block, *expr, Some(*scope))
})
Comment on lines +73 to 76
Copy link
Member

@Zalathar Zalathar Nov 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See how the code inside this closure uses this, but the code outside the closure uses self?

Being able to consistently use this in both places, regardless of whether they're nested in a closure, is the whole point of doing let this = self. So getting rid of it would make the code harder to read and harder to modify.

Expand Down Expand Up @@ -156,50 +154,50 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// └────────────────┘

let ignores_expr_result = matches!(pattern.kind, PatKind::Wild);
this.block_context.push(BlockFrame::Statement { ignores_expr_result });
self.block_context.push(BlockFrame::Statement { ignores_expr_result });

// Lower the `else` block first because its parent scope is actually
// enclosing the rest of the `let .. else ..` parts.
let else_block_span = this.thir[*else_block].span;
let else_block_span = self.thir[*else_block].span;
// This place is not really used because this destination place
// should never be used to take values at the end of the failure
// block.
let dummy_place = this.temp(this.tcx.types.never, else_block_span);
let failure_entry = this.cfg.start_new_block();
let dummy_place = self.temp(self.tcx.types.never, else_block_span);
let failure_entry = self.cfg.start_new_block();
let failure_block;
failure_block = this
failure_block = self
.ast_block(
dummy_place,
failure_entry,
*else_block,
this.source_info(else_block_span),
self.source_info(else_block_span),
)
.into_block();
this.cfg.terminate(
self.cfg.terminate(
failure_block,
this.source_info(else_block_span),
self.source_info(else_block_span),
TerminatorKind::Unreachable,
);

// Declare the bindings, which may create a source scope.
let remainder_span = remainder_scope.span(this.tcx, this.region_scope_tree);
this.push_scope((*remainder_scope, source_info));
let remainder_span = remainder_scope.span(self.tcx, self.region_scope_tree);
self.push_scope((*remainder_scope, source_info));
let_scope_stack.push(remainder_scope);

let visibility_scope =
Some(this.new_source_scope(remainder_span, LintLevel::Inherited));
Some(self.new_source_scope(remainder_span, LintLevel::Inherited));

let initializer_span = this.thir[*initializer].span;
let initializer_span = self.thir[*initializer].span;
let scope = (*init_scope, source_info);
let failure_and_block = this.in_scope(scope, *lint_level, |this| {
let failure_and_block = self.in_scope(scope, *lint_level, |this| {
this.declare_bindings(
visibility_scope,
remainder_span,
pattern,
None,
Some((Some(&destination), initializer_span)),
);
let else_block_span = this.thir[*else_block].span;
let else_block_span = self.thir[*else_block].span;
let (matching, failure) =
this.in_if_then_scope(last_remainder_scope, else_block_span, |this| {
this.lower_let_expr(
Expand All @@ -214,16 +212,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
matching.and(failure)
});
let failure = unpack!(block = failure_and_block);
this.cfg.goto(failure, source_info, failure_entry);
self.cfg.goto(failure, source_info, failure_entry);

if let Some(source_scope) = visibility_scope {
this.source_scope = source_scope;
self.source_scope = source_scope;
}
last_remainder_scope = *remainder_scope;
}
StmtKind::Let { init_scope, initializer: None, else_block: Some(_), .. } => {
span_bug!(
init_scope.span(this.tcx, this.region_scope_tree),
init_scope.span(self.tcx, self.region_scope_tree),
"initializer is missing, but else block is present in this let binding",
)
}
Expand All @@ -237,24 +235,24 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
span: _,
} => {
let ignores_expr_result = matches!(pattern.kind, PatKind::Wild);
this.block_context.push(BlockFrame::Statement { ignores_expr_result });
self.block_context.push(BlockFrame::Statement { ignores_expr_result });

// Enter the remainder scope, i.e., the bindings' destruction scope.
this.push_scope((*remainder_scope, source_info));
self.push_scope((*remainder_scope, source_info));
let_scope_stack.push(remainder_scope);

// Declare the bindings, which may create a source scope.
let remainder_span = remainder_scope.span(this.tcx, this.region_scope_tree);
let remainder_span = remainder_scope.span(self.tcx, self.region_scope_tree);

let visibility_scope =
Some(this.new_source_scope(remainder_span, LintLevel::Inherited));
Some(self.new_source_scope(remainder_span, LintLevel::Inherited));

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

block = this
block = self
.in_scope(scope, *lint_level, |this| {
this.declare_bindings(
visibility_scope,
Expand All @@ -269,7 +267,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
.into_block();
} else {
let scope = (*init_scope, source_info);
let _: BlockAnd<()> = this.in_scope(scope, *lint_level, |this| {
let _: BlockAnd<()> = self.in_scope(scope, *lint_level, |this| {
this.declare_bindings(
visibility_scope,
remainder_span,
Expand All @@ -281,7 +279,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
});

debug!("ast_block_stmts: pattern={:?}", pattern);
this.visit_primary_bindings(pattern, &mut |this, node, span| {
self.visit_primary_bindings(pattern, &mut |this, node, span| {
this.storage_live_binding(
block,
node,
Expand All @@ -296,30 +294,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

// Enter the visibility scope, after evaluating the initializer.
if let Some(source_scope) = visibility_scope {
this.source_scope = source_scope;
self.source_scope = source_scope;
}
last_remainder_scope = *remainder_scope;
}
}

let popped = this.block_context.pop();
let popped = self.block_context.pop();
assert!(popped.is_some_and(|bf| bf.is_statement()));
}

// Then, the block may have an optional trailing expression which is a “return” value
// of the block, which is stored into `destination`.
let tcx = this.tcx;
let destination_ty = destination.ty(&this.local_decls, tcx).ty;
let tcx = self.tcx;
let destination_ty = destination.ty(&self.local_decls, tcx).ty;
if let Some(expr_id) = expr {
let expr = &this.thir[expr_id];
let expr = &self.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 {
destination_ty.is_unit() || self.block_context.currently_ignores_tail_results();
self.block_context.push(BlockFrame::TailExpr {
info: BlockTailInfo { tail_result_is_ignored, span: expr.span },
});

block = this.expr_into_dest(destination, block, expr_id).into_block();
let popped = this.block_context.pop();
block = self.expr_into_dest(destination, block, expr_id).into_block();
let popped = self.block_context.pop();

assert!(popped.is_some_and(|bf| bf.is_tail_expr()));
} else {
Expand All @@ -334,16 +332,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
{
// We only want to assign an implicit `()` as the return value of the block if the
// block does not diverge. (Otherwise, we may try to assign a unit to a `!`-type.)
this.cfg.push_assign_unit(block, source_info, destination, this.tcx);
self.cfg.push_assign_unit(block, source_info, destination, self.tcx);
}
}
// Finally, we pop all the let scopes before exiting out from the scope of block
// itself.
for scope in let_scope_stack.into_iter().rev() {
block = this.pop_scope((*scope, source_info), block).into_block();
block = self.pop_scope((*scope, source_info), block).into_block();
}
// Restore the original source scope.
this.source_scope = outer_source_scope;
self.source_scope = outer_source_scope;
block.unit()
}
}
7 changes: 3 additions & 4 deletions compiler/rustc_mir_build/src/builder/expr/as_constant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// Compile `expr`, yielding a compile-time constant. Assumes that
/// `expr` is a valid compile-time constant!
pub(crate) fn as_constant(&mut self, expr: &Expr<'tcx>) -> ConstOperand<'tcx> {
let this = self;
let tcx = this.tcx;
let tcx = self.tcx;
let Expr { ty, temp_lifetime: _, span, ref kind } = *expr;
match kind {
ExprKind::Scope { region_scope: _, lint_level: _, value } => {
this.as_constant(&this.thir[*value])
self.as_constant(&self.thir[*value])
}
_ => as_constant_inner(
expr,
|user_ty| {
Some(this.canonical_user_type_annotations.push(CanonicalUserTypeAnnotation {
Some(self.canonical_user_type_annotations.push(CanonicalUserTypeAnnotation {
span,
user_ty: user_ty.clone(),
inferred_ty: ty,
Expand Down
33 changes: 15 additions & 18 deletions compiler/rustc_mir_build/src/builder/expr/as_operand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
local_info: LocalInfo<'tcx>,
needs_temporary: NeedsTemporary,
) -> BlockAnd<Operand<'tcx>> {
let this = self;

let expr = &this.thir[expr_id];
let expr = &self.thir[expr_id];
if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind {
let source_info = this.source_info(expr.span);
let source_info = self.source_info(expr.span);
let region_scope = (region_scope, source_info);
return this.in_scope(region_scope, lint_level, |this| {
return self.in_scope(region_scope, lint_level, |this| {
this.as_operand(block, scope, value, local_info, needs_temporary)
});
}
Expand All @@ -134,17 +132,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
match category {
Category::Constant
if matches!(needs_temporary, NeedsTemporary::No)
|| !expr.ty.needs_drop(this.tcx, this.typing_env()) =>
|| !expr.ty.needs_drop(self.tcx, self.typing_env()) =>
{
let constant = this.as_constant(expr);
let constant = self.as_constant(expr);
block.and(Operand::Constant(Box::new(constant)))
}
Category::Constant | Category::Place | Category::Rvalue(..) => {
let operand = unpack!(block = this.as_temp(block, scope, expr_id, Mutability::Mut));
let operand = unpack!(block = self.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 =
this.local_decls[operand].local_info.as_mut().unwrap_crate_local();
self.local_decls[operand].local_info.as_mut().unwrap_crate_local();
if let LocalInfo::Boring | LocalInfo::BlockTailTemp(_) = **decl_info {
**decl_info = local_info;
}
Expand All @@ -160,31 +158,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
scope: TempLifetime,
expr_id: ExprId,
) -> BlockAnd<Operand<'tcx>> {
let this = self;
let expr = &this.thir[expr_id];
let expr = &self.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 source_info = self.source_info(expr.span);
let region_scope = (region_scope, source_info);
return this.in_scope(region_scope, lint_level, |this| {
return self.in_scope(region_scope, lint_level, |this| {
this.as_call_operand(block, scope, value)
});
}

let tcx = this.tcx;
let tcx = self.tcx;

if tcx.features().unsized_fn_params() {
let ty = expr.ty;
if !ty.is_sized(tcx, this.typing_env()) {
if !ty.is_sized(tcx, self.typing_env()) {
// !sized means !copy, so this is an unsized move
assert!(!tcx.type_is_copy_modulo_regions(this.typing_env(), ty));
assert!(!tcx.type_is_copy_modulo_regions(self.typing_env(), ty));

// As described above, detect the case where we are passing a value of unsized
// 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, arg, Mutability::Mut));
let operand = unpack!(block = self.as_temp(block, scope, arg, Mutability::Mut));

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

this.as_operand(block, scope, expr_id, LocalInfo::Boring, NeedsTemporary::Maybe)
self.as_operand(block, scope, expr_id, LocalInfo::Boring, NeedsTemporary::Maybe)
}
}
Loading
Loading