Auto merge of #118771 - workingjubilee:rollup-q1p3riz, r=workingjubilee

Rollup of 7 pull requests

Successful merges:

 - #118198 (coverage: Use `SpanMarker` to improve coverage spans for `if !` expressions)
 - #118512 (Add tests related to normalization in implied bounds)
 - #118610 (update target feature following LLVM API change)
 - #118666 (coverage: Simplify the heuristic for ignoring `async fn` return spans)
 - #118737 (Extend tidy alphabetical checking to `tests/`.)
 - #118762 (Some more minor `async gen`-related nits)
 - #118764 (Make async generators fused by default)

r? `@ghost`
`@rustbot` modify labels: rollup

compiler/rustc_ast/src/ast.rs

@@ -2450,6 +2450,14 @@ impl CoroutineKind {
         matches!(self, CoroutineKind::Gen { .. })
     }
 
+    pub fn closure_id(self) -> NodeId {
+        match self {
+            CoroutineKind::Async { closure_id, .. }
+            | CoroutineKind::Gen { closure_id, .. }
+            | CoroutineKind::AsyncGen { closure_id, .. } => closure_id,
+        }
+    }
+
     /// In this case this is an `async` or `gen` return, the `NodeId` for the generated `impl Trait`
     /// item.
     pub fn return_id(self) -> (NodeId, Span) {

compiler/rustc_ast_lowering/src/item.rs

@@ -1035,10 +1035,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let (Some(coroutine_kind), Some(body)) = (coroutine_kind, body) else {
             return self.lower_fn_body_block(span, decl, body);
         };
-        // FIXME(gen_blocks): Introduce `closure_id` method and remove ALL destructuring.
-        let (CoroutineKind::Async { closure_id, .. }
-        | CoroutineKind::Gen { closure_id, .. }
-        | CoroutineKind::AsyncGen { closure_id, .. }) = coroutine_kind;
+        let closure_id = coroutine_kind.closure_id();
 
         self.lower_body(|this| {
             let mut parameters: Vec<hir::Param<'_>> = Vec::new();

compiler/rustc_ast_lowering/src/lib.rs

@@ -177,7 +177,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             } else {
                 [sym::gen_future].into()
             },
-            // FIXME(gen_blocks): how does `closure_track_caller`
+            // FIXME(gen_blocks): how does `closure_track_caller`/`async_fn_track_caller`
+            // interact with `gen`/`async gen` blocks
             allow_async_iterator: [sym::gen_future, sym::async_iterator].into(),
             generics_def_id_map: Default::default(),
             host_param_id: None,

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -1271,11 +1271,11 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
         // Functions cannot both be `const async` or `const gen`
         if let Some(&FnHeader {
             constness: Const::Yes(cspan),
-            coroutine_kind: Some(coro_kind),
+            coroutine_kind: Some(coroutine_kind),
             ..
         }) = fk.header()
         {
-            let aspan = match coro_kind {
+            let aspan = match coroutine_kind {
                 CoroutineKind::Async { span: aspan, .. }
                 | CoroutineKind::Gen { span: aspan, .. }
                 | CoroutineKind::AsyncGen { span: aspan, .. } => aspan,

compiler/rustc_builtin_macros/src/test.rs

@@ -541,8 +541,8 @@ fn check_test_signature(
         return Err(sd.emit_err(errors::TestBadFn { span: i.span, cause: span, kind: "unsafe" }));
     }
 
-    if let Some(coro_kind) = f.sig.header.coroutine_kind {
-        match coro_kind {
+    if let Some(coroutine_kind) = f.sig.header.coroutine_kind {
+        match coroutine_kind {
             ast::CoroutineKind::Async { span, .. } => {
                 return Err(sd.emit_err(errors::TestBadFn {
                     span: i.span,

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

@@ -100,6 +100,9 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
 
         let Coverage { kind } = coverage;
         match *kind {
+            // Span markers are only meaningful during MIR instrumentation,
+            // and have no effect during codegen.
+            CoverageKind::SpanMarker => {}
             CoverageKind::CounterIncrement { id } => {
                 func_coverage.mark_counter_id_seen(id);
                 // We need to explicitly drop the `RefMut` before calling into `instrprof_increment`,

compiler/rustc_codegen_llvm/src/llvm_util.rs

@@ -263,6 +263,10 @@ pub fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> LLVMFeature<'a> {
             "sve2-bitperm",
             TargetFeatureFoldStrength::EnableOnly("neon"),
         ),
+        // The unaligned-scalar-mem feature was renamed to fast-unaligned-access.
+        ("riscv32" | "riscv64", "fast-unaligned-access") if get_version().0 <= 17 => {
+            LLVMFeature::new("unaligned-scalar-mem")
+        }
         (_, s) => LLVMFeature::new(s),
     }
 }

compiler/rustc_codegen_ssa/src/target_features.rs

@@ -291,9 +291,9 @@ const RISCV_ALLOWED_FEATURES: &[(&str, Stability)] = &[
     ("d", Unstable(sym::riscv_target_feature)),
     ("e", Unstable(sym::riscv_target_feature)),
     ("f", Unstable(sym::riscv_target_feature)),
+    ("fast-unaligned-access", Unstable(sym::riscv_target_feature)),
     ("m", Stable),
     ("relax", Unstable(sym::riscv_target_feature)),
-    ("unaligned-scalar-mem", Unstable(sym::riscv_target_feature)),
     ("v", Unstable(sym::riscv_target_feature)),
     ("zba", Stable),
     ("zbb", Stable),

compiler/rustc_lint/src/early.rs

@@ -162,11 +162,8 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
         // Explicitly check for lints associated with 'closure_id', since
         // it does not have a corresponding AST node
         if let ast_visit::FnKind::Fn(_, _, sig, _, _, _) = fk {
-            if let Some(coro_kind) = sig.header.coroutine_kind {
-                let (ast::CoroutineKind::Async { closure_id, .. }
-                | ast::CoroutineKind::Gen { closure_id, .. }
-                | ast::CoroutineKind::AsyncGen { closure_id, .. }) = coro_kind;
-                self.check_id(closure_id);
+            if let Some(coroutine_kind) = sig.header.coroutine_kind {
+                self.check_id(coroutine_kind.closure_id());
             }
         }
     }
@@ -226,12 +223,10 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
         // it does not have a corresponding AST node
         match e.kind {
             ast::ExprKind::Closure(box ast::Closure {
-                coroutine_kind: Some(coro_kind), ..
+                coroutine_kind: Some(coroutine_kind),
+                ..
             }) => {
-                let (ast::CoroutineKind::Async { closure_id, .. }
-                | ast::CoroutineKind::Gen { closure_id, .. }
-                | ast::CoroutineKind::AsyncGen { closure_id, .. }) = coro_kind;
-                self.check_id(closure_id);
+                self.check_id(coroutine_kind.closure_id());
             }
             _ => {}
         }

compiler/rustc_middle/src/mir/coverage.rs

@@ -76,6 +76,13 @@ impl Debug for CovTerm {
 
 #[derive(Clone, PartialEq, TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
 pub enum CoverageKind {
+    /// Marks a span that might otherwise not be represented in MIR, so that
+    /// coverage instrumentation can associate it with its enclosing block/BCB.
+    ///
+    /// Only used by the `InstrumentCoverage` pass, and has no effect during
+    /// codegen.
+    SpanMarker,
+
     /// Marks the point in MIR control flow represented by a coverage counter.
     ///
     /// This is eventually lowered to `llvm.instrprof.increment` in LLVM IR.
@@ -99,6 +106,7 @@ impl Debug for CoverageKind {
     fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
         use CoverageKind::*;
         match self {
+            SpanMarker => write!(fmt, "SpanMarker"),
             CounterIncrement { id } => write!(fmt, "CounterIncrement({:?})", id.index()),
             ExpressionUsed { id } => write!(fmt, "ExpressionUsed({:?})", id.index()),
         }

compiler/rustc_mir_build/src/build/cfg.rs

@@ -101,6 +101,19 @@ impl<'tcx> CFG<'tcx> {
         self.push(block, stmt);
     }
 
+    /// Adds a dummy statement whose only role is to associate a span with its
+    /// enclosing block for the purposes of coverage instrumentation.
+    ///
+    /// This results in more accurate coverage reports for certain kinds of
+    /// syntax (e.g. `continue` or `if !`) that would otherwise not appear in MIR.
+    pub(crate) fn push_coverage_span_marker(&mut self, block: BasicBlock, source_info: SourceInfo) {
+        let kind = StatementKind::Coverage(Box::new(Coverage {
+            kind: coverage::CoverageKind::SpanMarker,
+        }));
+        let stmt = Statement { source_info, kind };
+        self.push(block, stmt);
+    }
+
     pub(crate) fn terminate(
         &mut self,
         block: BasicBlock,

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

@@ -90,6 +90,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let local_scope = this.local_scope();
                 let (success_block, failure_block) =
                     this.in_if_then_scope(local_scope, expr_span, |this| {
+                        // Help out coverage instrumentation by injecting a dummy statement with
+                        // the original condition's span (including `!`). This fixes #115468.
+                        if this.tcx.sess.instrument_coverage() {
+                            this.cfg.push_coverage_span_marker(block, this.source_info(expr_span));
+                        }
                         this.then_else_break(
                             block,
                             &this.thir[arg],

compiler/rustc_mir_build/src/build/scope.rs

@@ -90,7 +90,6 @@ use rustc_index::{IndexSlice, IndexVec};
 use rustc_middle::middle::region;
 use rustc_middle::mir::*;
 use rustc_middle::thir::{Expr, LintLevel};
-use rustc_middle::ty::Ty;
 use rustc_session::lint::Level;
 use rustc_span::{Span, DUMMY_SP};
 
@@ -660,14 +659,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             (None, Some(_)) => {
                 panic!("`return`, `become` and `break` with value and must have a destination")
             }
-            (None, None) if self.tcx.sess.instrument_coverage() => {
-                // Unlike `break` and `return`, which push an `Assign` statement to MIR, from which
-                // a Coverage code region can be generated, `continue` needs no `Assign`; but
-                // without one, the `InstrumentCoverage` MIR pass cannot generate a code region for
-                // `continue`. Coverage will be missing unless we add a dummy `Assign` to MIR.
-                self.add_dummy_assignment(span, block, source_info);
+            (None, None) => {
+                if self.tcx.sess.instrument_coverage() {
+                    // Normally we wouldn't build any MIR in this case, but that makes it
+                    // harder for coverage instrumentation to extract a relevant span for
+                    // `continue` expressions. So here we inject a dummy statement with the
+                    // desired span.
+                    self.cfg.push_coverage_span_marker(block, source_info);
+                }
             }
-            (None, None) => {}
         }
 
         let region_scope = self.scopes.breakable_scopes[break_index].region_scope;
@@ -723,14 +723,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         self.cfg.terminate(block, source_info, TerminatorKind::UnwindResume);
     }
 
-    // Add a dummy `Assign` statement to the CFG, with the span for the source code's `continue`
-    // statement.
-    fn add_dummy_assignment(&mut self, span: Span, block: BasicBlock, source_info: SourceInfo) {
-        let local_decl = LocalDecl::new(Ty::new_unit(self.tcx), span);
-        let temp_place = Place::from(self.local_decls.push(local_decl));
-        self.cfg.push_assign_unit(block, source_info, temp_place, self.tcx);
-    }
-
     fn leave_top_scope(&mut self, block: BasicBlock) -> BasicBlock {
         // If we are emitting a `drop` statement, we need to have the cached
         // diverge cleanup pads ready in case that drop panics.

compiler/rustc_mir_transform/src/coroutine.rs

@@ -252,15 +252,15 @@ struct TransformVisitor<'tcx> {
 
 impl<'tcx> TransformVisitor<'tcx> {
     fn insert_none_ret_block(&self, body: &mut Body<'tcx>) -> BasicBlock {
-        assert!(matches!(self.coroutine_kind, CoroutineKind::Gen(_)));
-
         let block = BasicBlock::new(body.basic_blocks.len());
         let source_info = SourceInfo::outermost(body.span);
-        let option_def_id = self.tcx.require_lang_item(LangItem::Option, None);
 
-        let statements = vec![Statement {
-            kind: StatementKind::Assign(Box::new((
-                Place::return_place(),
+        let none_value = match self.coroutine_kind {
+            CoroutineKind::Async(_) => span_bug!(body.span, "`Future`s are not fused inherently"),
+            CoroutineKind::Coroutine => span_bug!(body.span, "`Coroutine`s cannot be fused"),
+            // `gen` continues return `None`
+            CoroutineKind::Gen(_) => {
+                let option_def_id = self.tcx.require_lang_item(LangItem::Option, None);
                 Rvalue::Aggregate(
                     Box::new(AggregateKind::Adt(
                         option_def_id,
@@ -270,8 +270,29 @@ impl<'tcx> TransformVisitor<'tcx> {
                         None,
                     )),
                     IndexVec::new(),
-                ),
-            ))),
+                )
+            }
+            // `async gen` continues to return `Poll::Ready(None)`
+            CoroutineKind::AsyncGen(_) => {
+                let ty::Adt(_poll_adt, args) = *self.old_yield_ty.kind() else { bug!() };
+                let ty::Adt(_option_adt, args) = *args.type_at(0).kind() else { bug!() };
+                let yield_ty = args.type_at(0);
+                Rvalue::Use(Operand::Constant(Box::new(ConstOperand {
+                    span: source_info.span,
+                    const_: Const::Unevaluated(
+                        UnevaluatedConst::new(
+                            self.tcx.require_lang_item(LangItem::AsyncGenFinished, None),
+                            self.tcx.mk_args(&[yield_ty.into()]),
+                        ),
+                        self.old_yield_ty,
+                    ),
+                    user_ty: None,
+                })))
+            }
+        };
+
+        let statements = vec![Statement {
+            kind: StatementKind::Assign(Box::new((Place::return_place(), none_value))),
             source_info,
         }];
 
@@ -1393,11 +1414,12 @@ fn create_coroutine_resume_function<'tcx>(
 
     if can_return {
         let block = match coroutine_kind {
-            // FIXME(gen_blocks): Should `async gen` yield `None` when resumed once again?
-            CoroutineKind::Async(_) | CoroutineKind::AsyncGen(_) | CoroutineKind::Coroutine => {
+            CoroutineKind::Async(_) | CoroutineKind::Coroutine => {
                 insert_panic_block(tcx, body, ResumedAfterReturn(coroutine_kind))
             }
-            CoroutineKind::Gen(_) => transform.insert_none_ret_block(body),
+            CoroutineKind::AsyncGen(_) | CoroutineKind::Gen(_) => {
+                transform.insert_none_ret_block(body)
+            }
         };
         cases.insert(1, (RETURNED, block));
     }

compiler/rustc_mir_transform/src/coverage/spans.rs

@@ -319,29 +319,16 @@ impl<'a> CoverageSpansGenerator<'a> {
             }
         }
 
-        let prev = self.take_prev();
-        debug!("    AT END, adding last prev={prev:?}");
-
         // Drain any remaining dups into the output.
         for dup in self.pending_dups.drain(..) {
             debug!("    ...adding at least one pending dup={:?}", dup);
             self.refined_spans.push(dup);
         }
 
-        // Async functions wrap a closure that implements the body to be executed. The enclosing
-        // function is called and returns an `impl Future` without initially executing any of the
-        // body. To avoid showing the return from the enclosing function as a "covered" return from
-        // the closure, the enclosing function's `TerminatorKind::Return`s `CoverageSpan` is
-        // excluded. The closure's `Return` is the only one that will be counted. This provides
-        // adequate coverage, and more intuitive counts. (Avoids double-counting the closing brace
-        // of the function body.)
-        let body_ends_with_closure = if let Some(last_covspan) = self.refined_spans.last() {
-            last_covspan.is_closure && last_covspan.span.hi() == self.body_span.hi()
-        } else {
-            false
-        };
-
-        if !body_ends_with_closure {
+        // There is usually a final span remaining in `prev` after the loop ends,
+        // so add it to the output as well.
+        if let Some(prev) = self.some_prev.take() {
+            debug!("    AT END, adding last prev={prev:?}");
             self.refined_spans.push(prev);
         }
 
@@ -398,38 +385,36 @@ impl<'a> CoverageSpansGenerator<'a> {
         self.refined_spans.push(macro_name_cov);
     }
 
+    #[track_caller]
     fn curr(&self) -> &CoverageSpan {
-        self.some_curr
-            .as_ref()
-            .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr"))
+        self.some_curr.as_ref().unwrap_or_else(|| bug!("some_curr is None (curr)"))
     }
 
+    #[track_caller]
     fn curr_mut(&mut self) -> &mut CoverageSpan {
-        self.some_curr
-            .as_mut()
-            .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr"))
+        self.some_curr.as_mut().unwrap_or_else(|| bug!("some_curr is None (curr_mut)"))
     }
 
     /// If called, then the next call to `next_coverage_span()` will *not* update `prev` with the
     /// `curr` coverage span.
+    #[track_caller]
     fn take_curr(&mut self) -> CoverageSpan {
-        self.some_curr.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr"))
+        self.some_curr.take().unwrap_or_else(|| bug!("some_curr is None (take_curr)"))
     }
 
+    #[track_caller]
     fn prev(&self) -> &CoverageSpan {
-        self.some_prev
-            .as_ref()
-            .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev"))
+        self.some_prev.as_ref().unwrap_or_else(|| bug!("some_prev is None (prev)"))
     }
 
+    #[track_caller]
     fn prev_mut(&mut self) -> &mut CoverageSpan {
-        self.some_prev
-            .as_mut()
-            .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev"))
+        self.some_prev.as_mut().unwrap_or_else(|| bug!("some_prev is None (prev_mut)"))
     }
 
+    #[track_caller]
     fn take_prev(&mut self) -> CoverageSpan {
-        self.some_prev.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev"))
+        self.some_prev.take().unwrap_or_else(|| bug!("some_prev is None (take_prev)"))
     }
 
     /// If there are `pending_dups` but `prev` is not a matching dup (`prev.span` doesn't match the