From 7660d6bf2c4e02b4f809b99f35c0f290e6171d45 Mon Sep 17 00:00:00 2001 From: Frank King Date: Sat, 6 Jan 2024 18:22:37 +0800 Subject: [PATCH] Check representation of unnamed fields --- compiler/rustc_hir_analysis/messages.ftl | 11 ++ .../rustc_hir_analysis/src/check/check.rs | 49 ++++++++ compiler/rustc_hir_analysis/src/collect.rs | 6 +- compiler/rustc_hir_analysis/src/errors.rs | 30 +++++ compiler/rustc_resolve/src/def_collector.rs | 31 ++--- .../src/traits/select/mod.rs | 8 +- ...d_access.bar.SimplifyCfg-initial.after.mir | 6 +- ...d_access.foo.SimplifyCfg-initial.after.mir | 6 +- .../feature-gate-unnamed_fields.rs | 5 + .../feature-gate-unnamed_fields.stderr | 10 +- tests/ui/union/unnamed-fields/repr_check.rs | 69 +++++++++++ .../ui/union/unnamed-fields/repr_check.stderr | 108 ++++++++++++++++++ 12 files changed, 309 insertions(+), 30 deletions(-) create mode 100644 tests/ui/union/unnamed-fields/repr_check.rs create mode 100644 tests/ui/union/unnamed-fields/repr_check.stderr diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index 662d859e993..cf1a42c9285 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -439,6 +439,17 @@ hir_analysis_typeof_reserved_keyword_used = hir_analysis_unconstrained_opaque_type = unconstrained opaque type .note = `{$name}` must be used in combination with a concrete type within the same {$what} +hir_analysis_unnamed_fields_repr_field_defined = unnamed field defined here + +hir_analysis_unnamed_fields_repr_field_missing_repr_c = + named type of unnamed field must have `#[repr(C)]` representation + .label = unnamed field defined here + .field_ty_label = `{$field_ty}` defined here + +hir_analysis_unnamed_fields_repr_missing_repr_c = + {$adt_kind} with unnamed fields must have `#[repr(C)]` representation + .label = {$adt_kind} `{$adt_name}` defined here + hir_analysis_unrecognized_atomic_operation = unrecognized atomic operation function: `{$op}` .label = unrecognized atomic operation diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 7250dc81faf..3cbd67a482c 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -80,6 +80,7 @@ fn check_struct(tcx: TyCtxt<'_>, def_id: LocalDefId) { check_transparent(tcx, def); check_packed(tcx, span, def); + check_unnamed_fields(tcx, def); } fn check_union(tcx: TyCtxt<'_>, def_id: LocalDefId) { @@ -89,6 +90,54 @@ fn check_union(tcx: TyCtxt<'_>, def_id: LocalDefId) { check_transparent(tcx, def); check_union_fields(tcx, span, def_id); check_packed(tcx, span, def); + check_unnamed_fields(tcx, def); +} + +/// Check the representation of adts with unnamed fields. +fn check_unnamed_fields(tcx: TyCtxt<'_>, def: ty::AdtDef<'_>) { + if def.is_enum() { + return; + } + let variant = def.non_enum_variant(); + if !variant.has_unnamed_fields() { + return; + } + if !def.is_anonymous() { + let adt_kind = def.descr(); + let span = tcx.def_span(def.did()); + let unnamed_fields = variant + .fields + .iter() + .filter(|f| f.is_unnamed()) + .map(|f| { + let span = tcx.def_span(f.did); + errors::UnnamedFieldsReprFieldDefined { span } + }) + .collect::>(); + debug_assert_ne!(unnamed_fields.len(), 0, "expect unnamed fields in this adt"); + let adt_name = tcx.item_name(def.did()); + if !def.repr().c() { + tcx.dcx().emit_err(errors::UnnamedFieldsRepr::MissingReprC { + span, + adt_kind, + adt_name, + unnamed_fields, + }); + } + } + for field in variant.fields.iter().filter(|f| f.is_unnamed()) { + let field_ty = tcx.type_of(field.did).instantiate_identity(); + if let Some(adt) = field_ty.ty_adt_def() + && !adt.is_anonymous() + && !adt.repr().c() + { + tcx.dcx().emit_err(errors::UnnamedFieldsRepr::FieldMissingReprC { + span: tcx.def_span(field.did), + field_ty_span: tcx.def_span(adt.did()), + field_ty, + }); + } + } } /// Check that the fields of the `union` do not need dropping. diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 15ddbd84b2c..45938eacd65 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -997,7 +997,11 @@ fn adt_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::AdtDef<'_> { }; let is_anonymous = item.ident.name == kw::Empty; - let repr = tcx.repr_options_of_def(def_id.to_def_id()); + let repr = if is_anonymous { + tcx.adt_def(tcx.local_parent(def_id)).repr() + } else { + tcx.repr_options_of_def(def_id.to_def_id()) + }; let (kind, variants) = match &item.kind { ItemKind::Enum(def, _) => { let mut distance_from_explicit = 0; diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 1e172f1d9f0..f7fa3263eaa 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -1571,3 +1571,33 @@ pub(crate) enum UnusedGenericParameterHelp { #[help(hir_analysis_unused_generic_parameter_ty_alias_help)] TyAlias { param_name: Ident }, } + +#[derive(Diagnostic)] +pub enum UnnamedFieldsRepr<'a> { + #[diag(hir_analysis_unnamed_fields_repr_missing_repr_c)] + MissingReprC { + #[primary_span] + #[label] + span: Span, + adt_kind: &'static str, + adt_name: Symbol, + #[subdiagnostic] + unnamed_fields: Vec, + }, + #[diag(hir_analysis_unnamed_fields_repr_field_missing_repr_c)] + FieldMissingReprC { + #[primary_span] + #[label] + span: Span, + #[label(hir_analysis_field_ty_label)] + field_ty_span: Span, + field_ty: Ty<'a>, + }, +} + +#[derive(Subdiagnostic)] +#[note(hir_analysis_unnamed_fields_repr_field_defined)] +pub struct UnnamedFieldsReprFieldDefined { + #[primary_span] + pub span: Span, +} diff --git a/compiler/rustc_resolve/src/def_collector.rs b/compiler/rustc_resolve/src/def_collector.rs index 08b1c88873e..45aea585f97 100644 --- a/compiler/rustc_resolve/src/def_collector.rs +++ b/compiler/rustc_resolve/src/def_collector.rs @@ -80,6 +80,22 @@ impl<'a, 'b, 'tcx> DefCollector<'a, 'b, 'tcx> { let name = field.ident.map_or_else(|| sym::integer(index(self)), |ident| ident.name); let def = self.create_def(field.id, name, DefKind::Field, field.span); self.with_parent(def, |this| visit::walk_field_def(this, field)); + self.visit_anon_adt(&field.ty); + } + } + + fn visit_anon_adt(&mut self, ty: &'a Ty) { + let def_kind = match &ty.kind { + TyKind::AnonStruct(..) => DefKind::Struct, + TyKind::AnonUnion(..) => DefKind::Union, + _ => return, + }; + match &ty.kind { + TyKind::AnonStruct(node_id, _) | TyKind::AnonUnion(node_id, _) => { + let def_id = self.create_def(*node_id, kw::Empty, def_kind, ty.span); + self.with_parent(def_id, |this| visit::walk_ty(this, ty)); + } + _ => {} } } @@ -326,19 +342,8 @@ impl<'a, 'b, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'b, 'tcx> { fn visit_ty(&mut self, ty: &'a Ty) { match &ty.kind { TyKind::MacCall(..) => self.visit_macro_invoc(ty.id), - TyKind::AnonStruct(node_id, fields) | TyKind::AnonUnion(node_id, fields) => { - let def_kind = match &ty.kind { - TyKind::AnonStruct(..) => DefKind::Struct, - TyKind::AnonUnion(..) => DefKind::Union, - _ => unreachable!(), - }; - let def_id = self.create_def(*node_id, kw::Empty, def_kind, ty.span); - self.with_parent(def_id, |this| { - for f in fields { - this.visit_field_def(f); - } - }); - } + // Anonymous structs or unions are visited later after defined. + TyKind::AnonStruct(..) | TyKind::AnonUnion(..) => {} _ => visit::walk_ty(self, ty), } } diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 43ab476056a..60453f5ab9c 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -2216,11 +2216,9 @@ impl<'tcx> SelectionContext<'_, 'tcx> { // if all of its fields are `Copy` and `Clone` ty::Adt(adt, args) if adt.is_anonymous() => { // (*) binder moved here - Where( - obligation - .predicate - .rebind(adt.non_enum_variant().fields.iter().map(|f| f.ty(self.tcx(), args)).collect()), - ) + Where(obligation.predicate.rebind( + adt.non_enum_variant().fields.iter().map(|f| f.ty(self.tcx(), args)).collect(), + )) } ty::Adt(..) | ty::Alias(..) | ty::Param(..) | ty::Placeholder(..) => { diff --git a/tests/mir-opt/unnamed-fields/field_access.bar.SimplifyCfg-initial.after.mir b/tests/mir-opt/unnamed-fields/field_access.bar.SimplifyCfg-initial.after.mir index 2a57b272f9b..8edc7b5df88 100644 --- a/tests/mir-opt/unnamed-fields/field_access.bar.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/unnamed-fields/field_access.bar.SimplifyCfg-initial.after.mir @@ -26,7 +26,7 @@ fn bar(_1: Bar) -> () { StorageDead(_2); StorageLive(_4); StorageLive(_5); - _5 = ((_1.1: Bar::_::{anon_adt#0}).0: i8); + _5 = ((_1.1: Bar::{anon_adt#0}).0: i8); _4 = access::(move _5) -> [return: bb2, unwind: bb5]; } @@ -35,7 +35,7 @@ fn bar(_1: Bar) -> () { StorageDead(_4); StorageLive(_6); StorageLive(_7); - _7 = ((_1.1: Bar::_::{anon_adt#0}).1: bool); + _7 = ((_1.1: Bar::{anon_adt#0}).1: bool); _6 = access::(move _7) -> [return: bb3, unwind: bb5]; } @@ -44,7 +44,7 @@ fn bar(_1: Bar) -> () { StorageDead(_6); StorageLive(_8); StorageLive(_9); - _9 = (((_1.2: Bar::_::{anon_adt#0}).0: Bar::_::{anon_adt#0}::_::{anon_adt#0}).0: [u8; 1]); + _9 = (((_1.2: Bar::{anon_adt#1}).0: Bar::{anon_adt#1}::{anon_adt#0}).0: [u8; 1]); _8 = access::<[u8; 1]>(move _9) -> [return: bb4, unwind: bb5]; } diff --git a/tests/mir-opt/unnamed-fields/field_access.foo.SimplifyCfg-initial.after.mir b/tests/mir-opt/unnamed-fields/field_access.foo.SimplifyCfg-initial.after.mir index 8b10a6f13ce..d48a969f06e 100644 --- a/tests/mir-opt/unnamed-fields/field_access.foo.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/unnamed-fields/field_access.foo.SimplifyCfg-initial.after.mir @@ -24,7 +24,7 @@ fn foo(_1: Foo) -> () { StorageDead(_2); StorageLive(_4); StorageLive(_5); - _5 = ((_1.1: Foo::_::{anon_adt#0}).0: i8); + _5 = ((_1.1: Foo::{anon_adt#0}).0: i8); _4 = access::(move _5) -> [return: bb2, unwind: bb5]; } @@ -33,7 +33,7 @@ fn foo(_1: Foo) -> () { StorageDead(_4); StorageLive(_6); StorageLive(_7); - _7 = ((_1.1: Foo::_::{anon_adt#0}).1: bool); + _7 = ((_1.1: Foo::{anon_adt#0}).1: bool); _6 = access::(move _7) -> [return: bb3, unwind: bb5]; } @@ -42,7 +42,7 @@ fn foo(_1: Foo) -> () { StorageDead(_6); StorageLive(_8); StorageLive(_9); - _9 = (((_1.2: Foo::_::{anon_adt#0}).0: Foo::_::{anon_adt#0}::_::{anon_adt#0}).0: [u8; 1]); + _9 = (((_1.2: Foo::{anon_adt#1}).0: Foo::{anon_adt#1}::{anon_adt#0}).0: [u8; 1]); _8 = access::<[u8; 1]>(move _9) -> [return: bb4, unwind: bb5]; } diff --git a/tests/ui/feature-gates/feature-gate-unnamed_fields.rs b/tests/ui/feature-gates/feature-gate-unnamed_fields.rs index 6ee8de89564..302a9bbeb45 100644 --- a/tests/ui/feature-gates/feature-gate-unnamed_fields.rs +++ b/tests/ui/feature-gates/feature-gate-unnamed_fields.rs @@ -1,3 +1,4 @@ +#[repr(C)] struct Foo { foo: u8, _: union { //~ ERROR unnamed fields are not yet fully implemented [E0658] @@ -7,6 +8,7 @@ struct Foo { } } +#[repr(C)] union Bar { foobar: u8, _: struct { //~ ERROR unnamed fields are not yet fully implemented [E0658] @@ -16,7 +18,10 @@ union Bar { } } +#[repr(C)] struct S; + +#[repr(C)] struct Baz { _: S //~ ERROR unnamed fields are not yet fully implemented [E0658] } diff --git a/tests/ui/feature-gates/feature-gate-unnamed_fields.stderr b/tests/ui/feature-gates/feature-gate-unnamed_fields.stderr index 8fa342c08ae..bc9e95bab98 100644 --- a/tests/ui/feature-gates/feature-gate-unnamed_fields.stderr +++ b/tests/ui/feature-gates/feature-gate-unnamed_fields.stderr @@ -1,5 +1,5 @@ error[E0658]: unnamed fields are not yet fully implemented - --> $DIR/feature-gate-unnamed_fields.rs:3:5 + --> $DIR/feature-gate-unnamed_fields.rs:4:5 | LL | _: union { | ^ @@ -9,7 +9,7 @@ LL | _: union { = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error[E0658]: unnamed fields are not yet fully implemented - --> $DIR/feature-gate-unnamed_fields.rs:3:8 + --> $DIR/feature-gate-unnamed_fields.rs:4:8 | LL | _: union { | ________^ @@ -24,7 +24,7 @@ LL | | } = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error[E0658]: unnamed fields are not yet fully implemented - --> $DIR/feature-gate-unnamed_fields.rs:12:5 + --> $DIR/feature-gate-unnamed_fields.rs:14:5 | LL | _: struct { | ^ @@ -34,7 +34,7 @@ LL | _: struct { = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error[E0658]: unnamed fields are not yet fully implemented - --> $DIR/feature-gate-unnamed_fields.rs:12:8 + --> $DIR/feature-gate-unnamed_fields.rs:14:8 | LL | _: struct { | ________^ @@ -49,7 +49,7 @@ LL | | } = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error[E0658]: unnamed fields are not yet fully implemented - --> $DIR/feature-gate-unnamed_fields.rs:21:5 + --> $DIR/feature-gate-unnamed_fields.rs:26:5 | LL | _: S | ^ diff --git a/tests/ui/union/unnamed-fields/repr_check.rs b/tests/ui/union/unnamed-fields/repr_check.rs new file mode 100644 index 00000000000..b50b54b20af --- /dev/null +++ b/tests/ui/union/unnamed-fields/repr_check.rs @@ -0,0 +1,69 @@ +#![allow(incomplete_features)] +#![feature(unnamed_fields)] + +struct A { //~ ERROR struct with unnamed fields must have `#[repr(C)]` representation + //~^ NOTE struct `A` defined here + _: struct { //~ NOTE unnamed field defined here + a: i32, + }, + _: struct { //~ NOTE unnamed field defined here + _: struct { + b: i32, + }, + }, +} + +union B { //~ ERROR union with unnamed fields must have `#[repr(C)]` representation + //~^ NOTE union `B` defined here + _: union { //~ NOTE unnamed field defined here + a: i32, + }, + _: union { //~ NOTE unnamed field defined here + _: union { + b: i32, + }, + }, +} + +#[derive(Clone, Copy)] +#[repr(C)] +struct Foo {} + +#[derive(Clone, Copy)] +struct Bar {} +//~^ `Bar` defined here +//~| `Bar` defined here +//~| `Bar` defined here +//~| `Bar` defined here + +struct C { //~ ERROR struct with unnamed fields must have `#[repr(C)]` representation + //~^ NOTE struct `C` defined here + _: Foo, //~ NOTE unnamed field defined here +} + +union D { //~ ERROR union with unnamed fields must have `#[repr(C)]` representation + //~^ NOTE union `D` defined here + _: Foo, //~ NOTE unnamed field defined here +} + +#[repr(C)] +struct E { + _: Bar, //~ ERROR named type of unnamed field must have `#[repr(C)]` representation + //~^ NOTE unnamed field defined here + _: struct { + _: Bar, //~ ERROR named type of unnamed field must have `#[repr(C)]` representation + //~^ NOTE unnamed field defined here + }, +} + +#[repr(C)] +union F { + _: Bar, //~ ERROR named type of unnamed field must have `#[repr(C)]` representation + //~^ NOTE unnamed field defined here + _: union { + _: Bar, //~ ERROR named type of unnamed field must have `#[repr(C)]` representation + //~^ NOTE unnamed field defined here + }, +} + +fn main() {} diff --git a/tests/ui/union/unnamed-fields/repr_check.stderr b/tests/ui/union/unnamed-fields/repr_check.stderr new file mode 100644 index 00000000000..ca6a1f3a780 --- /dev/null +++ b/tests/ui/union/unnamed-fields/repr_check.stderr @@ -0,0 +1,108 @@ +error: struct with unnamed fields must have `#[repr(C)]` representation + --> $DIR/repr_check.rs:4:1 + | +LL | struct A { + | ^^^^^^^^ struct `A` defined here + | +note: unnamed field defined here + --> $DIR/repr_check.rs:6:5 + | +LL | / _: struct { +LL | | a: i32, +LL | | }, + | |_____^ +note: unnamed field defined here + --> $DIR/repr_check.rs:9:5 + | +LL | / _: struct { +LL | | _: struct { +LL | | b: i32, +LL | | }, +LL | | }, + | |_____^ + +error: union with unnamed fields must have `#[repr(C)]` representation + --> $DIR/repr_check.rs:16:1 + | +LL | union B { + | ^^^^^^^ union `B` defined here + | +note: unnamed field defined here + --> $DIR/repr_check.rs:18:5 + | +LL | / _: union { +LL | | a: i32, +LL | | }, + | |_____^ +note: unnamed field defined here + --> $DIR/repr_check.rs:21:5 + | +LL | / _: union { +LL | | _: union { +LL | | b: i32, +LL | | }, +LL | | }, + | |_____^ + +error: struct with unnamed fields must have `#[repr(C)]` representation + --> $DIR/repr_check.rs:39:1 + | +LL | struct C { + | ^^^^^^^^ struct `C` defined here + | +note: unnamed field defined here + --> $DIR/repr_check.rs:41:5 + | +LL | _: Foo, + | ^^^^^^ + +error: union with unnamed fields must have `#[repr(C)]` representation + --> $DIR/repr_check.rs:44:1 + | +LL | union D { + | ^^^^^^^ union `D` defined here + | +note: unnamed field defined here + --> $DIR/repr_check.rs:46:5 + | +LL | _: Foo, + | ^^^^^^ + +error: named type of unnamed field must have `#[repr(C)]` representation + --> $DIR/repr_check.rs:51:5 + | +LL | struct Bar {} + | ---------- `Bar` defined here +... +LL | _: Bar, + | ^^^^^^ unnamed field defined here + +error: named type of unnamed field must have `#[repr(C)]` representation + --> $DIR/repr_check.rs:54:9 + | +LL | struct Bar {} + | ---------- `Bar` defined here +... +LL | _: Bar, + | ^^^^^^ unnamed field defined here + +error: named type of unnamed field must have `#[repr(C)]` representation + --> $DIR/repr_check.rs:61:5 + | +LL | struct Bar {} + | ---------- `Bar` defined here +... +LL | _: Bar, + | ^^^^^^ unnamed field defined here + +error: named type of unnamed field must have `#[repr(C)]` representation + --> $DIR/repr_check.rs:64:9 + | +LL | struct Bar {} + | ---------- `Bar` defined here +... +LL | _: Bar, + | ^^^^^^ unnamed field defined here + +error: aborting due to 8 previous errors +