Rollup merge of #149016 - Zalathar:let-this-self, r=Kivooeo

Document the `let this = self;` idiom used in MIR building

In `rustc_mir_build` there are a few `Builder` methods that start with `let this = self;`, so that subsequent code can uniformly refer to the builder as `this`, instead of having to choose between `self` at the top level or `this` when nested in closures that need to borrow the builder.

There is some existing documentation of the idiom in `expr_into_dest`:

https://github.com/rust-lang/rust/blob/69d4d5fc0e4db60272aac85ef27ecccef5764f3a/compiler/rustc_mir_build/src/builder/expr/into.rs#L32-L35

But that documentation is brief and hard to find, especially if one is unaware that such documentation even exists.

---

This PR therefore adds a longer explanation of the `let this = self;` idiom in the module documentation for `rustc_mir_build::builder`, and makes that documentation easier to find by adding a searchable tag (“LET_THIS_SELF”) to the documentation and to each occurrence of the idiom.

Author: matthiaskrgr

compiler/rustc_mir_build/src/builder/block.rs

@@ -39,7 +39,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         expr: Option<ExprId>,
         region_scope: Scope,
     ) -> BlockAnd<()> {
-        let this = self;
+        let this = self; // See "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:

compiler/rustc_mir_build/src/builder/expr/as_constant.rs

@@ -19,7 +19,7 @@ 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 this = self; // See "LET_THIS_SELF".
         let tcx = this.tcx;
         let Expr { ty, temp_scope_id: _, span, ref kind } = *expr;
         match kind {

compiler/rustc_mir_build/src/builder/expr/as_operand.rs

@@ -119,7 +119,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         local_info: LocalInfo<'tcx>,
         needs_temporary: NeedsTemporary,
     ) -> BlockAnd<Operand<'tcx>> {
-        let this = self;
+        let this = self; // See "LET_THIS_SELF".
 
         let expr = &this.thir[expr_id];
         if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind {
@@ -161,7 +161,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         scope: TempLifetime,
         expr_id: ExprId,
     ) -> BlockAnd<Operand<'tcx>> {
-        let this = self;
+        let this = self; // See "LET_THIS_SELF".
         let expr = &this.thir[expr_id];
         debug!("as_call_operand(block={:?}, expr={:?})", block, expr);
 

compiler/rustc_mir_build/src/builder/expr/as_place.rs

@@ -423,7 +423,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         let expr = &self.thir[expr_id];
         debug!("expr_as_place(block={:?}, expr={:?}, mutability={:?})", block, expr, mutability);
 
-        let this = self;
+        let this = self; // See "LET_THIS_SELF".
         let expr_span = expr.span;
         let source_info = this.source_info(expr_span);
         match expr.kind {

compiler/rustc_mir_build/src/builder/expr/as_rvalue.rs

@@ -47,7 +47,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         scope: TempLifetime,
         expr_id: ExprId,
     ) -> BlockAnd<Rvalue<'tcx>> {
-        let this = self;
+        let this = self; // See "LET_THIS_SELF".
         let expr = &this.thir[expr_id];
         debug!("expr_as_rvalue(block={:?}, scope={:?}, expr={:?})", block, scope, expr);
 
@@ -676,7 +676,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         scope: TempLifetime,
         outer_source_info: SourceInfo,
     ) -> BlockAnd<Rvalue<'tcx>> {
-        let this = self;
+        let this = self; // See "LET_THIS_SELF".
         let value_expr = &this.thir[value];
         let elem_ty = value_expr.ty;
         if this.check_constness(&value_expr.kind) {
@@ -716,7 +716,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         mut block: BasicBlock,
         arg: ExprId,
     ) -> BlockAnd<Operand<'tcx>> {
-        let this = self;
+        let this = self; // See "LET_THIS_SELF".
 
         let source_info = this.source_info(upvar_span);
         let temp = this.local_decls.push(LocalDecl::new(upvar_ty, upvar_span));

compiler/rustc_mir_build/src/builder/expr/as_temp.rs

@@ -34,7 +34,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         expr_id: ExprId,
         mutability: Mutability,
     ) -> BlockAnd<Local> {
-        let this = self;
+        let this = self; // See "LET_THIS_SELF".
 
         let expr = &this.thir[expr_id];
         let expr_span = expr.span;

compiler/rustc_mir_build/src/builder/expr/into.rs

@@ -32,7 +32,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         // since we frequently have to reference `self` from within a
         // closure, where `self` would be shadowed, it's easier to
         // just use the name `this` uniformly
-        let this = self;
+        let this = self; // See "LET_THIS_SELF".
         let expr = &this.thir[expr_id];
         let expr_span = expr.span;
         let source_info = this.source_info(expr_span);

compiler/rustc_mir_build/src/builder/expr/stmt.rs

@@ -18,7 +18,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         expr_id: ExprId,
         statement_scope: Option<region::Scope>,
     ) -> BlockAnd<()> {
-        let this = self;
+        let this = self; // See "LET_THIS_SELF".
         let expr = &this.thir[expr_id];
         let expr_span = expr.span;
         let source_info = this.source_info(expr.span);

compiler/rustc_mir_build/src/builder/matches/mod.rs

@@ -109,7 +109,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         expr_id: ExprId,   // Condition expression to lower
         args: ThenElseArgs,
     ) -> BlockAnd<()> {
-        let this = self;
+        let this = self; // See "LET_THIS_SELF".
         let expr = &this.thir[expr_id];
         let expr_span = expr.span;
 

compiler/rustc_mir_build/src/builder/mod.rs

@@ -2,6 +2,23 @@
 //! "Go to file" feature to silently ignore all files in the module, probably
 //! because it assumes that "build" is a build-output directory.
 //! See <https://github.com/rust-lang/rust/pull/134365>.
+//!
+//! ## The `let this = self;` idiom (LET_THIS_SELF)
+//!
+//! Throughout MIR building there are several places where a `Builder` method
+//! needs to borrow `self`, and then re-expose it to a closure as `|this|`.
+//!
+//! In complex builder methods, potentially with multiple levels of nesting, it
+//! would thus become necessary to mentally keep track of whether the builder
+//! is `self` (at the top level) or `this` (nested in a closure), or to replace
+//! one with the other when moving code in or out of a closure.
+//!
+//! (The borrow checker will prevent incorrect usage, but having to go back and
+//! satisfy the borrow checker still creates contributor friction.)
+//!
+//! To reduce that friction, some builder methods therefore start with
+//! `let this = self;` or similar, allowing subsequent code to uniformly refer
+//! to the builder as `this` (and never `self`), even when not nested.
 
 use itertools::Itertools;
 use rustc_abi::{ExternAbi, FieldIdx};