Skip to content

Commit

Permalink
check uniqueness of nested fields
Browse files Browse the repository at this point in the history
  • Loading branch information
frank-king committed Dec 20, 2023
1 parent 161ea3e commit 043ed32
Show file tree
Hide file tree
Showing 13 changed files with 1,841 additions and 96 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1336,7 +1336,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
// } // | owner node of struct or union item
// // ^_____________________|
// }
//
// ```
TyKind::AnonStruct(def_node_id, fields) | TyKind::AnonUnion(def_node_id, fields) => {
let (def_kind, item_kind): (DefKind, fn(_, _) -> _) = match t.kind {
TyKind::AnonStruct(..) => (DefKind::Struct, hir::ItemKind::Struct),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ impl<'a> AstValidator<'a> {
}
}
TyKind::AnonStruct(_, ref fields) | TyKind::AnonUnion(_, ref fields) => {
walk_list!(self, visit_field_def, fields)
walk_list!(self, visit_struct_field_def, fields)
}
_ => visit::walk_ty(self, t),
}
Expand Down
19 changes: 19 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,25 @@ hir_analysis_field_already_declared =
.label = field already declared
.previous_decl_label = `{$field_name}` first declared here
hir_analysis_field_already_declared_both_nested =
field `{$field_name}` is already declared
.label = field `{$field_name}` declared in this unnamed field
.nested_field_decl_note = field `{$field_name}` declared here
.previous_decl_label = `{$field_name}` first declared here in this unnamed field
.previous_nested_field_decl_note = field `{$field_name}` first declared here
hir_analysis_field_already_declared_current_nested =
field `{$field_name}` is already declared
.label = field `{$field_name}` declared in this unnamed field
.nested_field_decl_note = field `{$field_name}` declared here
.previous_decl_label = `{$field_name}` first declared here
hir_analysis_field_already_declared_previous_nested =
field `{$field_name}` is already declared
.label = field already declared
.previous_decl_label = `{$field_name}` first declared here in this unnamed field
.previous_nested_field_decl_note = field `{$field_name}` first declared here
hir_analysis_function_not_found_in_trait = function not found in this trait
hir_analysis_function_not_have_default_implementation = function doesn't have a default implementation
Expand Down
202 changes: 137 additions & 65 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -781,64 +781,100 @@ fn convert_enum_variant_types(tcx: TyCtxt<'_>, def_id: DefId) {
}
}

