bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #131045 - compiler-errors:remove-unnamed_fields, r=wesleywiser

Browse source 
Retire the `unnamed_fields` feature for now

`#![feature(unnamed_fields)]` was implemented in part in #115131 and #115367, however work on that feature has (afaict) stalled and in the mean time there have been some concerns raised (e.g.[^1][^2]) about whether `unnamed_fields` is worthwhile to have in the language, especially in its current desugaring. Because it represents a compiler implementation burden including a new kind of anonymous ADT and additional complication to field selection, and is quite prone to bugs today, I'm choosing to remove the feature.

However, since I'm not one to really write a bunch of words, I'm specifically *not* going to de-RFC this feature. This PR essentially *rolls back* the state of this feature to "RFC accepted but not yet implemented"; however if anyone wants to formally unapprove the RFC from the t-lang side, then please be my guest. I'm just not totally willing to summarize the various language-facing reasons for why this feature is or is not worthwhile, since I'm coming from the compiler side mostly.

Fixes #117942
Fixes #121161
Fixes #121263
Fixes #121299
Fixes #121722
Fixes #121799
Fixes #126969
Fixes #131041

Tracking:
* https://github.com/rust-lang/rust/issues/49804

[^1]: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Unnamed.20struct.2Funion.20fields
[^2]: https://github.com/rust-lang/rust/issues/49804#issuecomment-1972619108
This commit is contained in:
bors 2024-10-11 13:11:13 +00:00
commit f4966590d8
66 changed files with 32 additions and 3708 deletions
Download patch file Download diff file Expand all files Collapse all files

56
compiler/rustc_hir_analysis/src/check/check.rs
View file

@ -76,7 +76,6 @@ 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) {
@ -86,61 +85,6 @@ 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::<Vec<_>>();
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,
sugg_span: span.shrink_to_lo(),
});
}
}
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_enum()
{
if !adt.is_anonymous() && !adt.repr().c() {
let field_ty_span = tcx.def_span(adt.did());
tcx.dcx().emit_err(errors::UnnamedFieldsRepr::FieldMissingReprC {
span: tcx.def_span(field.did),
field_ty_span,
field_ty,
field_adt_kind: adt.descr(),
sugg_span: field_ty_span.shrink_to_lo(),
});
}
} else {
tcx.dcx().emit_err(errors::InvalidUnnamedFieldTy { span: tcx.def_span(field.did) });
}
}
}
/// Check that the fields of the `union` do not need dropping.

104
compiler/rustc_hir_analysis/src/collect.rs
View file

