Auto merge of rust-lang#121270 - clubby789:more-unnamed-fields-checks, r=<try>

bors · bors · commit cc60b4c88e30 · 2024-02-19T04:23:25.000Z
Check for accessing unnamed field in `find_field` Fixes rust-lang#121263 I'm not entirely sure this is the right solution, but it seems to work 😓
diff --git a/compiler/rustc_builtin_macros/messages.ftl b/compiler/rustc_builtin_macros/messages.ftl
@@ -245,3 +245,9 @@ builtin_macros_unexpected_lit = expected path to a trait, found literal
     .other = for example, write `#[derive(Debug)]` for `Debug`
 
 builtin_macros_unnameable_test_items = cannot test inner items
+
+builtin_macros_unnamed_field_derive = only `Copy` and `Clone` may be derived on structs with unnamed fields
+    .note = unnamed field
+
+builtin_macros_unnamed_field_derive_clone =
+    deriving `Clone` on a type with unnamed fields requires also deriving `Copy`
diff --git a/compiler/rustc_builtin_macros/src/deriving/clone.rs b/compiler/rustc_builtin_macros/src/deriving/clone.rs
@@ -1,6 +1,7 @@
 use crate::deriving::generic::ty::*;
 use crate::deriving::generic::*;
 use crate::deriving::path_std;
+use crate::errors;
 use rustc_ast::{self as ast, Generics, ItemKind, MetaItem, VariantData};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_expand::base::{Annotatable, ExtCtxt};