/*
/// In a type definition, we check that unnamed field names are distinct.
fn check_unnamed_fields_defn<'tcx>(tcx: TyCtxt<'tcx>, item: &hir::Item<'tcx>) {
let mut seen_fields: FxHashMap<Ident, Option<Span>> = Default::default();
fn check_fields_anon_adt_defn<'tcx>(tcx: TyCtxt<'tcx>, item: &hir::Item<'tcx>, seen_fields: &mut FxHashMap<Ident, Option<Span>>) {
let fields = match &item.kind {
hir::ItemKind::Struct(fields, _) | hir::ItemKind::Union(fields, _) => fields,
_ => return,
};
for field in fields.fields() {
if field.ident.name == kw::Underscore {
if let hir::TyKind::AnonAdt(item_id) = field.ty.kind() {
let item = tcx.hir().item(item_id);
check_fields_anon_adt_defn(tcx, item, &mut *seen_fields);
} else {
let field_ty = match tcx.type_of(field.def_id).instantiate_identity().ty_adt_def() {
Some(adt_ty) => adt_ty,
None => {
tcx.sess.emit_err(err);
return;
}
};
if let Some(def_id) = field_ty.did().as_local() {
let item = tcx.hir().item(hir::ItemId { owner_id: hir::OwnerId { def_id }});
check_fields_anon_adt_defn(tcx, item, &mut *seen_fields);
}
#[derive(Clone, Copy)]
struct NestedSpan {
span: Span,
nested_field_span: Span,
}

#[derive(Clone, Copy)]
enum FieldDeclSpan {
NotNested(Span),
Nested(NestedSpan),
}

impl From<Span> for FieldDeclSpan {
fn from(span: Span) -> Self {
Self::NotNested(span)
}
}

impl From<NestedSpan> for FieldDeclSpan {
fn from(span: NestedSpan) -> Self {
Self::Nested(span)
}
}

/// Check the uniqueness of fields across adt where there are
/// nested fields imported from an unnamed field.
fn check_field_uniqueness_in_nested_adt(
tcx: TyCtxt<'_>,
adt_def: ty::AdtDef<'_>,
check: &mut impl FnMut(Ident, /* nested_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 tcx.type_of(field.did).instantiate_identity().kind() {
ty::Adt(adt_def, _) => {
check_field_uniqueness_in_nested_adt(tcx, *adt_def, &mut *check);
}
field_ty.flags()
let inner_adt_def = field_ty.ty_adt_def().expect("expect an adt");
check_fields_anon_adt_defn(tcx, adt_def, &mut *seen_fields);
} else {
let span = field.did.as_local().map(|did| {
let hir_id = tcx.hir().local_def_id_to_hir_id(did);
tcx.hir().span(hir_id)
});
match seen_fields.get(&ident.normalize_to_macros_2_0()).cloned() {
Some(Some(prev_span)) => {
tcx.sess.emit_err(errors::FieldAlreadyDeclared {
field_name: ident,
span: f.span,
prev_span,
});
}
Some(None) => {
tcx.sess.emit_err(errors::FieldAlreadyDeclared {
field_name: f.ident,
span: f.span,
prev_span,
});
ty_kind => bug!(
"Unexpected ty kind in check_field_uniqueness_in_nested_adt(): {ty_kind:?}"
),
}
} else {
check(field.ident(tcx), tcx.def_span(field.did));
}
}
}

/// 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
/// annoymous adt.
fn check_field_uniqueness(
tcx: TyCtxt<'_>,
field: &hir::FieldDef<'_>,
check: &mut impl FnMut(Ident, FieldDeclSpan),
) {
if field.ident.name == kw::Underscore {
let ty_span = field.ty.span;
match &field.ty.kind {
hir::TyKind::AnonAdt(item_id) => {
match &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| check_field_uniqueness(tcx, f, &mut *check));
}
None =>
seen_fields.insert(f.ident.normalize_to_macros_2_0(), f.span);
item_kind => span_bug!(
ty_span,
"Unexpected item kind in check_field_uniqueness(): {item_kind:?}"
),
}
}
hir::TyKind::Path(hir::QPath::Resolved(_, hir::Path { res, .. })) => {
check_field_uniqueness_in_nested_adt(
tcx,
tcx.adt_def(res.def_id()),
&mut |ident, nested_field_span| {
check(ident, NestedSpan { span: field.span, nested_field_span }.into())
},
);
}
// 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)
_ => {
debug_assert!(tcx.sess.has_errors().is_some());
tcx.sess.abort_if_errors()
}
}
return;
}
check(field.ident, field.span.into());
}
*/

