Skip to content

Commit

Permalink
Auto merge of #121270 - clubby789:more-unnamed-fields-checks, r=<try>
Browse files Browse the repository at this point in the history
Check for accessing unnamed field in `find_field`

Fixes #121263
I'm not entirely sure this is the right solution, but it seems to work 😓
  • Loading branch information
bors committed Feb 19, 2024
2 parents 6122397 + c863574 commit cc60b4c
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 37 deletions.
6 changes: 6 additions & 0 deletions compiler/rustc_builtin_macros/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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`
71 changes: 41 additions & 30 deletions compiler/rustc_builtin_macros/src/deriving/clone.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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),
Expand Down
32 changes: 25 additions & 7 deletions compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_builtin_macros/src/deriving/generic/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<'_>,
Expand All @@ -69,6 +70,10 @@ impl Path {
}
}
}

pub fn components(&self) -> &[Symbol] {
&self.path
}
}

/// A type. Supports pointers, Self, and literals.
Expand Down
16 changes: 16 additions & 0 deletions compiler/rustc_builtin_macros/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_expand/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down Expand Up @@ -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,
Expand Down
29 changes: 29 additions & 0 deletions tests/ui/union/unnamed-fields/derive.rs
Original file line number Diff line number Diff line change
@@ -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() {}
23 changes: 23 additions & 0 deletions tests/ui/union/unnamed-fields/derive.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit cc60b4c

Please sign in to comment.