@ -1007,79 +1007,6 @@ impl<'tcx> FieldUniquenessCheckContext<'tcx> {
}
}
}
/// Check the uniqueness of fields across adt where there are
/// nested fields imported from an unnamed field.
fn check_field_in_nested_adt(&mut self, adt_def: ty::AdtDef<'_>, unnamed_field_span: Span) {
for field in adt_def.all_fields() {
if field.is_unnamed() {
// Here we don't care about the generic parameters, so `instantiate_identity` is enough.
match self.tcx.type_of(field.did).instantiate_identity().kind() {
ty::Adt(adt_def, _) => {
self.check_field_in_nested_adt(*adt_def, unnamed_field_span);
}
ty_kind => span_bug!(
self.tcx.def_span(field.did),
"Unexpected TyKind in FieldUniquenessCheckContext::check_field_in_nested_adt(): {ty_kind:?}"
),
}
} else {
self.check_field_decl(
field.ident(self.tcx),
NestedSpan {
span: unnamed_field_span,
nested_field_span: self.tcx.def_span(field.did),
}
.into(),
);
}
}
}
/// Check the uniqueness of fields in a struct variant, and recursively
/// check the nested fields if it is an unnamed field with type of an
/// anonymous adt.
fn check_field(&mut self, field: &hir::FieldDef<'_>) {
if field.ident.name != kw::Underscore {
self.check_field_decl(field.ident, field.span.into());
return;
}
match &field.ty.kind {
hir::TyKind::AnonAdt(item_id) => {
match &self.tcx.hir_node(item_id.hir_id()).expect_item().kind {
hir::ItemKind::Struct(variant_data, ..)
| hir::ItemKind::Union(variant_data, ..) => {
variant_data.fields().iter().for_each(|f| self.check_field(f));
}
item_kind => span_bug!(
field.ty.span,
"Unexpected ItemKind in FieldUniquenessCheckContext::check_field(): {item_kind:?}"
),
}
}
hir::TyKind::Path(hir::QPath::Resolved(_, hir::Path { res, .. })) => {
// If this is a direct path to an ADT, we can check it
// If this is a type alias or non-ADT, `check_unnamed_fields` should verify it
if let Some(def_id) = res.opt_def_id()
&& let Some(local) = def_id.as_local()
&& let Node::Item(item) = self.tcx.hir_node_by_def_id(local)
&& item.is_adt()
{
self.check_field_in_nested_adt(self.tcx.adt_def(def_id), field.span);
}
}
// Abort due to errors (there must be an error if an unnamed field
// has any type kind other than an anonymous adt or a named adt)
ty_kind => {
self.tcx.dcx().span_delayed_bug(
field.ty.span,
format!("Unexpected TyKind in FieldUniquenessCheckContext::check_field(): {ty_kind:?}"),
);
// FIXME: errors during AST validation should abort the compilation before reaching here.
self.tcx.dcx().abort_if_errors();
}
}
}
}
fn lower_variant(
@ -1090,20 +1017,13 @@ fn lower_variant(
def: &hir::VariantData<'_>,
adt_kind: ty::AdtKind,
parent_did: LocalDefId,
is_anonymous: bool,
) -> ty::VariantDef {
let mut has_unnamed_fields = false;
let mut field_uniqueness_check_ctx = FieldUniquenessCheckContext::new(tcx);
let fields = def
.fields()
.iter()
.inspect(|f| {
has_unnamed_fields |= f.ident.name == kw::Underscore;
// We only check named ADT here because anonymous ADTs are checked inside
// the named ADT in which they are defined.
if !is_anonymous {
field_uniqueness_check_ctx.check_field(f);
}
.inspect(|field| {
field_uniqueness_check_ctx.check_field_decl(field.ident, field.span.into());
})
.map(|f| ty::FieldDef {
did: f.def_id.to_def_id(),
@ -1127,7 +1047,6 @@ fn lower_variant(
adt_kind == AdtKind::Struct && tcx.has_attr(parent_did, sym::non_exhaustive)
|| variant_did
.is_some_and(|variant_did| tcx.has_attr(variant_did, sym::non_exhaustive)),
has_unnamed_fields,
)
}
@ -1138,20 +1057,7 @@ fn adt_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::AdtDef<'_> {
bug!("expected ADT to be an item");
};
let is_anonymous = item.ident.name == kw::Empty;
let repr = if is_anonymous {
let parent = tcx.local_parent(def_id);
if let Node::Item(item) = tcx.hir_node_by_def_id(parent)
&& item.is_struct_or_union()
{
tcx.adt_def(parent).repr()
} else {
tcx.dcx().span_delayed_bug(item.span, "anonymous field inside non struct/union");
ty::ReprOptions::default()
}
} else {
tcx.repr_options_of_def(def_id)
};
let repr = tcx.repr_options_of_def(def_id);
let (kind, variants) = match &item.kind {
ItemKind::Enum(def, _) => {
let mut distance_from_explicit = 0;
@ -1175,7 +1081,6 @@ fn adt_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::AdtDef<'_> {
&v.data,
AdtKind::Enum,
def_id,
is_anonymous,
)
})
.collect();
@ -1195,7 +1100,6 @@ fn adt_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::AdtDef<'_> {
def,
adt_kind,
def_id,
is_anonymous,
))
.collect();
@ -1203,7 +1107,7 @@ fn adt_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::AdtDef<'_> {
}
_ => bug!("{:?} is not an ADT", item.owner_id.def_id),
};
tcx.mk_adt_def(def_id.to_def_id(), kind, variants, repr, is_anonymous)
tcx.mk_adt_def(def_id.to_def_id(), kind, variants, repr)
}
fn trait_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::TraitDef {

42
compiler/rustc_hir_analysis/src/errors.rs
View file

@ -734,13 +734,6 @@ pub(crate) struct InvalidUnionField {
pub note: (),
}
#[derive(Diagnostic)]
#[diag(hir_analysis_invalid_unnamed_field_ty)]
pub(crate) struct InvalidUnnamedFieldTy {
#[primary_span]
pub span: Span,
}
#[derive(Diagnostic)]
#[diag(hir_analysis_return_type_notation_on_non_rpitit)]
pub(crate) struct ReturnTypeNotationOnNonRpitit<'tcx> {
@ -1598,41 +1591,6 @@ pub(crate) struct UnconstrainedGenericParameter {
pub const_param_note2: bool,
}
#[derive(Diagnostic)]
pub(crate) 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<UnnamedFieldsReprFieldDefined>,
#[suggestion(code = "#[repr(C)]\n")]
sugg_span: Span,
},
#[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>,
field_adt_kind: &'static str,
#[suggestion(code = "#[repr(C)]\n")]
sugg_span: Span,
},
}
#[derive(Subdiagnostic)]
#[note(hir_analysis_unnamed_fields_repr_field_defined)]
pub(crate) struct UnnamedFieldsReprFieldDefined {
#[primary_span]
pub span: Span,
}
#[derive(Diagnostic)]
#[diag(hir_analysis_opaque_captures_higher_ranked_lifetime, code = E0657)]
pub(crate) struct OpaqueCapturesHigherRankedLifetime {