Auto merge of rust-lang#157320 - JonathanBrouwer:rollup-ECUZfbo, r=JonathanBrouwer

Rollup of 6 pull requests

Successful merges:

 - rust-lang#155418 (transmute: fix check for whether newtypes have equal size)
 - rust-lang#156603 (Clarify E0381 diagnostics for branch conditions)
 - rust-lang#156643 (Document run-make external dependencies)
 - rust-lang#157009 (Avoid `unreachable_code` on required return values)
 - rust-lang#157308 (make typing mode exhaustive again...)
 - rust-lang#157312 (disallow most attrs on eiis)

Author: bors

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

@@ -836,7 +836,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
             && !visitor
                 .errors
                 .iter()
-                .map(|(sp, _)| *sp)
+                .map(|error| error.span)
                 .any(|sp| span < sp && !sp.contains(span))
         }) {
             show_assign_sugg = true;
@@ -865,8 +865,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
         err.span_label(span, format!("{path} {used} here but it {isnt_initialized}"));
 
         let mut shown = false;
-        for (sp, label) in visitor.errors {
-            if sp < span && !sp.overlaps(span) {
+        let mut shown_condition_value = false;
+        for error in visitor.errors {
+            if error.span < span && !error.span.overlaps(span) {
                 // When we have a case like `match-cfg-fake-edges.rs`, we don't want to mention
                 // match arms coming after the primary span because they aren't relevant:
                 // ```
@@ -880,7 +881,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 //     _ => {} // We don't want to point to this.
                 // };
                 // ```
-                err.span_label(sp, label);
+                shown_condition_value |= error.kind.describes_condition_value();
+                err.span_label(error.span, error.label);
                 shown = true;
             }
         }
@@ -893,6 +895,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
         }
 
         err.span_label(decl_span, "binding declared here but left uninitialized");
+        if shown_condition_value {
+            err.note(
+                "when checking initialization, the compiler describes possible control-flow paths \
+                 without evaluating whether branch conditions can actually have the values shown",
+            );
+        }
         if show_assign_sugg {
             struct LetVisitor {
                 decl_span: Span,
@@ -4680,7 +4688,31 @@ struct ConditionVisitor<'tcx> {
     tcx: TyCtxt<'tcx>,
     spans: Vec<Span>,
     name: String,
-    errors: Vec<(Span, String)>,
+    errors: Vec<ConditionError>,
+}
+
+struct ConditionError {
+    span: Span,
+    label: String,
+    kind: ConditionErrorKind,
+}
+
+impl ConditionError {
+    fn new(span: Span, kind: ConditionErrorKind, label: String) -> Self {
+        Self { span, label, kind }
+    }
+}
+
+#[derive(Clone, Copy)]
+enum ConditionErrorKind {
+    ConditionValue,
+    Other,
+}
+
+impl ConditionErrorKind {
+    fn describes_condition_value(self) -> bool {
+        matches!(self, Self::ConditionValue)
+    }
 }
 
 impl<'v, 'tcx> Visitor<'v> for ConditionVisitor<'tcx> {
@@ -4690,15 +4722,17 @@ impl<'v, 'tcx> Visitor<'v> for ConditionVisitor<'tcx> {
                 // `if` expressions with no `else` that initialize the binding might be missing an
                 // `else` arm.
                 if ReferencedStatementsVisitor(&self.spans).visit_expr(body).is_break() {
-                    self.errors.push((
+                    self.errors.push(ConditionError::new(
                         cond.span,
+                        ConditionErrorKind::ConditionValue,
                         format!(
                             "if this `if` condition is `false`, {} is not initialized",
                             self.name,
                         ),
                     ));
-                    self.errors.push((
+                    self.errors.push(ConditionError::new(
                         ex.span.shrink_to_hi(),
+                        ConditionErrorKind::Other,
                         format!("an `else` arm might be missing here, initializing {}", self.name),
                     ));
                 }
@@ -4712,17 +4746,19 @@ impl<'v, 'tcx> Visitor<'v> for ConditionVisitor<'tcx> {
                     (true, true) | (false, false) => {}
                     (true, false) => {
                         if other.span.is_desugaring(DesugaringKind::WhileLoop) {
-                            self.errors.push((
+                            self.errors.push(ConditionError::new(
                                 cond.span,
+                                ConditionErrorKind::ConditionValue,
                                 format!(
                                     "if this condition isn't met and the `while` loop runs 0 \
                                      times, {} is not initialized",
                                     self.name
                                 ),
                             ));
                         } else {
-                            self.errors.push((
+                            self.errors.push(ConditionError::new(
                                 body.span.shrink_to_hi().until(other.span),
+                                ConditionErrorKind::ConditionValue,
                                 format!(
                                     "if the `if` condition is `false` and this `else` arm is \
                                      executed, {} is not initialized",
@@ -4732,8 +4768,9 @@ impl<'v, 'tcx> Visitor<'v> for ConditionVisitor<'tcx> {
                         }
                     }
                     (false, true) => {
-                        self.errors.push((
+                        self.errors.push(ConditionError::new(
                             cond.span,
+                            ConditionErrorKind::ConditionValue,
                             format!(
                                 "if this condition is `true`, {} is not initialized",
                                 self.name
@@ -4753,8 +4790,9 @@ impl<'v, 'tcx> Visitor<'v> for ConditionVisitor<'tcx> {
                     for (arm, seen) in arms.iter().zip(results) {
                         if !seen {
                             if loop_desugar == hir::MatchSource::ForLoopDesugar {
-                                self.errors.push((
+                                self.errors.push(ConditionError::new(
                                     e.span,
+                                    ConditionErrorKind::Other,
                                     format!(
                                         "if the `for` loop runs 0 times, {} is not initialized",
                                         self.name
@@ -4767,8 +4805,9 @@ impl<'v, 'tcx> Visitor<'v> for ConditionVisitor<'tcx> {
                                 ) {
                                     continue;
                                 }
-                                self.errors.push((
+                                self.errors.push(ConditionError::new(
                                     arm.pat.span.to(guard.span),
+                                    ConditionErrorKind::ConditionValue,
                                     format!(
                                         "if this pattern and condition are matched, {} is not \
                                          initialized",
@@ -4782,8 +4821,9 @@ impl<'v, 'tcx> Visitor<'v> for ConditionVisitor<'tcx> {
                                 ) {
                                     continue;
                                 }
-                                self.errors.push((
+                                self.errors.push(ConditionError::new(
                                     arm.pat.span,
+                                    ConditionErrorKind::Other,
                                     format!(
                                         "if this pattern is matched, {} is not initialized",
                                         self.name

compiler/rustc_builtin_macros/src/eii.rs

@@ -10,10 +10,10 @@ use rustc_span::{ErrorGuaranteed, Ident, Span, kw, sym};
 use thin_vec::{ThinVec, thin_vec};
 
 use crate::errors::{
-    EiiExternTargetExpectedList, EiiExternTargetExpectedMacro, EiiExternTargetExpectedUnsafe,
-    EiiMacroExpectedMaxOneArgument, EiiOnlyOnce, EiiSharedMacroInStatementPosition,
-    EiiSharedMacroTarget, EiiStaticArgumentRequired, EiiStaticDefault,
-    EiiStaticMultipleImplementations, EiiStaticMutable,
+    EiiAttributeNotSupported, EiiExternTargetExpectedList, EiiExternTargetExpectedMacro,
+    EiiExternTargetExpectedUnsafe, EiiMacroExpectedMaxOneArgument, EiiOnlyOnce,
+    EiiSharedMacroInStatementPosition, EiiSharedMacroTarget, EiiStaticArgumentRequired,
+    EiiStaticDefault, EiiStaticMultipleImplementations, EiiStaticMutable,
 };
 
 /// ```rust
@@ -128,6 +128,8 @@ fn eii_(
 
     let attrs_from_decl =
         filter_attrs_for_multiple_eii_attr(ecx, attrs, eii_attr_span, &meta_item.path);
+    let (macro_attrs, foreign_item_attrs, default_func_attrs) =
+        split_attrs(ecx, item_span, attrs_from_decl);
 
     let Ok(macro_name) = name_for_impl_macro(ecx, foreign_item_name, &meta_item) else {
         // we don't need to wrap in Annotatable::Stmt conditionally since
@@ -148,6 +150,7 @@ fn eii_(
             eii_attr_span,
             item_span,
             foreign_item_name,
+            default_func_attrs,
         ))
     }
 
@@ -157,22 +160,65 @@ fn eii_(
         item_span,
         kind,
         vis,
-        &attrs_from_decl,
+        foreign_item_attrs,
     ));
     module_items.push(generate_attribute_macro_to_implement(
         ecx,
         eii_attr_span,
         macro_name,
         foreign_item_name,
         impl_unsafe,
-        &attrs_from_decl,
+        macro_attrs,
     ));
 
     // we don't need to wrap in Annotatable::Stmt conditionally since
     // EII can't be used on items in statement position
     module_items.into_iter().map(Annotatable::Item).collect()
 }
 
+fn split_attrs(
+    ecx: &mut ExtCtxt<'_>,
+    span: Span,
+    attrs: ThinVec<Attribute>,
+) -> (ThinVec<Attribute>, ThinVec<Attribute>, ThinVec<Attribute>) {
+    let mut macro_attributes = ThinVec::new();
+    let mut foreign_item_attributes = ThinVec::new();
+    let mut default_attributes = ThinVec::new();
+
+    for attr in attrs {
+        match attr.name() {
+            // Inline only matters for the default function being inlined into callsites
+            Some(sym::inline) => default_attributes.push(attr),
+            // If an eii is marked a lang item, that's because we want to call its declaration, so
+            // mark the foreign item as the lang item
+            Some(sym::lang) => foreign_item_attributes.push(attr),
+            // Deprecating an eii means deprecating the macro and the foreign item
+            Some(sym::deprecated) => {
+                foreign_item_attributes.push(attr.clone());
+                macro_attributes.push(attr);
+            }
+            // The stability of an EII affects the usage of the macro and calling the foreign item
+            Some(sym::stable) | Some(sym::unstable) => {
+                foreign_item_attributes.push(attr.clone());
+                macro_attributes.push(attr);
+            }
+            // Doc attributes should be forwarded to the macro and the foreign item, since those are
+            // the two items you interact with as a user.
+            // FIXME: idk yet how EIIs show up in docs, might want to customize
+            _ if attr.is_doc_comment() => {
+                foreign_item_attributes.push(attr.clone());
+                macro_attributes.push(attr);
+            }
+            Some(sym::eii) => unreachable!("should already be filtered out"),
+            _ => {
+                ecx.dcx().emit_err(EiiAttributeNotSupported { span, attr_span: attr.span() });
+            }
+        }
+    }
+
+    (macro_attributes, foreign_item_attributes, default_attributes)
+}
+
 /// Decide on the name of the macro that can be used to implement the EII.
 /// This is either an explicitly given name, or the name of the item in the
 /// declaration of the EII.
@@ -228,10 +274,8 @@ fn generate_default_func_impl(
     eii_attr_span: Span,
     item_span: Span,
     foreign_item_name: Ident,
+    attrs: ThinVec<Attribute>,
 ) -> Box<ast::Item> {
-    // FIXME: re-add some original attrs
-    let attrs = ThinVec::new();
-
     let mut default_func = func.clone();
     default_func.eii_impls.push(EiiImpl {
         node_id: DUMMY_NODE_ID,
@@ -289,10 +333,9 @@ fn generate_foreign_item(
     item_span: Span,
     item_kind: &ItemKind,
     vis: Visibility,
-    attrs_from_decl: &[Attribute],
+    attrs_from_decl: ThinVec<Attribute>,
 ) -> Box<ast::Item> {
-    let mut foreign_item_attrs = ThinVec::new();
-    foreign_item_attrs.extend_from_slice(attrs_from_decl);
+    let mut foreign_item_attrs = attrs_from_decl;
 
     // Add the rustc_eii_foreign_item on the foreign item. Usually, foreign items are mangled.
     // This attribute makes sure that we later know that this foreign item's symbol should not be.
@@ -381,13 +424,9 @@ fn generate_attribute_macro_to_implement(
     macro_name: Ident,
     foreign_item_name: Ident,
     impl_unsafe: bool,
-    attrs_from_decl: &[Attribute],
+    attrs_from_decl: ThinVec<Attribute>,
 ) -> Box<ast::Item> {
-    let mut macro_attrs = ThinVec::new();
-
-    // To avoid e.g. `error: attribute macro has missing stability attribute`
-    // errors for eii's in std.
-    macro_attrs.extend_from_slice(attrs_from_decl);
+    let mut macro_attrs = attrs_from_decl;
 
     // Avoid "missing stability attribute" errors for eiis in std. See #146993.
     macro_attrs.push(ecx.attr_name_value_str(sym::rustc_macro_transparency, sym::semiopaque, span));

compiler/rustc_builtin_macros/src/errors.rs

@@ -1186,6 +1186,15 @@ pub(crate) struct EiiMacroExpectedMaxOneArgument {
     pub name: String,
 }
 
+#[derive(Diagnostic)]
+#[diag("only a small subset of attributes are supported on externally implementable items")]
+pub(crate) struct EiiAttributeNotSupported {
+    #[primary_span]
+    pub span: Span,
+    #[note("this attribute is not supported")]
+    pub attr_span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag("named argument `{$named_arg_name}` is not used by name")]
 pub(crate) struct NamedArgumentUsedPositionally {

compiler/rustc_middle/src/ty/layout.rs

@@ -454,13 +454,42 @@ impl<'tcx> SizeSkeleton<'tcx> {
             }
 
             ty::Adt(def, args) => {
-                // Only newtypes and enums w/ nullable pointer optimization.
+                // Only newtypes and enums w/ nullable pointer optimization (NPO).
                 if def.is_union() || def.variants().is_empty() || def.variants().len() > 2 {
                     return Err(err);
                 }
+                // Only default repr types.
+                {
+                    // We can ignore the seed and some particular flags that can never affect the
+                    // layout of newtypes / NPO types, but we have to check everything else.
+                    // If you are adding a new field to `ReprOptions`, make sure to extend the check
+                    // below so that we bail out if it is not at its default value!
+                    let ReprOptions { int, align, pack, flags, scalable, field_shuffle_seed: _ } =
+                        def.repr();
+                    let mut ignored_flags = ReprFlags::IS_TRANSPARENT
+                        | ReprFlags::IS_LINEAR
+                        | ReprFlags::RANDOMIZE_LAYOUT;
+                    if def.is_struct() {
+                        // `repr(C)` is only okay for structs, not for enums.
+                        // Below, the *only* thing we do for structs is propagating
+                        // `SizeSkeleton::Pointer`. We do *not* assume that `repr(C)` preserved
+                        // ZST-ness (which might stop being true eventually).
+                        ignored_flags |= ReprFlags::IS_C;
+                    }
+                    if int.is_some()
+                        || align.is_some()
+                        || pack.is_some()
+                        || flags.difference(ignored_flags) != ReprFlags::default()
+                        || scalable.is_some()
+                    {
+                        return Err(err);
+                    }
+                }
 
                 // Get a zero-sized variant or a pointer newtype.
-                let zero_or_ptr_variant = |i| {
+                // Returns `Ok(None)` for 1-ZST types, `Ok(Some)` if (ignoring all 1-ZST fields)
+                // there's just a single pointer, and `Err` otherwise.
+                let zero_or_ptr_variant = |i| -> Result<Option<SizeSkeleton<'tcx>>, _> {
                     let i = VariantIdx::from_usize(i);
                     let fields = def.variant(i).fields.iter().map(|field| {
                         SizeSkeleton::compute_inner(
@@ -494,7 +523,8 @@ impl<'tcx> SizeSkeleton<'tcx> {
                 };
 
                 let v0 = zero_or_ptr_variant(0)?;
-                // Newtype.
+                // Single-variant case: Check if this is a newtype around a pointer.
+                // Such types are themselves pointer-sized.
                 if def.variants().len() == 1 {
                     if let Some(SizeSkeleton::Pointer { non_zero, tail }) = v0 {
                         return Ok(SizeSkeleton::Pointer { non_zero, tail });
@@ -504,7 +534,9 @@ impl<'tcx> SizeSkeleton<'tcx> {
                 }
 
                 let v1 = zero_or_ptr_variant(1)?;
-                // Nullable pointer enum optimization.
+                // 2-variant case: Check if one variant is a *non-zero* pointer and the other a
+                // 1-ZST. Such types are eligible to for the nullable pointer enum optimization, so
+                // they are themselves pointer-sized.
                 match (v0, v1) {
                     (Some(SizeSkeleton::Pointer { non_zero: true, tail }), None)
                     | (None, Some(SizeSkeleton::Pointer { non_zero: true, tail })) => {

compiler/rustc_mir_build/src/builder/mod.rs

@@ -890,6 +890,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     {
                         continue;
                     }
+                    // Ignore return value plumbing. After a call returning a non-`!`
+                    // uninhabited type, a tail expression can be unreachable while
+                    // still being needed to satisfy the surrounding return type.
+                    StatementKind::Assign((place, _)) if place.as_local() == Some(RETURN_PLACE) => {
+                        continue;
+                    }
                     StatementKind::StorageLive(_) | StatementKind::StorageDead(_) => {
                         continue;
                     }