From 985f756f9067b085adc38e18ef975b515ef416f4 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Mon, 19 Feb 2024 01:53:42 +0000 Subject: [PATCH 1/2] Use `expr_field` in builtin derives and assert that we aren't accessing `_` --- .../rustc_builtin_macros/src/deriving/generic/mod.rs | 12 +++++------- compiler/rustc_expand/src/build.rs | 1 + 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index 6eeb028728c9d..865963f702d38 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -1578,14 +1578,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_expand/src/build.rs b/compiler/rustc_expand/src/build.rs index 5879e025e7dbd..ff7535d57e888 100644 --- 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, field: Ident) -> P { + debug_assert_ne!(field.name, kw::Underscore); self.expr(span, ast::ExprKind::Field(expr, field)) } From c8635741812f7f4514ed1e1635fae523dfdf3bbf Mon Sep 17 00:00:00 2001 From: clubby789 Date: Mon, 19 Feb 2024 02:58:44 +0000 Subject: [PATCH 2/2] Prevent builtin derive macro (except Copy+Clone) usage on types with unnamed fields --- compiler/rustc_builtin_macros/messages.ftl | 6 ++ .../src/deriving/clone.rs | 71 +++++++++++-------- .../src/deriving/generic/mod.rs | 20 ++++++ .../src/deriving/generic/ty.rs | 5 ++ compiler/rustc_builtin_macros/src/errors.rs | 16 +++++ compiler/rustc_expand/src/build.rs | 3 + tests/ui/union/unnamed-fields/derive.rs | 29 ++++++++ tests/ui/union/unnamed-fields/derive.stderr | 23 ++++++ 8 files changed, 143 insertions(+), 30 deletions(-) create mode 100644 tests/ui/union/unnamed-fields/derive.rs create mode 100644 tests/ui/union/unnamed-fields/derive.stderr diff --git a/compiler/rustc_builtin_macros/messages.ftl b/compiler/rustc_builtin_macros/messages.ftl index dda466b026d91..62a4038ea34d3 100644 --- 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 index 267405ac32e24..42304f810e39a 100644 --- 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 index 865963f702d38..ead4e47fcadbf 100644 --- 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::>(); + 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, diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/ty.rs b/compiler/rustc_builtin_macros/src/deriving/generic/ty.rs index 603cefdd38629..82b428b885052 100644 --- 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 { 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 index 8d2e06bf30dac..9e04e24edb2f1 100644 --- 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, +} + +#[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 index ff7535d57e888..3e542daaf0bc0 100644 --- a/compiler/rustc_expand/src/build.rs +++ b/compiler/rustc_expand/src/build.rs @@ -324,6 +324,9 @@ impl<'a> ExtCtxt<'a> { is_placeholder: false, } } + pub fn expr_err(&self, span: Span) -> P { + 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 new file mode 100644 index 0000000000000..a5410bb88f1f5 --- /dev/null +++ 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 new file mode 100644 index 0000000000000..2507e5a450b6d --- /dev/null +++ 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 +