Auto merge of #123455 - matthiaskrgr:rollup-b6nu296, r=matthiaskrgr

Rollup of 9 pull requests

Successful merges:

 - #121546 (Error out of layout calculation if a non-last struct field is unsized)
 - #122448 (Port hir-tree run-make test to ui test)
 - #123212 (CFI: Change type transformation to use TypeFolder)
 - #123218 (Add test for getting parent HIR for synthetic HIR node)
 - #123324 (match lowering: make false edges more precise)
 - #123389 (Avoid panicking unnecessarily on startup)
 - #123397 (Fix diagnostic for qualifier in extern block)
 - #123431 (Stabilize `proc_macro_byte_character` and `proc_macro_c_str_literals`)
 - #123439 (coverage: Remove useless constants)

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

compiler/rustc_ast/src/ast.rs

@@ -2484,6 +2484,14 @@ pub enum CoroutineKind {
 }
 
 impl CoroutineKind {
+    pub fn span(self) -> Span {
+        match self {
+            CoroutineKind::Async { span, .. } => span,
+            CoroutineKind::Gen { span, .. } => span,
+            CoroutineKind::AsyncGen { span, .. } => span,
+        }
+    }
+
     pub fn is_async(self) -> bool {
         matches!(self, CoroutineKind::Async { .. })
     }

compiler/rustc_ast_passes/messages.ftl

@@ -68,7 +68,7 @@ ast_passes_extern_block_suggestion = if you meant to declare an externally defin
 
 ast_passes_extern_fn_qualifiers = functions in `extern` blocks cannot have qualifiers
     .label = in this `extern` block
-    .suggestion = remove the qualifiers
+    .suggestion = remove this qualifier
 
 ast_passes_extern_item_ascii = items in `extern` blocks cannot use non-ascii identifiers
     .label = in this `extern` block

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -514,13 +514,32 @@ impl<'a> AstValidator<'a> {
     }
 
     /// An `fn` in `extern { ... }` cannot have qualifiers, e.g. `async fn`.
-    fn check_foreign_fn_headerless(&self, ident: Ident, span: Span, header: FnHeader) {
-        if header.has_qualifiers() {
+    fn check_foreign_fn_headerless(
+        &self,
+        // Deconstruct to ensure exhaustiveness
+        FnHeader { unsafety, coroutine_kind, constness, ext }: FnHeader,
+    ) {
+        let report_err = |span| {
             self.dcx().emit_err(errors::FnQualifierInExtern {
-                span: ident.span,
+                span: span,
                 block: self.current_extern_span(),
-                sugg_span: span.until(ident.span.shrink_to_lo()),
             });
+        };
+        match unsafety {
+            Unsafe::Yes(span) => report_err(span),
+            Unsafe::No => (),
+        }
+        match coroutine_kind {
+            Some(knd) => report_err(knd.span()),
+            None => (),
+        }
+        match constness {
+            Const::Yes(span) => report_err(span),
+            Const::No => (),
+        }
+        match ext {
+            Extern::None => (),
+            Extern::Implicit(span) | Extern::Explicit(_, span) => report_err(span),
         }
     }
 
@@ -1145,7 +1164,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
             ForeignItemKind::Fn(box Fn { defaultness, sig, body, .. }) => {
                 self.check_defaultness(fi.span, *defaultness);
                 self.check_foreign_fn_bodyless(fi.ident, body.as_deref());
-                self.check_foreign_fn_headerless(fi.ident, fi.span, sig.header);
+                self.check_foreign_fn_headerless(sig.header);
                 self.check_foreign_item_ascii_only(fi.ident);
             }
             ForeignItemKind::TyAlias(box TyAlias {

compiler/rustc_ast_passes/src/errors.rs

@@ -270,11 +270,10 @@ pub struct FnBodyInExtern {
 #[diag(ast_passes_extern_fn_qualifiers)]
 pub struct FnQualifierInExtern {
     #[primary_span]
+    #[suggestion(code = "", applicability = "maybe-incorrect")]
     pub span: Span,
     #[label]
     pub block: Span,
-    #[suggestion(code = "fn ", applicability = "maybe-incorrect", style = "verbose")]
-    pub sugg_span: Span,
 }
 
 #[derive(Diagnostic)]

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

@@ -24,8 +24,6 @@ pub(crate) mod ffi;
 pub(crate) mod map_data;
 pub mod mapgen;
 
-const VAR_ALIGN: Align = Align::EIGHT;
-
 /// A context object for maintaining all state needed by the coverageinfo module.
 pub struct CrateCoverageContext<'ll, 'tcx> {
     /// Coverage data for each instrumented function identified by DefId.
@@ -226,7 +224,8 @@ pub(crate) fn save_cov_data_to_mod<'ll, 'tcx>(
     llvm::set_global_constant(llglobal, true);
     llvm::set_linkage(llglobal, llvm::Linkage::PrivateLinkage);
     llvm::set_section(llglobal, &covmap_section_name);
-    llvm::set_alignment(llglobal, VAR_ALIGN);
+    // LLVM's coverage mapping format specifies 8-byte alignment for items in this section.
+    llvm::set_alignment(llglobal, Align::EIGHT);
     cx.add_used_global(llglobal);
 }
 
@@ -256,7 +255,8 @@ pub(crate) fn save_func_record_to_mod<'ll, 'tcx>(
     llvm::set_linkage(llglobal, llvm::Linkage::LinkOnceODRLinkage);
     llvm::set_visibility(llglobal, llvm::Visibility::Hidden);
     llvm::set_section(llglobal, covfun_section_name);
-    llvm::set_alignment(llglobal, VAR_ALIGN);
+    // LLVM's coverage mapping format specifies 8-byte alignment for items in this section.
+    llvm::set_alignment(llglobal, Align::EIGHT);
     llvm::set_comdat(cx.llmod, llglobal, &func_record_var_name);
     cx.add_used_global(llglobal);
 }

compiler/rustc_middle/src/mir/coverage.rs

@@ -33,10 +33,6 @@ rustc_index::newtype_index! {
     pub struct CounterId {}
 }
 
-impl CounterId {
-    pub const START: Self = Self::ZERO;
-}
-
 rustc_index::newtype_index! {
     /// ID of a coverage-counter expression. Values ascend from 0.
     ///
@@ -55,10 +51,6 @@ rustc_index::newtype_index! {
     pub struct ExpressionId {}
 }
 
-impl ExpressionId {
-    pub const START: Self = Self::ZERO;
-}
-
 /// Enum that can hold a constant zero value, the ID of an physical coverage
 /// counter, or the ID of a coverage-counter expression.
 ///

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

@@ -214,12 +214,77 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     ///
     /// ## False edges
     ///
-    /// We don't want to have the exact structure of the decision tree be
-    /// visible through borrow checking. False edges ensure that the CFG as
-    /// seen by borrow checking doesn't encode this. False edges are added:
+    /// We don't want to have the exact structure of the decision tree be visible through borrow
+    /// checking. Specifically we want borrowck to think that:
+    /// - at any point, any or none of the patterns and guards seen so far may have been tested;
+    /// - after the match, any of the patterns may have matched.
     ///
-    /// * From each pre-binding block to the next pre-binding block.
-    /// * From each otherwise block to the next pre-binding block.
+    /// For example, all of these would fail to error if borrowck could see the real CFG (examples
+    /// taken from `tests/ui/nll/match-cfg-fake-edges.rs`):
+    /// ```ignore (too many errors, this is already in the test suite)
+    /// let x = String::new();
+    /// let _ = match true {
+    ///     _ => {},
+    ///     _ => drop(x),
+    /// };
+    /// // Borrowck must not know the second arm is never run.
+    /// drop(x); //~ ERROR use of moved value
+    ///
+    /// let x;
+    /// # let y = true;
+    /// match y {
+    ///     _ if { x = 2; true } => {},
+    ///     // Borrowck must not know the guard is always run.
+    ///     _ => drop(x), //~ ERROR used binding `x` is possibly-uninitialized
+    /// };
+    ///
+    /// let x = String::new();
+    /// # let y = true;
+    /// match y {
+    ///     false if { drop(x); true } => {},
+    ///     // Borrowck must not know the guard is not run in the `true` case.
+    ///     true => drop(x), //~ ERROR use of moved value: `x`
+    ///     false => {},
+    /// };
+    ///
+    /// # let mut y = (true, true);
+    /// let r = &mut y.1;
+    /// match y {
+    ///     //~^ ERROR cannot use `y.1` because it was mutably borrowed
+    ///     (false, true) => {}
+    ///     // Borrowck must not know we don't test `y.1` when `y.0` is `true`.
+    ///     (true, _) => drop(r),
+    ///     (false, _) => {}
+    /// };
+    /// ```
+    ///
+    /// We add false edges to act as if we were naively matching each arm in order. What we need is
+    /// a (fake) path from each candidate to the next, specifically from candidate C's pre-binding
+    /// block to next candidate D's pre-binding block. For maximum precision (needed for deref
+    /// patterns), we choose the earliest node on D's success path that doesn't also lead to C (to
+    /// avoid loops).
+    ///
+    /// This turns out to be easy to compute: that block is the `start_block` of the first call to
+    /// `match_candidates` where D is the first candidate in the list.
+    ///
+    /// For example:
+    /// ```rust
+    /// # let (x, y) = (true, true);
+    /// match (x, y) {
+    ///   (true, true) => 1,
+    ///   (false, true) => 2,
+    ///   (true, false) => 3,
+    ///   _ => 4,
+    /// }
+    /// # ;
+    /// ```
+    /// In this example, the pre-binding block of arm 1 has a false edge to the block for result
+    /// `false` of the first test on `x`. The other arms have false edges to the pre-binding blocks
+    /// of the next arm.
+    ///
+    /// On top of this, we also add a false edge from the otherwise_block of each guard to the
+    /// aforementioned start block of the next candidate, to ensure borrock doesn't rely on which
+    /// guards may have run.
     #[instrument(level = "debug", skip(self, arms))]
     pub(crate) fn match_expr(
         &mut self,
@@ -365,7 +430,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         for candidate in candidates {
             candidate.visit_leaves(|leaf_candidate| {
                 if let Some(ref mut prev) = previous_candidate {
-                    prev.next_candidate_pre_binding_block = leaf_candidate.pre_binding_block;
+                    assert!(leaf_candidate.false_edge_start_block.is_some());
+                    prev.next_candidate_start_block = leaf_candidate.false_edge_start_block;
                 }
                 previous_candidate = Some(leaf_candidate);
             });
@@ -1010,8 +1076,12 @@ struct Candidate<'pat, 'tcx> {
 
     /// The block before the `bindings` have been established.
     pre_binding_block: Option<BasicBlock>,
-    /// The pre-binding block of the next candidate.
-    next_candidate_pre_binding_block: Option<BasicBlock>,
+
+    /// The earliest block that has only candidates >= this one as descendents. Used for false
+    /// edges, see the doc for [`Builder::match_expr`].
+    false_edge_start_block: Option<BasicBlock>,
+    /// The `false_edge_start_block` of the next candidate.
+    next_candidate_start_block: Option<BasicBlock>,
 }
 
 impl<'tcx, 'pat> Candidate<'pat, 'tcx> {
@@ -1033,7 +1103,8 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> {
             or_span: None,
             otherwise_block: None,
             pre_binding_block: None,
-            next_candidate_pre_binding_block: None,
+            false_edge_start_block: None,
+            next_candidate_start_block: None,
         }
     }
 
@@ -1325,6 +1396,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         otherwise_block: BasicBlock,
         candidates: &mut [&mut Candidate<'_, 'tcx>],
     ) {
+        if let [first, ..] = candidates {
+            if first.false_edge_start_block.is_none() {
+                first.false_edge_start_block = Some(start_block);
+            }
+        }
+
         match candidates {
             [] => {
                 // If there are no candidates that still need testing, we're done. Since all matches are
@@ -1545,6 +1622,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             .into_iter()
             .map(|flat_pat| Candidate::from_flat_pat(flat_pat, candidate.has_guard))
             .collect();
+        candidate.subcandidates[0].false_edge_start_block = candidate.false_edge_start_block;
     }
 
     /// Try to merge all of the subcandidates of the given candidate into one. This avoids
@@ -1564,6 +1642,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             let any_matches = self.cfg.start_new_block();
             let or_span = candidate.or_span.take().unwrap();
             let source_info = self.source_info(or_span);
+            if candidate.false_edge_start_block.is_none() {
+                candidate.false_edge_start_block =
+                    candidate.subcandidates[0].false_edge_start_block;
+            }
             for subcandidate in mem::take(&mut candidate.subcandidates) {
                 let or_block = subcandidate.pre_binding_block.unwrap();
                 self.cfg.goto(or_block, source_info, any_matches);
@@ -1979,12 +2061,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
         let mut block = candidate.pre_binding_block.unwrap();
 
-        if candidate.next_candidate_pre_binding_block.is_some() {
+        if candidate.next_candidate_start_block.is_some() {
             let fresh_block = self.cfg.start_new_block();
             self.false_edges(
                 block,
                 fresh_block,
-                candidate.next_candidate_pre_binding_block,
+                candidate.next_candidate_start_block,
                 candidate_source_info,
             );
             block = fresh_block;
@@ -2132,7 +2214,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             self.false_edges(
                 otherwise_post_guard_block,
                 otherwise_block,
-                candidate.next_candidate_pre_binding_block,
+                candidate.next_candidate_start_block,
                 source_info,
             );
 

compiler/rustc_mir_transform/src/coverage/query.rs

@@ -59,7 +59,7 @@ fn coverage_ids_info<'tcx>(
             _ => None,
         })
         .max()
-        .unwrap_or(CounterId::START);
+        .unwrap_or(CounterId::ZERO);
 
     CoverageIdsInfo { max_counter_id }
 }