Skip to content

Commit

Permalink
Auto merge of #69366 - Centril:unified-items, r=petrochenkov
Browse files Browse the repository at this point in the history 
parse: unify item parsing & filter illegal item kinds

This PR fully unifies item parsing into a single `fn parse_item_common` method which produces `Option<Item>`. The `Item` is then mapped into `ForeignItem` and `AssocItem` as necessary by transforming the `*Kind` and converting contextually bad variants into `None`, thereby filtering them away.

The PR does not yet unmerge the definition of `ForeignItemKind` from `AssocItemKind`. I've left that as future work as it didn't feel like this parser-focused PR would be the best one to deal with it. Changes to the AST data structures are instead kept to a reasonable minimum.

Based on #69361.

Fixes #48137.
RELNOTES: Now, `item` macro fragments can be interpolated into `impl`, `trait`, and `extern` contexts. See `src/test/ui/parser/issue-48137-macros-cannot-interpolate-impl-items.rs` for the relevant test.

r? @petrochenkov
cc @estebank
  • Loading branch information
@bors
bors committed Feb 24, 2020
2 parents 6d0e58b + 1c75f5a commit 79cd224
Show file tree
Hide file tree
Showing 88 changed files with 1,977 additions and 644 deletions.
60 changes: 28 additions & 32 deletions src/librustc_ast_lowering/item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
ids
}
ItemKind::Const(ref ty, ..) => {
ItemKind::Const(_, ref ty, ..) => {
let mut ids = smallvec![i.id];
if self.sess.features_untracked().impl_trait_in_bindings {
let mut visitor = ImplTraitTypeIdVisitor { ids: &mut ids };
Expand Down Expand Up @@ -264,11 +264,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
let (ty, body_id) = self.lower_const_item(t, span, e.as_deref());
hir::ItemKind::Static(ty, m, body_id)
}
ItemKind::Const(ref t, ref e) => {
ItemKind::Const(_, ref t, ref e) => {
let (ty, body_id) = self.lower_const_item(t, span, e.as_deref());
hir::ItemKind::Const(ty, body_id)
}
ItemKind::Fn(FnSig { ref decl, header }, ref generics, ref body) => {
ItemKind::Fn(_, FnSig { ref decl, header }, ref generics, ref body) => {
let fn_def_id = self.resolver.definitions().local_def_id(id);
self.with_new_scopes(|this| {
this.current_item = Some(ident.span);
Expand Down Expand Up @@ -297,24 +297,24 @@ impl<'hir> LoweringContext<'_, 'hir> {
ItemKind::Mod(ref m) => hir::ItemKind::Mod(self.lower_mod(m)),
ItemKind::ForeignMod(ref nm) => hir::ItemKind::ForeignMod(self.lower_foreign_mod(nm)),
ItemKind::GlobalAsm(ref ga) => hir::ItemKind::GlobalAsm(self.lower_global_asm(ga)),
ItemKind::TyAlias(ref generics, _, Some(ref ty)) => match ty.kind.opaque_top_hack() {
ItemKind::TyAlias(_, ref gen, _, Some(ref ty)) => match ty.kind.opaque_top_hack() {
None => {
let ty = self.lower_ty(ty, ImplTraitContext::disallowed());
let generics = self.lower_generics(generics, ImplTraitContext::disallowed());
let generics = self.lower_generics(gen, ImplTraitContext::disallowed());
hir::ItemKind::TyAlias(ty, generics)
}
Some(bounds) => {
let ctx = || ImplTraitContext::OpaqueTy(None, hir::OpaqueTyOrigin::Misc);
let ty = hir::OpaqueTy {
generics: self.lower_generics(generics, ctx()),
generics: self.lower_generics(gen, ctx()),
bounds: self.lower_param_bounds(bounds, ctx()),
impl_trait_fn: None,
origin: hir::OpaqueTyOrigin::TypeAlias,
};
hir::ItemKind::OpaqueTy(ty)
}
},
ItemKind::TyAlias(ref generics, _, None) => {
ItemKind::TyAlias(_, ref generics, _, None) => {
let ty = self.arena.alloc(self.ty(span, hir::TyKind::Err));
let generics = self.lower_generics(generics, ImplTraitContext::disallowed());
hir::ItemKind::TyAlias(ty, generics)
Expand Down Expand Up @@ -654,7 +654,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
ident: i.ident,
attrs: self.lower_attrs(&i.attrs),
kind: match i.kind {
ForeignItemKind::Fn(ref sig, ref generics, _) => {
ForeignItemKind::Fn(_, ref sig, ref generics, _) => {
let fdec = &sig.decl;
let (generics, (fn_dec, fn_args)) = self.add_in_band_defs(
generics,
Expand All @@ -675,7 +675,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let ty = self.lower_ty(t, ImplTraitContext::disallowed());
hir::ForeignItemKind::Static(ty, m)
}
ForeignItemKind::Const(ref t, _) => {
ForeignItemKind::Const(_, ref t, _) => {
// For recovery purposes.
let ty = self.lower_ty(t, ImplTraitContext::disallowed());
hir::ForeignItemKind::Static(ty, Mutability::Not)
Expand Down Expand Up @@ -758,24 +758,24 @@ impl<'hir> LoweringContext<'_, 'hir> {

let (generics, kind) = match i.kind {
AssocItemKind::Static(ref ty, _, ref default) // Let's pretend this is a `const`.
| AssocItemKind::Const(ref ty, ref default) => {
| AssocItemKind::Const(_, ref ty, ref default) => {
let ty = self.lower_ty(ty, ImplTraitContext::disallowed());
let body = default.as_ref().map(|x| self.lower_const_body(i.span, Some(x)));
(hir::Generics::empty(), hir::TraitItemKind::Const(ty, body))
}
AssocItemKind::Fn(ref sig, ref generics, None) => {
AssocItemKind::Fn(_, ref sig, ref generics, None) => {
let names = self.lower_fn_params_to_names(&sig.decl);
let (generics, sig) =
self.lower_method_sig(generics, sig, trait_item_def_id, false, None);
(generics, hir::TraitItemKind::Method(sig, hir::TraitMethod::Required(names)))
}
AssocItemKind::Fn(ref sig, ref generics, Some(ref body)) => {
AssocItemKind::Fn(_, ref sig, ref generics, Some(ref body)) => {
let body_id = self.lower_fn_body_block(i.span, &sig.decl, Some(body));
let (generics, sig) =
self.lower_method_sig(generics, sig, trait_item_def_id, false, None);
(generics, hir::TraitItemKind::Method(sig, hir::TraitMethod::Provided(body_id)))
}
AssocItemKind::TyAlias(ref generics, ref bounds, ref default) => {
AssocItemKind::TyAlias(_, ref generics, ref bounds, ref default) => {
let ty = default.as_ref().map(|x| self.lower_ty(x, ImplTraitContext::disallowed()));
let generics = self.lower_generics(generics, ImplTraitContext::disallowed());
let kind = hir::TraitItemKind::Type(
Expand All @@ -801,22 +801,18 @@ impl<'hir> LoweringContext<'_, 'hir> {
fn lower_trait_item_ref(&mut self, i: &AssocItem) -> hir::TraitItemRef {
let (kind, has_default) = match &i.kind {
AssocItemKind::Static(_, _, default) // Let's pretend this is a `const` for recovery.
| AssocItemKind::Const(_, default) => {
| AssocItemKind::Const(_, _, default) => {
(hir::AssocItemKind::Const, default.is_some())
}
AssocItemKind::TyAlias(_, _, default) => (hir::AssocItemKind::Type, default.is_some()),
AssocItemKind::Fn(sig, _, default) => {
AssocItemKind::TyAlias(_, _, _, default) => (hir::AssocItemKind::Type, default.is_some()),
AssocItemKind::Fn(_, sig, _, default) => {
(hir::AssocItemKind::Method { has_self: sig.decl.has_self() }, default.is_some())
}
AssocItemKind::Macro(..) => unimplemented!(),
};
hir::TraitItemRef {
id: hir::TraitItemId { hir_id: self.lower_node_id(i.id) },
ident: i.ident,
span: i.span,
defaultness: self.lower_defaultness(Defaultness::Default, has_default),
kind,
}
let id = hir::TraitItemId { hir_id: self.lower_node_id(i.id) };
let defaultness = hir::Defaultness::Default { has_value: has_default };
hir::TraitItemRef { id, ident: i.ident, span: i.span, defaultness, kind }
}

/// Construct `ExprKind::Err` for the given `span`.
Expand All @@ -827,15 +823,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
fn lower_impl_item(&mut self, i: &AssocItem) -> hir::ImplItem<'hir> {
let impl_item_def_id = self.resolver.definitions().local_def_id(i.id);

let (generics, kind) = match i.kind {
AssocItemKind::Static(ref ty, _, ref expr) | AssocItemKind::Const(ref ty, ref expr) => {
let (generics, kind) = match &i.kind {
AssocItemKind::Static(ty, _, expr) | AssocItemKind::Const(_, ty, expr) => {
let ty = self.lower_ty(ty, ImplTraitContext::disallowed());
(
hir::Generics::empty(),
hir::ImplItemKind::Const(ty, self.lower_const_body(i.span, expr.as_deref())),
)
}
AssocItemKind::Fn(ref sig, ref generics, ref body) => {
AssocItemKind::Fn(_, sig, generics, body) => {
self.current_item = Some(i.span);
let asyncness = sig.header.asyncness;
let body_id =
Expand All @@ -851,7 +847,7 @@ impl<'hir> LoweringContext<'_, 'hir> {

(generics, hir::ImplItemKind::Method(sig, body_id))
}
AssocItemKind::TyAlias(ref generics, _, ref ty) => {
AssocItemKind::TyAlias(_, generics, _, ty) => {
let generics = self.lower_generics(generics, ImplTraitContext::disallowed());
let kind = match ty {
None => {
Expand Down Expand Up @@ -880,7 +876,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
attrs: self.lower_attrs(&i.attrs),
generics,
vis: self.lower_visibility(&i.vis, None),
defaultness: self.lower_defaultness(i.defaultness, true /* [1] */),
defaultness: self.lower_defaultness(i.kind.defaultness(), true /* [1] */),
kind,
span: i.span,
}
Expand All @@ -894,17 +890,17 @@ impl<'hir> LoweringContext<'_, 'hir> {
ident: i.ident,
span: i.span,
vis: self.lower_visibility(&i.vis, Some(i.id)),
defaultness: self.lower_defaultness(i.defaultness, true /* [1] */),
defaultness: self.lower_defaultness(i.kind.defaultness(), true /* [1] */),
kind: match &i.kind {
AssocItemKind::Static(..) // Let's pretend this is a `const` for recovery.
| AssocItemKind::Const(..) => hir::AssocItemKind::Const,
AssocItemKind::TyAlias(_, _, ty) => {
AssocItemKind::TyAlias(.., ty) => {
match ty.as_deref().and_then(|ty| ty.kind.opaque_top_hack()) {
None => hir::AssocItemKind::Type,
Some(_) => hir::AssocItemKind::OpaqueTy,
}
}
AssocItemKind::Fn(sig, _, _) => {
AssocItemKind::Fn(_, sig, ..) => {
hir::AssocItemKind::Method { has_self: sig.decl.has_self() }
}
AssocItemKind::Macro(..) => unimplemented!(),
Expand Down Expand Up @@ -948,7 +944,7 @@ impl<'hir> LoweringContext<'_, 'hir> {

fn lower_defaultness(&self, d: Defaultness, has_value: bool) -> hir::Defaultness {
match d {
Defaultness::Default => hir::Defaultness::Default { has_value: has_value },
Defaultness::Default(_) => hir::Defaultness::Default { has_value },
Defaultness::Final => {
assert!(has_value);
hir::Defaultness::Final
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_ast_lowering/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
ItemKind::Struct(_, ref generics)
| ItemKind::Union(_, ref generics)
| ItemKind::Enum(_, ref generics)
| ItemKind::TyAlias(ref generics, ..)
| ItemKind::TyAlias(_, ref generics, ..)
| ItemKind::Trait(_, _, ref generics, ..) => {
let def_id = self.lctx.resolver.definitions().local_def_id(item.id);
let count = generics
Expand Down Expand Up @@ -490,7 +490,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
self.lctx.allocate_hir_id_counter(item.id);
let owner = match (&item.kind, ctxt) {
// Ignore patterns in trait methods without bodies.
(AssocItemKind::Fn(_, _, None), AssocCtxt::Trait) => None,
(AssocItemKind::Fn(_, _, _, None), AssocCtxt::Trait) => None,
_ => Some(item.id),
};
self.with_hir_id_owner(owner, |this| visit::walk_assoc_item(this, item, ctxt));
Expand Down
39 changes: 24 additions & 15 deletions src/librustc_ast_passes/ast_validation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,9 +400,11 @@ impl<'a> AstValidator<'a> {
}

fn check_defaultness(&self, span: Span, defaultness: Defaultness) {
if let Defaultness::Default = defaultness {
if let Defaultness::Default(def_span) = defaultness {
let span = self.session.source_map().def_span(span);
self.err_handler()
.struct_span_err(span, "`default` is only allowed on items in `impl` definitions")
.span_label(def_span, "`default` because of this")
.emit();
}
}
Expand Down Expand Up @@ -863,10 +865,12 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
if polarity == ImplPolarity::Negative {
self.err_handler().span_err(item.span, "inherent impls cannot be negative");
}
if defaultness == Defaultness::Default {
if let Defaultness::Default(def_span) = defaultness {
let span = self.session.source_map().def_span(item.span);
self.err_handler()
.struct_span_err(item.span, "inherent impls cannot be default")
.note("only trait implementations may be annotated with default")
.struct_span_err(span, "inherent impls cannot be `default`")
.span_label(def_span, "`default` because of this")
.note("only trait implementations may be annotated with `default`")
.emit();
}
if let Const::Yes(span) = constness {
Expand All @@ -877,7 +881,8 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
.emit();
}
}
ItemKind::Fn(ref sig, ref generics, ref body) => {
ItemKind::Fn(def, ref sig, ref generics, ref body) => {
self.check_defaultness(item.span, def);
self.check_const_fn_const_generic(item.span, sig, generics);

if body.is_none() {
Expand Down Expand Up @@ -961,15 +966,17 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
self.err_handler().span_err(item.span, "unions cannot have zero fields");
}
}
ItemKind::Const(.., None) => {
ItemKind::Const(def, .., None) => {
self.check_defaultness(item.span, def);
let msg = "free constant item without body";
self.error_item_without_body(item.span, "constant", msg, " = <expr>;");
}
ItemKind::Static(.., None) => {
let msg = "free static item without body";
self.error_item_without_body(item.span, "static", msg, " = <expr>;");
}
ItemKind::TyAlias(_, ref bounds, ref body) => {
ItemKind::TyAlias(def, _, ref bounds, ref body) => {
self.check_defaultness(item.span, def);
if body.is_none() {
let msg = "free type alias without body";
self.error_item_without_body(item.span, "type", msg, " = <type>;");
Expand All @@ -984,11 +991,13 @@ impl<'a> Visitor<'a> for AstValidator<'a> {

fn visit_foreign_item(&mut self, fi: &'a ForeignItem) {
match &fi.kind {
ForeignItemKind::Fn(sig, _, body) => {
ForeignItemKind::Fn(def, sig, _, body) => {
self.check_defaultness(fi.span, *def);
self.check_foreign_fn_bodyless(fi.ident, body.as_deref());
self.check_foreign_fn_headerless(fi.ident, fi.span, sig.header);
}
ForeignItemKind::TyAlias(generics, bounds, body) => {
ForeignItemKind::TyAlias(def, generics, bounds, body) => {
self.check_defaultness(fi.span, *def);
self.check_foreign_kind_bodyless(fi.ident, "type", body.as_ref().map(|b| b.span));
self.check_type_no_bounds(bounds, "`extern` blocks");
self.check_foreign_ty_genericless(generics);
Expand Down Expand Up @@ -1229,19 +1238,19 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
}

fn visit_assoc_item(&mut self, item: &'a AssocItem, ctxt: AssocCtxt) {
if ctxt == AssocCtxt::Trait {
self.check_defaultness(item.span, item.defaultness);
if ctxt == AssocCtxt::Trait || !self.in_trait_impl {
self.check_defaultness(item.span, item.kind.defaultness());
}

if ctxt == AssocCtxt::Impl {
match &item.kind {
AssocItemKind::Const(_, body) => {
AssocItemKind::Const(_, _, body) => {
self.check_impl_item_provided(item.span, body, "constant", " = <expr>;");
}
AssocItemKind::Fn(_, _, body) => {
AssocItemKind::Fn(_, _, _, body) => {
self.check_impl_item_provided(item.span, body, "function", " { <body> }");
}
AssocItemKind::TyAlias(_, bounds, body) => {
AssocItemKind::TyAlias(_, _, bounds, body) => {
self.check_impl_item_provided(item.span, body, "type", " = <type>;");
self.check_type_no_bounds(bounds, "`impl`s");
}
Expand All @@ -1251,7 +1260,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {

if ctxt == AssocCtxt::Trait || self.in_trait_impl {
self.invalid_visibility(&item.vis, None);
if let AssocItemKind::Fn(sig, _, _) = &item.kind {
if let AssocItemKind::Fn(_, sig, _, _) = &item.kind {
self.check_trait_fn_not_const(sig.header.constness);
self.check_trait_fn_not_async(item.span, sig.header.asyncness);
}
Expand Down
10 changes: 5 additions & 5 deletions src/librustc_ast_passes/feature_gate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
);
}

if let ast::Defaultness::Default = defaultness {
if let ast::Defaultness::Default(_) = defaultness {
gate_feature_post!(&self, specialization, i.span, "specialization is unstable");
}
}
Expand All @@ -372,7 +372,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
gate_feature_post!(&self, decl_macro, i.span, msg);
}

ast::ItemKind::TyAlias(_, _, Some(ref ty)) => self.check_impl_trait(&ty),
ast::ItemKind::TyAlias(_, _, _, Some(ref ty)) => self.check_impl_trait(&ty),

_ => {}
}
Expand Down Expand Up @@ -543,17 +543,17 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
}

fn visit_assoc_item(&mut self, i: &'a ast::AssocItem, ctxt: AssocCtxt) {
if i.defaultness == ast::Defaultness::Default {
if let ast::Defaultness::Default(_) = i.kind.defaultness() {
gate_feature_post!(&self, specialization, i.span, "specialization is unstable");
}

match i.kind {
ast::AssocItemKind::Fn(ref sig, _, _) => {
ast::AssocItemKind::Fn(_, ref sig, _, _) => {
if let (ast::Const::Yes(_), AssocCtxt::Trait) = (sig.header.constness, ctxt) {
gate_feature_post!(&self, const_fn, i.span, "const fn is unstable");
}
}
ast::AssocItemKind::TyAlias(ref generics, _, ref ty) => {
ast::AssocItemKind::TyAlias(_, ref generics, _, ref ty) => {
if let (Some(_), AssocCtxt::Trait) = (ty, ctxt) {
gate_feature_post!(
&self,
Expand Down
Loading

0 comments on commit 79cd224

Please sign in to comment.