From d4ad050ce5778a09566f6f9ec172565815d54604 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Sun, 16 May 2021 09:51:00 -0500 Subject: [PATCH] Check and deny anonymous fields on `ast_validation` --- .../rustc_ast_passes/src/ast_validation.rs | 254 +++++++++++++----- 1 file changed, 184 insertions(+), 70 deletions(-) diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 6d6438920c0..ba2da769497 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -175,10 +175,30 @@ impl<'a> AstValidator<'a> { } } } + TyKind::AnonymousStruct(ref fields, ..) | TyKind::AnonymousUnion(ref fields, ..) => { + self.with_banned_assoc_ty_bound(|this| { + walk_list!(this, visit_struct_field_def, fields) + }); + } _ => visit::walk_ty(self, t), } } + fn visit_struct_field_def(&mut self, field: &'a FieldDef) { + if let Some(ident) = field.ident { + if ident.name == kw::Underscore { + self.check_anonymous_field(field); + self.visit_vis(&field.vis); + self.visit_ident(ident); + self.visit_ty_common(&field.ty); + self.walk_ty(&field.ty); + walk_list!(self, visit_attribute, &field.attrs); + return; + } + } + self.visit_field_def(field); + } + fn err_handler(&self) -> &rustc_errors::Handler { &self.session.diagnostic() } @@ -213,6 +233,66 @@ impl<'a> AstValidator<'a> { err.emit(); } + fn check_anonymous_field(&self, field: &FieldDef) { + let FieldDef { ty, .. } = field; + match &ty.kind { + TyKind::AnonymousStruct(..) | TyKind::AnonymousUnion(..) => { + // We already checked for `kw::Underscore` before calling this function, + // so skip the check + } + TyKind::Path(..) => { + // If the anonymous field contains a Path as type, we can't determine + // if the path is a valid struct or union, so skip the check + } + _ => { + let msg = "unnamed fields can only have struct or union types"; + let label = "not a struct or union"; + self.err_handler() + .struct_span_err(field.span, msg) + .span_label(ty.span, label) + .emit(); + } + } + } + + fn deny_anonymous_struct(&self, ty: &Ty) { + match &ty.kind { + TyKind::AnonymousStruct(..) => { + self.err_handler() + .struct_span_err( + ty.span, + "anonymous structs are not allowed outside of unnamed struct or union fields", + ) + .span_label(ty.span, "anonymous struct declared here") + .emit(); + } + TyKind::AnonymousUnion(..) => { + self.err_handler() + .struct_span_err( + ty.span, + "anonymous unions are not allowed outside of unnamed struct or union fields", + ) + .span_label(ty.span, "anonymous union declared here") + .emit(); + } + _ => {} + } + } + + fn deny_anonymous_field(&self, field: &FieldDef) { + if let Some(ident) = field.ident { + if ident.name == kw::Underscore { + self.err_handler() + .struct_span_err( + field.span, + "anonymous fields are not allowed outside of structs or unions", + ) + .span_label(ident.span, "anonymous field declared here") + .emit() + } + } + } + fn check_decl_no_pat(decl: &FnDecl, mut report_err: impl FnMut(Span, Option, bool)) { for Param { pat, .. } in &decl.inputs { match pat.kind { @@ -732,6 +812,71 @@ impl<'a> AstValidator<'a> { ) .emit(); } + + fn visit_ty_common(&mut self, ty: &'a Ty) { + match ty.kind { + TyKind::BareFn(ref bfty) => { + self.check_fn_decl(&bfty.decl, SelfSemantic::No); + Self::check_decl_no_pat(&bfty.decl, |span, _, _| { + struct_span_err!( + self.session, + span, + E0561, + "patterns aren't allowed in function pointer types" + ) + .emit(); + }); + self.check_late_bound_lifetime_defs(&bfty.generic_params); + } + TyKind::TraitObject(ref bounds, ..) => { + let mut any_lifetime_bounds = false; + for bound in bounds { + if let GenericBound::Outlives(ref lifetime) = *bound { + if any_lifetime_bounds { + struct_span_err!( + self.session, + lifetime.ident.span, + E0226, + "only a single explicit lifetime bound is permitted" + ) + .emit(); + break; + } + any_lifetime_bounds = true; + } + } + self.no_questions_in_bounds(bounds, "trait object types", false); + } + TyKind::ImplTrait(_, ref bounds) => { + if self.is_impl_trait_banned { + struct_span_err!( + self.session, + ty.span, + E0667, + "`impl Trait` is not allowed in path parameters" + ) + .emit(); + } + + if let Some(outer_impl_trait_sp) = self.outer_impl_trait { + struct_span_err!( + self.session, + ty.span, + E0666, + "nested `impl Trait` is not allowed" + ) + .span_label(outer_impl_trait_sp, "outer `impl Trait`") + .span_label(ty.span, "nested `impl Trait` here") + .emit(); + } + + if !bounds.iter().any(|b| matches!(b, GenericBound::Trait(..))) { + self.err_handler().span_err(ty.span, "at least one trait must be specified"); + } + } + _ => {} + } + } } /// Checks that generic parameters are in the correct order, @@ -850,72 +995,8 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } fn visit_ty(&mut self, ty: &'a Ty) { - match ty.kind { - TyKind::BareFn(ref bfty) => { - self.check_fn_decl(&bfty.decl, SelfSemantic::No); - Self::check_decl_no_pat(&bfty.decl, |span, _, _| { - struct_span_err!( - self.session, - span, - E0561, - "patterns aren't allowed in function pointer types" - ) - .emit(); - }); - self.check_late_bound_lifetime_defs(&bfty.generic_params); - } - TyKind::TraitObject(ref bounds, ..) => { - let mut any_lifetime_bounds = false; - for bound in bounds { - if let GenericBound::Outlives(ref lifetime) = *bound { - if any_lifetime_bounds { - struct_span_err!( - self.session, - lifetime.ident.span, - E0226, - "only a single explicit lifetime bound is permitted" - ) - .emit(); - break; - } - any_lifetime_bounds = true; - } - } - self.no_questions_in_bounds(bounds, "trait object types", false); - } - TyKind::ImplTrait(_, ref bounds) => { - if self.is_impl_trait_banned { - struct_span_err!( - self.session, - ty.span, - E0667, - "`impl Trait` is not allowed in path parameters" - ) - .emit(); - } - - if let Some(outer_impl_trait_sp) = self.outer_impl_trait { - struct_span_err!( - self.session, - ty.span, - E0666, - "nested `impl Trait` is not allowed" - ) - .span_label(outer_impl_trait_sp, "outer `impl Trait`") - .span_label(ty.span, "nested `impl Trait` here") - .emit(); - } - - if !bounds.iter().any(|b| matches!(b, GenericBound::Trait(..))) { - self.err_handler().span_err(ty.span, "at least one trait must be specified"); - } - - self.walk_ty(ty); - return; - } - _ => {} - } - + self.visit_ty_common(ty); + self.deny_anonymous_struct(ty); self.walk_ty(ty) } @@ -929,6 +1010,11 @@ impl<'a> Visitor<'a> for AstValidator<'a> { visit::walk_lifetime(self, lifetime); } + fn visit_field_def(&mut self, s: &'a FieldDef) { + self.deny_anonymous_field(s); + visit::walk_field_def(self, s) + } + fn visit_item(&mut self, item: &'a Item) { if item.attrs.iter().any(|attr| self.session.is_proc_macro_attr(attr)) { self.has_proc_macro_decls = true; @@ -1084,14 +1170,42 @@ impl<'a> Visitor<'a> for AstValidator<'a> { self.check_mod_file_item_asciionly(item.ident); } } - ItemKind::Union(ref vdata, _) => { - if let VariantData::Tuple(..) | VariantData::Unit(..) = vdata { - self.err_handler() - .span_err(item.span, "tuple and unit unions are not permitted"); + ItemKind::Struct(ref vdata, ref generics) => match vdata { + // Duplicating the `Visitor` logic allows catching all cases + // of `Anonymous(Struct, Union)` outside of a field struct or union. + // + // Inside `visit_ty` the validator catches every `Anonymous(Struct, Union)` it + // encounters, and only on `ItemKind::Struct` and `ItemKind::Union` + // it uses `visit_ty_common`, which doesn't contain that specific check. + VariantData::Struct(ref fields, ..) => { + self.visit_vis(&item.vis); + self.visit_ident(item.ident); + self.visit_generics(generics); + self.with_banned_assoc_ty_bound(|this| { + walk_list!(this, visit_struct_field_def, fields); + }); + walk_list!(self, visit_attribute, &item.attrs); + return; } + _ => {} + }, + ItemKind::Union(ref vdata, ref generics) => { if vdata.fields().is_empty() { self.err_handler().span_err(item.span, "unions cannot have zero fields"); } + match vdata { + VariantData::Struct(ref fields, ..) => { + self.visit_vis(&item.vis); + self.visit_ident(item.ident); + self.visit_generics(generics); + self.with_banned_assoc_ty_bound(|this| { + walk_list!(this, visit_struct_field_def, fields); + }); + walk_list!(self, visit_attribute, &item.attrs); + return; + } + _ => {} + } } ItemKind::Const(def, .., None) => { self.check_defaultness(item.span, def);