Apply suggestions from code review

folkertdev · traviscross · folkertdev · commit 3e1f56f8753d · 2025-04-30T17:23:32.000+02:00
Co-authored-by: Travis Cross &lt;tc@traviscross.com&gt;
diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs
@@ -613,7 +613,10 @@ pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         EncodeCrossCrate::Yes, min_generic_const_args, experimental!(type_const),
     ),
 
-    // #[loop_match] and #[const_continue]
+    // The `#[loop_match]` and `#[const_continue]` attributes are part of the
+    // lang experiment for RFC 3720 tracked in:
+    //
+    // - https://github.com/rust-lang/rust/issues/132306
     gated!(
         const_continue, Normal, template!(Word), ErrorFollowing,
         EncodeCrossCrate::No, loop_match, experimental!(const_continue)
diff --git a/compiler/rustc_feature/src/unstable.rs b/compiler/rustc_feature/src/unstable.rs
@@ -550,7 +550,7 @@ declare_features! (
     /// to pass custom arguments to the linker.
     (unstable, link_arg_attribute, "1.76.0", Some(99427)),
     /// Allows fused `loop`/`match` for direct intraprocedural jumps.
-    (incomplete, loop_match, "CURRENT_RUSTC_VERSION", Some(138777)),
+    (incomplete, loop_match, "CURRENT_RUSTC_VERSION", Some(132306)),
     /// Give access to additional metadata about declarative macro meta-variables.
     (unstable, macro_metavar_expr, "1.61.0", Some(83527)),
     /// Provides a way to concatenate identifiers using metavariable expressions.
diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs
@@ -377,7 +377,7 @@ pub enum ExprKind<'tcx> {
     },
     /// A `#[loop_match] loop { state = 'blk: { match state { ... } } }` expression.
     LoopMatch {
-        /// The state variable that is updated, and also the scrutinee of the match
+        /// The state variable that is updated, and also the scrutinee of the match.
         state: ExprId,
         region_scope: region::Scope,
         arms: Box<[ArmId]>,
diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl
@@ -227,11 +227,11 @@ mir_build_loop_match_bad_statements =
     statements are not allowed in this position within a `#[loop_match]`
 
 mir_build_loop_match_invalid_match =
-    invalid match on `loop_match` state
-    .note = only matches on local variables are valid
+    invalid match on `#[loop_match]` state
+    .note = a local variable must be the scrutinee within a `#[loop_match]`
 
 mir_build_loop_match_invalid_update =
-    invalid update of the `loop_match` state
+    invalid update of the `#[loop_match]` state
     .label = the assignment must update this variable
 
 mir_build_loop_match_missing_assignment =
diff --git a/compiler/rustc_mir_build/src/builder/expr/into.rs b/compiler/rustc_mir_build/src/builder/expr/into.rs
@@ -310,7 +310,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
                     // The `built_tree` maps match arms to their basic block (where control flow
                     // jumps to when a value matches the arm). This structure is stored so that a
-                    // #[const_continue] can figure out what basic block to jump to.
+                    // `#[const_continue]` can figure out what basic block to jump to.
                     let built_tree = this.lower_match_tree(
                         body_block,
                         scrutinee_span,
@@ -327,7 +327,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     // hence the `in_breakable_scope`.
                     //
                     // Then `in_const_continuable_scope` stores information for the lowering of
-                    // #[const_continue], and finally the match is lowered in the standard way.
+                    // `#[const_continue]`, and finally the match is lowered in the standard way.
                     unpack!(
                         body_block = this.in_scope(
                             (region_scope, source_info),
diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs
@@ -2868,7 +2868,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         true
     }
 
-    /// Attempt to statically pick the BasicBlock that a value would resolve to at runtime.
+    /// Attempt to statically pick the `BasicBlock` that a value would resolve to at runtime.
     pub(crate) fn static_pattern_match(
         &self,
         cx: &RustcPatCtxt<'_, 'tcx>,
@@ -2927,7 +2927,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             // a lot of care around intrinsics. For an issue to happen here, it would require a
             // macro expanding to a `simd_shuffle` call without wrapping the constant argument in a
             // `const {}` block, but the user pass through arbitrary expressions.
-            // FIXME(oli-obk): replace the magic const generic argument of `simd_shuffle` with a
+
+            // FIXME(oli-obk): Replace the magic const generic argument of `simd_shuffle` with a
             // real const generic, and get rid of this entire function.
             other => span_bug!(constant.span, "{other:#?}"),
         };
@@ -2978,7 +2979,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             },
             Constructor::Wildcard => true,
 
-            // these we may eventually support
+            // These we may eventually support:
             Constructor::Struct
             | Constructor::Ref
             | Constructor::Slice(_)
@@ -2990,7 +2991,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             | Constructor::F128Range(..)
             | Constructor::Str(_) => bug!("unsupported pattern constructor {:?}", pat.ctor()),
 
-            // these should never occur here
+            // These should never occur here:
             Constructor::Opaque(_)
             | Constructor::Never
             | Constructor::NonExhaustive
diff --git a/compiler/rustc_mir_build/src/builder/scope.rs b/compiler/rustc_mir_build/src/builder/scope.rs
@@ -874,8 +874,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             _ => span_bug!(state_decl.source_info.span, "unsupported #[loop_match] state"),
         };
 
-        // The PatCtxt is normally used in pattern exhaustiveness checking, but reused here
-        // because it performs normalization and const evaluation.
+        // The `PatCtxt` is normally used in pattern exhaustiveness checking, but reused
+        // here because it performs normalization and const evaluation.
         let dropless_arena = rustc_arena::DroplessArena::default();
         let typeck_results = self.tcx.typeck(self.def_id);
         let cx = RustcPatCtxt {
diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs
@@ -873,7 +873,7 @@ impl<'tcx> ThirBuildCx<'tcx> {
                 if is_loop_match {
                     let dcx = self.tcx.dcx();
 
-                    // accept either `state = expr` or `state = expr;`
+                    // Accept either `state = expr` or `state = expr;`.
                     let loop_body_expr = match body.stmts {
                         [] => match body.expr {
                             Some(expr) => expr,
diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl
@@ -104,7 +104,7 @@ passes_const_continue_attr =
     .label = not a break expression
 
 passes_const_continue_bad_label =
-    `#[const_continue]` must break to a labeled block that participates in a `#[loop_match]`
+    `#[const_continue]` must break to a labeled block in a `#[loop_match]`
 
 passes_const_stable_not_stable =
     attribute `#[rustc_const_stable]` can only be applied to functions that are declared `#[stable]`
diff --git a/compiler/rustc_passes/src/loops.rs b/compiler/rustc_passes/src/loops.rs
@@ -39,9 +39,9 @@ enum Context {
     AnonConst,
     /// E.g. `const { ... }`.
     ConstBlock,
-    /// `#[loop_match] loop { state = 'label: { /* ... */ } }`
+    /// E.g. `#[loop_match] loop { state = 'label: { /* ... */ } }`.
     LoopMatch {
-        /// The label of the labeled block (so not the loop!)
+        /// The label of the labeled block (not of the loop itself).
         labeled_block: Label,
     },
 }
@@ -228,7 +228,7 @@ impl<'hir> Visitor<'hir> for CheckLoopVisitor<'hir> {
                     Err(hir::LoopIdError::UnresolvedLabel) => None,
                 };
 
-                // a #[const_continue] must be to a block that participates in #[loop_match]
+                // A `#[const_continue]` must break to a block in a `#[loop_match]`.
                 let attrs = self.tcx.hir_attrs(e.hir_id);
                 if attrs.iter().any(|attr| attr.has_name(sym::const_continue)) {
                     if let Some(break_label) = break_label.label {
@@ -438,9 +438,9 @@ impl<'hir> CheckLoopVisitor<'hir> {
             return None;
         }
 
-        // NOTE: diagnostics are emitted during MIR construction.
+        // NOTE: Diagnostics are emitted during MIR construction.
 
-        // accept either `state = expr` or `state = expr;`
+        // Accept either `state = expr` or `state = expr;`.
         let loop_body_expr = match body.stmts {
             [] => match body.expr {
                 Some(expr) => expr,
diff --git a/tests/ui/feature-gates/feature-gate-loop-match.rs b/tests/ui/feature-gates/feature-gate-loop-match.rs
@@ -1,4 +1,5 @@
-// A #[const_continue] to a labeled loop should error.
+// Test that `#[loop_match]` and `#[const_continue]` cannot be used without
+// `#![feature(loop_match)]`.
 
 enum State {
     A,
diff --git a/tests/ui/feature-gates/feature-gate-loop-match.stderr b/tests/ui/feature-gates/feature-gate-loop-match.stderr
@@ -1,30 +1,30 @@
 error[E0658]: the `#[loop_match]` attribute is an experimental feature
-  --> $DIR/feature-gate-loop-match.rs:11:5
+  --> $DIR/feature-gate-loop-match.rs:12:5
    |
 LL |     #[loop_match]
    |     ^^^^^^^^^^^^^
    |
-   = note: see issue #138777 <https://github.com/rust-lang/rust/issues/138777> for more information
+   = note: see issue #132306 <https://github.com/rust-lang/rust/issues/132306> for more information
    = help: add `#![feature(loop_match)]` to the crate attributes to enable
    = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
 
 error[E0658]: the `#[const_continue]` attribute is an experimental feature
-  --> $DIR/feature-gate-loop-match.rs:16:21
+  --> $DIR/feature-gate-loop-match.rs:17:21
    |
 LL |                     #[const_continue]
    |                     ^^^^^^^^^^^^^^^^^
    |
-   = note: see issue #138777 <https://github.com/rust-lang/rust/issues/138777> for more information
+   = note: see issue #132306 <https://github.com/rust-lang/rust/issues/132306> for more information
    = help: add `#![feature(loop_match)]` to the crate attributes to enable
    = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
 
 error[E0658]: the `#[const_continue]` attribute is an experimental feature
-  --> $DIR/feature-gate-loop-match.rs:21:21
+  --> $DIR/feature-gate-loop-match.rs:22:21
    |
 LL |                     #[const_continue]
    |                     ^^^^^^^^^^^^^^^^^
    |
-   = note: see issue #138777 <https://github.com/rust-lang/rust/issues/138777> for more information
+   = note: see issue #132306 <https://github.com/rust-lang/rust/issues/132306> for more information
    = help: add `#![feature(loop_match)]` to the crate attributes to enable
    = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
 
diff --git a/tests/ui/loop-match/break-to-block.rs b/tests/ui/loop-match/break-to-block.rs
@@ -1,4 +1,4 @@
-// Test that a break without #[const_continue] still works as expected.
+// Test that a `break` without `#[const_continue]` still works as expected.
 
 //@ run-pass
 
diff --git a/tests/ui/loop-match/const-continue-to-block.rs b/tests/ui/loop-match/const-continue-to-block.rs
@@ -1,5 +1,5 @@
-// A #[const_continue] to a normal labeled block (that does is not part of a #[loop_match]) should
-// error.
+// Test that a `#[const_continue]` that breaks to a normal labeled block (that
+// is not part of a `#[loop_match]`) produces an error.
 
 #![allow(incomplete_features)]
 #![feature(loop_match)]
@@ -18,7 +18,7 @@ fn const_continue_to_block() -> u8 {
                 _ => 'b: {
                     #[const_continue]
                     break 'b 2;
-                    //~^ ERROR `#[const_continue]` must break to a labeled block that participates in a `#[loop_match]`
+                    //~^ ERROR `#[const_continue]` must break to a labeled block in a `#[loop_match]`
                 }
             }
         }
diff --git a/tests/ui/loop-match/const-continue-to-block.stderr b/tests/ui/loop-match/const-continue-to-block.stderr
@@ -1,4 +1,4 @@
-error: `#[const_continue]` must break to a labeled block that participates in a `#[loop_match]`
+error: `#[const_continue]` must break to a labeled block in a `#[loop_match]`
   --> $DIR/const-continue-to-block.rs:20:27
    |
 LL |                     break 'b 2;
diff --git a/tests/ui/loop-match/const-continue-to-loop.rs b/tests/ui/loop-match/const-continue-to-loop.rs
@@ -1,3 +1,7 @@
+// Test that a `#[const_continue]` that breaks to the label of the loop itself
+// rather than to the label of the block within the `#[loop_match]` produces an
+// error.
+
 #![allow(incomplete_features)]
 #![feature(loop_match)]
 #![crate_type = "lib"]
@@ -15,7 +19,7 @@ fn const_continue_to_loop() -> u8 {
                 _ => {
                     #[const_continue]
                     break 'a 2;
-                    //~^ ERROR `#[const_continue]` must break to a labeled block that participates in a `#[loop_match]`
+                    //~^ ERROR `#[const_continue]` must break to a labeled block in a `#[loop_match]`
                 }
             }
         }
diff --git a/tests/ui/loop-match/const-continue-to-loop.stderr b/tests/ui/loop-match/const-continue-to-loop.stderr
@@ -1,5 +1,5 @@
-error: `#[const_continue]` must break to a labeled block that participates in a `#[loop_match]`
-  --> $DIR/const-continue-to-loop.rs:17:27
+error: `#[const_continue]` must break to a labeled block in a `#[loop_match]`
+  --> $DIR/const-continue-to-loop.rs:21:27
    |
 LL |                     break 'a 2;
    |                           ^^
diff --git a/tests/ui/loop-match/integer-patterns.rs b/tests/ui/loop-match/integer-patterns.rs
@@ -1,4 +1,4 @@
-// Test that signed and unsigned integer patterns work with #[loop_match].
+// Test that signed and unsigned integer patterns work with `#[loop_match]`.
 
 //@ run-pass
 
diff --git a/tests/ui/loop-match/invalid-attribute.rs b/tests/ui/loop-match/invalid-attribute.rs
@@ -1,5 +1,5 @@
-// Checks that #[loop_match] and #[const_continue] attributes can be
-// placed on expressions only.
+// Test that the `#[loop_match]` and `#[const_continue]` attributes can only be
+// placed on expressions.
 
 #![allow(incomplete_features)]
 #![feature(loop_match)]
diff --git a/tests/ui/loop-match/invalid.rs b/tests/ui/loop-match/invalid.rs
@@ -1,4 +1,5 @@
-// Test that a good error is emitted when #[loop_match] is applied to syntax it does not support.
+// Test that the correct error is emitted when `#[loop_match]` is applied to
+// syntax it does not support.
 #![allow(incomplete_features)]
 #![feature(loop_match)]
 #![crate_type = "lib"]
@@ -15,7 +16,7 @@ fn invalid_update() {
     #[loop_match]
     loop {
         fake = 'blk: {
-            //~^ ERROR invalid update of the `loop_match` state
+            //~^ ERROR invalid update of the `#[loop_match]` state
             match state {
                 _ => State::B,
             }
@@ -29,7 +30,7 @@ fn invalid_scrutinee() {
     loop {
         state = 'blk: {
             match State::A {
-                //~^ ERROR invalid match on `loop_match` state
+                //~^ ERROR invalid match on `#[loop_match]` state
                 _ => State::B,
             }
         }
diff --git a/tests/ui/loop-match/invalid.stderr b/tests/ui/loop-match/invalid.stderr
@@ -1,5 +1,5 @@
 error[E0308]: mismatched types
-  --> $DIR/invalid.rs:121:21
+  --> $DIR/invalid.rs:122:21
    |
 LL |                     break 'blk;
    |                     ^^^^^^^^^^ expected `State`, found `()`
@@ -9,49 +9,49 @@ help: give the `break` a value of the expected type
 LL |                     break 'blk /* value */;
    |                                +++++++++++
 
-error: invalid update of the `loop_match` state
-  --> $DIR/invalid.rs:17:9
+error: invalid update of the `#[loop_match]` state
+  --> $DIR/invalid.rs:18:9
    |
 LL |         fake = 'blk: {
    |         ^^^^
 LL |
 LL |             match state {
    |                   ----- the assignment must update this variable
 
-error: invalid match on `loop_match` state
-  --> $DIR/invalid.rs:31:19
+error: invalid match on `#[loop_match]` state
+  --> $DIR/invalid.rs:32:19
    |
 LL |             match State::A {
    |                   ^^^^^^^^
    |
-   = note: only matches on local variables are valid
+   = note: a local variable must be the scrutinee within a `#[loop_match]`
 
 error: statements are not allowed in this position within a `#[loop_match]`
-  --> $DIR/invalid.rs:43:9
+  --> $DIR/invalid.rs:44:9
    |
 LL |         1;
    |         ^^
 
 error: statements are not allowed in this position within a `#[loop_match]`
-  --> $DIR/invalid.rs:58:13
+  --> $DIR/invalid.rs:59:13
    |
 LL |             1;
    |             ^^
 
 error: this expression must be a single `match` wrapped in a labeled block
-  --> $DIR/invalid.rs:71:17
+  --> $DIR/invalid.rs:72:17
    |
 LL |         state = State::B
    |                 ^^^^^^^^
 
 error: this expression must be a single `match` wrapped in a labeled block
-  --> $DIR/invalid.rs:81:13
+  --> $DIR/invalid.rs:82:13
    |
 LL |             State::B
    |             ^^^^^^^^
 
 error: this expression must be a single `match` wrapped in a labeled block
-  --> $DIR/invalid.rs:91:17
+  --> $DIR/invalid.rs:92:17
    |
 LL |           state = 'blk: {
    |  _________________^
@@ -60,13 +60,13 @@ LL | |         }
    | |_________^
 
 error: expected a single assignment expression
-  --> $DIR/invalid.rs:101:9
+  --> $DIR/invalid.rs:102:9
    |
 LL |         ()
    |         ^^
 
 error: expected a single assignment expression
-  --> $DIR/invalid.rs:108:10
+  --> $DIR/invalid.rs:109:10
    |
 LL |       loop {
    |  __________^
@@ -75,13 +75,13 @@ LL | |     }
    | |_____^
 
 error: a `#[const_continue]` must break to a label with a value
-  --> $DIR/invalid.rs:138:21
+  --> $DIR/invalid.rs:139:21
    |
 LL |                     break 'blk;
    |                     ^^^^^^^^^^
 
 error: match arms that are part of a `#[loop_match]` cannot have guards
-  --> $DIR/invalid.rs:156:29
+  --> $DIR/invalid.rs:157:29
    |
 LL |                 State::B if cond => break 'a,
    |                             ^^^^
diff --git a/tests/ui/loop-match/loop-match.rs b/tests/ui/loop-match/loop-match.rs
diff --git a/tests/ui/loop-match/nested.rs b/tests/ui/loop-match/nested.rs
diff --git a/tests/ui/loop-match/or-patterns.rs b/tests/ui/loop-match/or-patterns.rs
diff --git a/tests/ui/loop-match/unsupported-type.rs b/tests/ui/loop-match/unsupported-type.rs
diff --git a/tests/ui/loop-match/unsupported-type.stderr b/tests/ui/loop-match/unsupported-type.stderr
diff --git a/tests/ui/loop-match/unwind.rs b/tests/ui/loop-match/unwind.rs