@@ -32,42 +33,52 @@ pub fn expand_deriving_clone(
     let bounds;
     let substructure;
     let is_simple;
-    match item {
-        Annotatable::Item(annitem) => match &annitem.kind {
-            ItemKind::Struct(_, Generics { params, .. })
-            | ItemKind::Enum(_, Generics { params, .. }) => {
-                let container_id = cx.current_expansion.id.expn_data().parent.expect_local();
-                let has_derive_copy = cx.resolver.has_derive_copy(container_id);
-                if has_derive_copy
-                    && !params
-                        .iter()
-                        .any(|param| matches!(param.kind, ast::GenericParamKind::Type { .. }))
-                {
-                    bounds = vec![];
-                    is_simple = true;
-                    substructure = combine_substructure(Box::new(|c, s, sub| {
-                        cs_clone_simple("Clone", c, s, sub, false)
-                    }));
-                } else {
-                    bounds = vec![];
-                    is_simple = false;
-                    substructure =
-                        combine_substructure(Box::new(|c, s, sub| cs_clone("Clone", c, s, sub)));
-                }
-            }
-            ItemKind::Union(..) => {
-                bounds = vec![Path(path_std!(marker::Copy))];
+    let Annotatable::Item(annitem) = item else {
+        cx.dcx().span_bug(span, "`#[derive(Clone)]` on trait item or impl item")
+    };
+    let has_unnamed = if let ItemKind::Struct(VariantData::Struct { fields, .. }, _) = &annitem.kind
+    {
+        fields.iter().any(|f| f.ident.is_some_and(|i| i.name == kw::Underscore))
+    } else {
+        false
+    };
+    match &annitem.kind {
+        ItemKind::Struct(_, Generics { params, .. })
+        | ItemKind::Enum(_, Generics { params, .. }) => {
+            let container_id = cx.current_expansion.id.expn_data().parent.expect_local();
+            let has_derive_copy = cx.resolver.has_derive_copy(container_id);
+            if has_derive_copy
+                && !params
+                    .iter()
+                    .any(|param| matches!(param.kind, ast::GenericParamKind::Type { .. }))
+            {
+                bounds = vec![];
                 is_simple = true;
                 substructure = combine_substructure(Box::new(|c, s, sub| {
-                    cs_clone_simple("Clone", c, s, sub, true)
+                    cs_clone_simple("Clone", c, s, sub, false)
                 }));
+            } else {
+                bounds = vec![];
+                is_simple = false;
+                substructure =
+                    combine_substructure(Box::new(|c, s, sub| cs_clone("Clone", c, s, sub)));
             }
-            _ => cx.dcx().span_bug(span, "`#[derive(Clone)]` on wrong item kind"),
-        },
-
-        _ => cx.dcx().span_bug(span, "`#[derive(Clone)]` on trait item or impl item"),
+        }
+        ItemKind::Union(..) => {
+            bounds = vec![Path(path_std!(marker::Copy))];
+            is_simple = true;
+            substructure = combine_substructure(Box::new(|c, s, sub| {
+                cs_clone_simple("Clone", c, s, sub, true)
+            }));
+        }
+        _ => cx.dcx().span_bug(span, "`#[derive(Clone)]` on wrong item kind"),
     }
 
+    if !is_simple && has_unnamed {
+        cx.dcx().emit_err(errors::UnnamedFieldDeriveClone { span });
+        return;
+    };
+
     let trait_def = TraitDef {
         span,
         path: path_std!(clone::Clone),
diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
@@ -475,6 +475,26 @@ impl<'a> TraitDef<'a> {
                     false
                 });
 
+                if let ast::ItemKind::Struct(struct_def, _) | ast::ItemKind::Union(struct_def, _) =
+                    &item.kind
+                    && let Some(&trait_name) = self.path.components().last()
+                    && !(trait_name == sym::Copy || trait_name == sym::Clone)
+                {
+                    let unnamed = struct_def
+                        .fields()
+                        .iter()
+                        .filter(|f| f.ident.is_some_and(|i| i.name == kw::Underscore))
+                        .map(|f| f.span)
+                        .collect::<Vec<_>>();
+                    if !unnamed.is_empty() {
+                        cx.dcx().emit_err(errors::UnnamedFieldDerive {
+                            span: self.span,
+                            fields: unnamed,
+                        });
+                        return;
+                    }
+                };
+
                 let newitem = match &item.kind {
                     ast::ItemKind::Struct(struct_def, generics) => self.expand_struct_def(
                         cx,
@@ -1578,14 +1598,12 @@ impl<'a> TraitDef<'a> {
                     // `unwrap_or_else` case otherwise the hygiene is wrong and we get
                     // "field `0` of struct `Point` is private" errors on tuple
                     // structs.
-                    let mut field_expr = cx.expr(
+                    let mut field_expr = cx.expr_field(
                         sp,
-                        ast::ExprKind::Field(
-                            selflike_arg.clone(),
-                            struct_field.ident.unwrap_or_else(|| {
-                                Ident::from_str_and_span(&i.to_string(), struct_field.span)
-                            }),
-                        ),
+                        selflike_arg.clone(),
+                        struct_field.ident.unwrap_or_else(|| {
+                            Ident::from_str_and_span(&i.to_string(), struct_field.span)
+                        }),
                     );
                     if is_packed {
                         // In general, fields in packed structs are copied via a
diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/ty.rs b/compiler/rustc_builtin_macros/src/deriving/generic/ty.rs
@@ -48,6 +48,7 @@ impl Path {
     ) -> P<ast::Ty> {
         cx.ty_path(self.to_path(cx, span, self_ty, self_generics))
     }
+
     pub fn to_path(
         &self,
         cx: &ExtCtxt<'_>,
@@ -69,6 +70,10 @@ impl Path {
             }
         }
     }
+
+    pub fn components(&self) -> &[Symbol] {
+        &self.path
+    }
 }
 
 /// A type. Supports pointers, Self, and literals.
diff --git a/compiler/rustc_builtin_macros/src/errors.rs b/compiler/rustc_builtin_macros/src/errors.rs
@@ -842,6 +842,22 @@ pub(crate) struct TestRunnerNargs {
     pub(crate) span: Span,
 }
 
+#[derive(Diagnostic)]
+#[diag(builtin_macros_unnamed_field_derive)]
+pub(crate) struct UnnamedFieldDerive {
+    #[primary_span]
+    pub(crate) span: Span,
+    #[note]
+    pub(crate) fields: Vec<Span>,
+}
+
+#[derive(Diagnostic)]
+#[diag(builtin_macros_unnamed_field_derive_clone)]
+pub(crate) struct UnnamedFieldDeriveClone {
+    #[primary_span]
+    pub(crate) span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(builtin_macros_expected_register_class_or_explicit_register)]
 pub(crate) struct ExpectedRegisterClassOrExplicitRegister {
diff --git a/compiler/rustc_expand/src/build.rs b/compiler/rustc_expand/src/build.rs
@@ -259,6 +259,7 @@ impl<'a> ExtCtxt<'a> {
     }
 
     pub fn expr_field(&self, span: Span, expr: P<Expr>, field: Ident) -> P<ast::Expr> {
+        debug_assert_ne!(field.name, kw::Underscore);
         self.expr(span, ast::ExprKind::Field(expr, field))
     }
 
@@ -323,6 +324,9 @@ impl<'a> ExtCtxt<'a> {
             is_placeholder: false,
         }
     }
+    pub fn expr_err(&self, span: Span) -> P<ast::Expr> {
+        self.expr(span, ast::ExprKind::Err)
+    }
     pub fn expr_struct(
         &self,
         span: Span,
diff --git a/tests/ui/union/unnamed-fields/derive.rs b/tests/ui/union/unnamed-fields/derive.rs
@@ -0,0 +1,29 @@
+#![allow(incomplete_features)]
+#![feature(unnamed_fields)]
+
+#[derive(Clone, Copy, Debug)]
+#[repr(C)]
+struct Foo {
+    a: u8,
+}
+
+#[repr(C)]
+#[derive(Debug)]  //~ ERROR only `Copy` and `Clone` may be derived on structs with unnamed fields
+struct TestUnsupported {
+    _: Foo,
+}
+
+#[repr(C)]
+#[derive(Clone, Copy)]
+struct Test {
+    _: Foo,
+}
+
+#[repr(C)]
+#[derive(Clone)]  //~ ERROR deriving `Clone` on a type with unnamed fields requires also deriving `Copy`
+struct TestClone {
+     _: Foo,
+}
+
+
+fn main() {}
diff --git a/tests/ui/union/unnamed-fields/derive.stderr b/tests/ui/union/unnamed-fields/derive.stderr
@@ -0,0 +1,23 @@
+error: only `Copy` and `Clone` may be derived on structs with unnamed fields
+  --> $DIR/derive.rs:11:10
+   |
+LL | #[derive(Debug)]
+   |          ^^^^^
+   |
+note: unnamed field
+  --> $DIR/derive.rs:13:5
+   |
+LL |     _: Foo,
+   |     ^^^^^^
+   = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: deriving `Clone` on a type with unnamed fields requires also deriving `Copy`
+  --> $DIR/derive.rs:23:10
+   |
+LL | #[derive(Clone)]
+   |          ^^^^^
+   |
+   = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: aborting due to 2 previous errors
+

Original file line number	Diff line number	Diff line change
`@@ -48,6 +48,7 @@ impl Path {`
`48`	`48`	`) -> P<ast::Ty> {`
`49`	`49`	`cx.ty_path(self.to_path(cx, span, self_ty, self_generics))`
`50`	`50`	`}`
	`51`	`+`
`51`	`52`	`pub fn to_path(`
`52`	`53`	`&self,`
`53`	`54`	`cx: &ExtCtxt<'_>,`
`@@ -69,6 +70,10 @@ impl Path {`
`69`	`70`	`}`
`70`	`71`	`}`
`71`	`72`	`}`
	`73`	`+`
	`74`	`+ pub fn components(&self) -> &[Symbol] {`
	`75`	`+ &self.path`
	`76`	`+ }`
`72`	`77`	`}`
`73`	`78`
`74`	`79`	`/// A type. Supports pointers, Self, and literals.`
Original file line number	Diff line number	Diff line change
`@@ -259,6 +259,7 @@ impl<'a> ExtCtxt<'a> {`
`259`	`259`	`}`
`260`	`260`
`261`	`261`	`pub fn expr_field(&self, span: Span, expr: P<Expr>, field: Ident) -> P<ast::Expr> {`
	`262`	`+ debug_assert_ne!(field.name, kw::Underscore);`
`262`	`263`	`self.expr(span, ast::ExprKind::Field(expr, field))`
`263`	`264`	`}`
`264`	`265`
`@@ -323,6 +324,9 @@ impl<'a> ExtCtxt<'a> {`
`323`	`324`	`is_placeholder: false,`
`324`	`325`	`}`
`325`	`326`	`}`
	`327`	`+ pub fn expr_err(&self, span: Span) -> P<ast::Expr> {`
	`328`	`+ self.expr(span, ast::ExprKind::Err)`
	`329`	`+ }`
`326`	`330`	`pub fn expr_struct(`
`327`	`331`	`&self,`
`328`	`332`	`span: Span,`