fn convert_variant(
tcx: TyCtxt<'_>,
Expand All @@ -848,27 +884,61 @@ fn convert_variant(
def: &hir::VariantData<'_>,
adt_kind: ty::AdtKind,
parent_did: LocalDefId,
is_anonymous: bool,
) -> ty::VariantDef {
let mut has_unnamed_fields = false;
let mut seen_fields: FxHashMap<Ident, Span> = Default::default();
let mut seen_fields: FxHashMap<Ident, FieldDeclSpan> = Default::default();
let fields = def
.fields()
.iter()
.inspect(|f| {
// Skip the unnamed field here, we will check it later.
if f.ident.name == kw::Underscore {
has_unnamed_fields = true;
return;
}
let dup_span = seen_fields.get(&f.ident.normalize_to_macros_2_0()).cloned();
if let Some(prev_span) = dup_span {
tcx.sess.emit_err(errors::FieldAlreadyDeclared {
field_name: f.ident,
span: f.span,
prev_span,
has_unnamed_fields |= f.ident.name == kw::Underscore;
if !is_anonymous {
check_field_uniqueness(tcx, f, &mut |ident, field_decl| {
use FieldDeclSpan::*;
let field_name = ident.name;
let ident = ident.normalize_to_macros_2_0();
match (field_decl, seen_fields.get(&ident).copied()) {
(NotNested(span), Some(NotNested(prev_span))) => {
tcx.sess.emit_err(errors::FieldAlreadyDeclared::NotNested {
field_name,
span,
prev_span,
});
}
(NotNested(span), Some(Nested(prev))) => {
tcx.sess.emit_err(errors::FieldAlreadyDeclared::PreviousNested {
field_name,
span,
prev_span: prev.span,
prev_nested_field_span: prev.nested_field_span,
});
}
(
Nested(NestedSpan { span, nested_field_span }),
Some(NotNested(prev_span)),
) => {
tcx.sess.emit_err(errors::FieldAlreadyDeclared::CurrentNested {
field_name,
span,
nested_field_span,
prev_span,
});
}
(Nested(NestedSpan { span, nested_field_span }), Some(Nested(prev))) => {
tcx.sess.emit_err(errors::FieldAlreadyDeclared::BothNested {
field_name,
span,
nested_field_span,
prev_span: prev.span,
prev_nested_field_span: prev.nested_field_span,
});
}
(field_decl, None) => {
seen_fields.insert(ident, field_decl);
}
}
});
} else {
seen_fields.insert(f.ident.normalize_to_macros_2_0(), f.span);
}
})
.map(|f| ty::FieldDef {
Expand Down Expand Up @@ -929,6 +999,7 @@ fn adt_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::AdtDef<'_> {
&v.data,
AdtKind::Enum,
def_id,
is_anonymous,
)
})
.collect();
Expand All @@ -948,6 +1019,7 @@ fn adt_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::AdtDef<'_> {
def,
adt_kind,
def_id,
is_anonymous,
))
.collect();

Expand Down
53 changes: 45 additions & 8 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,14 +186,51 @@ pub struct DropImplOnWrongItem {
}

#[derive(Diagnostic)]
#[diag(hir_analysis_field_already_declared, code = "E0124")]
pub struct FieldAlreadyDeclared {
pub field_name: Ident,
#[primary_span]
#[label]
pub span: Span,
#[label(hir_analysis_previous_decl_label)]
pub prev_span: Span,
pub enum FieldAlreadyDeclared {
#[diag(hir_analysis_field_already_declared, code = "E0124")]
NotNested {
field_name: Symbol,
#[primary_span]
#[label]
span: Span,
#[label(hir_analysis_previous_decl_label)]
prev_span: Span,
},
#[diag(hir_analysis_field_already_declared_current_nested)]
CurrentNested {
field_name: Symbol,
#[primary_span]
#[label]
span: Span,
#[note(hir_analysis_nested_field_decl_note)]
nested_field_span: Span,
#[label(hir_analysis_previous_decl_label)]
prev_span: Span,
},
#[diag(hir_analysis_field_already_declared_previous_nested)]
PreviousNested {
field_name: Symbol,
#[primary_span]
#[label]
span: Span,
#[label(hir_analysis_previous_decl_label)]
prev_span: Span,
#[note(hir_analysis_previous_nested_field_decl_note)]
prev_nested_field_span: Span,
},
#[diag(hir_analysis_field_already_declared_both_nested)]
BothNested {
field_name: Symbol,
#[primary_span]
#[label]
span: Span,
#[note(hir_analysis_nested_field_decl_note)]
nested_field_span: Span,
#[label(hir_analysis_previous_decl_label)]
prev_span: Span,
#[note(hir_analysis_previous_nested_field_decl_note)]
prev_nested_field_span: Span,
},
}

#[derive(Diagnostic)]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ impl<'tcx> AdtDef<'tcx> {
}

/// Returns an iterator over all fields contained
/// by this ADT.
/// by this ADT (nested unnamed fields are not expanded).
#[inline]
pub fn all_fields(self) -> impl Iterator<Item = &'tcx FieldDef> + Clone {
self.variants().iter().flat_map(|v| v.fields.iter())
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2037,6 +2037,11 @@ impl<'tcx> FieldDef {
pub fn ident(&self, tcx: TyCtxt<'_>) -> Ident {
Ident::new(self.name, tcx.def_ident_span(self.did).unwrap())
}

/// Returns whether the field is unnamed
pub fn is_unnamed(&self) -> bool {
self.name == rustc_span::symbol::kw::Underscore
}
}

#[derive(Debug, PartialEq, Eq)]
Expand Down
Loading

0 comments on commit 043ed32

Please sign in to comment.