Skip to content

parse: unify item parsing & filter illegal item kinds #69366

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 17 commits into from
Feb 24, 2020
Merged

parse: unify item parsing & filter illegal item kinds #69366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
fa2a792
add `Span` to `ast::Defaultness::Default`.
Centril Feb 22, 2020
d41fc13
`parse_defaultness`: avoid hardcoded list of keywords.
Centril Feb 22, 2020
7017058
ast: add `Defaultness` to `Item`, making `AssocItem` an alias.
Centril Feb 22, 2020
9ed4c09
parse: extract `error_on_unmatched_vis`.
Centril Feb 22, 2020
a920a05
parse: recover `default` on free items.
Centril Feb 22, 2020
a63f35d
parse: use `parse_item_common` in `parse_foreign_item`.
Centril Feb 22, 2020
a05c83b
parse: use `parse_item_common` in `parse_assoc_item_`.
Centril Feb 22, 2020
a3b0829
parse: move token hack into `parse_item_common`.
Centril Feb 22, 2020
abc46a5
parse: harden `default` test.
Centril Feb 22, 2020
d446c73
parser: refactor away at_end
Centril Feb 22, 2020
ab84914
parser: tweak unmatched wording
Centril Feb 23, 2020
b01c1e2
parser: tweak item kind wording
Centril Feb 23, 2020
842027f
parse: `NtItem` -> `parse_item_common`.
Centril Feb 23, 2020
62930d3
parse/ast: move `Defaultness` into variants.
Centril Feb 23, 2020
e66a39b
parse: tweak `parse_item_` for more reuse.
Centril Feb 23, 2020
fde5939
parse: tweak diagnostic wordings
Centril Feb 23, 2020
1c75f5a
parse: test bad variants wrt. issue 48137.
Centril Feb 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 28 additions & 32 deletions src/librustc_ast_lowering/item.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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