Skip to content

Commit

Permalink
Rollup merge of rust-lang#69194 - Centril:assoc-extern-fuse, r=petroc…
Browse files Browse the repository at this point in the history
…henkov

parse: fuse associated and extern items up to defaultness

Language changes:

- The grammar of extern `type` aliases is unified with associated ones, and becomes:
  ```rust
  TypeItem = "type" ident generics {":" bounds}? where_clause {"=" type}? ";" ;
  ```

  Semantic restrictions (`ast_validation`) are added to forbid any parameters in `generics`, any bounds in `bounds`, and any predicates in `where_clause`, as well as the presence of a type expression (`= u8`).

  (Work still remains to fuse this with free `type` aliases, but this can be done later.)

- The grammar of constants and static items (free, associated, and extern) now permits the absence of an expression, and becomes:

  ```rust
  GlobalItem = {"const" {ident | "_"} | "static" "mut"? ident} {"=" expr}? ";" ;
  ```

  - A semantic restriction is added to enforce the presence of the expression (the body).
  - A semantic restriction is added to reject `const _` in associated contexts.

Together, these changes allow us to fuse the grammar of associated items and extern items up to `default`ness which is the main goal of the PR.

-----------------------

We are now very close to fully fusing the entirely of item parsing and their ASTs. To progress further, we must make a decision: should we parse e.g. `default use foo::bar;` and whatnot? Accepting that is likely easiest from a parsing perspective, as it does not require using look-ahead, but it is perhaps not too onerous to only accept it for `fn`s (and all their various qualifiers), `const`s, `static`s, and `type`s.

r? @petrochenkov
  • Loading branch information
Centril authored Feb 18, 2020
2 parents 981acd9 + 045b7d5 commit b864d23
Show file tree
Hide file tree
Showing 77 changed files with 1,314 additions and 714 deletions.
102 changes: 53 additions & 49 deletions src/librustc_ast_lowering/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,26 +269,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
self.lower_use_tree(use_tree, &prefix, id, vis, ident, attrs)
}
ItemKind::Static(ref t, m, ref e) => {
let ty = self.lower_ty(
t,
if self.sess.features_untracked().impl_trait_in_bindings {
ImplTraitContext::OpaqueTy(None, hir::OpaqueTyOrigin::Misc)
} else {
ImplTraitContext::Disallowed(ImplTraitPosition::Binding)
},
);
hir::ItemKind::Static(ty, m, self.lower_const_body(span, Some(e)))
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) => {
let ty = self.lower_ty(
t,
if self.sess.features_untracked().impl_trait_in_bindings {
ImplTraitContext::OpaqueTy(None, hir::OpaqueTyOrigin::Misc)
} else {
ImplTraitContext::Disallowed(ImplTraitPosition::Binding)
},
);
hir::ItemKind::Const(ty, self.lower_const_body(span, Some(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) => {
let fn_def_id = self.resolver.definitions().local_def_id(id);
Expand Down Expand Up @@ -457,6 +443,21 @@ impl<'hir> LoweringContext<'_, 'hir> {
// not cause an assertion failure inside the `lower_defaultness` function.
}

fn lower_const_item(
&mut self,
ty: &Ty,
span: Span,
body: Option<&Expr>,
) -> (&'hir hir::Ty<'hir>, hir::BodyId) {
let itctx = if self.sess.features_untracked().impl_trait_in_bindings {
ImplTraitContext::OpaqueTy(None, hir::OpaqueTyOrigin::Misc)
} else {
ImplTraitContext::Disallowed(ImplTraitPosition::Binding)
};
let ty = self.lower_ty(ty, itctx);
(ty, self.lower_const_body(span, body))
}

fn lower_use_tree(
&mut self,
tree: &UseTree,
Expand Down Expand Up @@ -678,11 +679,16 @@ impl<'hir> LoweringContext<'_, 'hir> {

hir::ForeignItemKind::Fn(fn_dec, fn_args, generics)
}
ForeignItemKind::Static(ref t, m) => {
ForeignItemKind::Static(ref t, m, _) => {
let ty = self.lower_ty(t, ImplTraitContext::disallowed());
hir::ForeignItemKind::Static(ty, m)
}
ForeignItemKind::Ty => hir::ForeignItemKind::Type,
ForeignItemKind::Const(ref t, _) => {
// For recovery purposes.
let ty = self.lower_ty(t, ImplTraitContext::disallowed());
hir::ForeignItemKind::Static(ty, Mutability::Not)
}
ForeignItemKind::TyAlias(..) => hir::ForeignItemKind::Type,
ForeignItemKind::Macro(_) => panic!("macro shouldn't exist here"),
},
vis: self.lower_visibility(&i.vis, None),
Expand Down Expand Up @@ -759,32 +765,27 @@ impl<'hir> LoweringContext<'_, 'hir> {
let trait_item_def_id = self.resolver.definitions().local_def_id(i.id);

let (generics, kind) = match i.kind {
AssocItemKind::Const(ref ty, ref default) => {
let generics = self.lower_generics(&i.generics, ImplTraitContext::disallowed());
AssocItemKind::Static(ref ty, _, ref default) // Let's pretend this is a `const`.
| AssocItemKind::Const(ref ty, ref default) => {
let ty = self.lower_ty(ty, ImplTraitContext::disallowed());
(
generics,
hir::TraitItemKind::Const(
ty,
default.as_ref().map(|x| self.lower_const_body(i.span, Some(x))),
),
)
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, 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(&i.generics, sig, trait_item_def_id, false, None);
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, 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(&i.generics, sig, trait_item_def_id, false, None);
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 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(&i.generics, ImplTraitContext::disallowed());
let generics = self.lower_generics(generics, ImplTraitContext::disallowed());
let kind = hir::TraitItemKind::Type(
self.lower_param_bounds(bounds, ImplTraitContext::disallowed()),
ty,
Expand All @@ -806,10 +807,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
}

fn lower_trait_item_ref(&mut self, i: &AssocItem) -> hir::TraitItemRef {
let (kind, has_default) = match i.kind {
AssocItemKind::Const(_, ref default) => (hir::AssocItemKind::Const, default.is_some()),
AssocItemKind::TyAlias(_, ref default) => (hir::AssocItemKind::Type, default.is_some()),
AssocItemKind::Fn(ref sig, ref default) => {
let (kind, has_default) = match &i.kind {
AssocItemKind::Static(_, _, default) // Let's pretend this is a `const` for recovery.
| AssocItemKind::Const(_, default) => {
(hir::AssocItemKind::Const, default.is_some())
}
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!(),
Expand All @@ -832,22 +836,21 @@ impl<'hir> LoweringContext<'_, 'hir> {
let impl_item_def_id = self.resolver.definitions().local_def_id(i.id);

let (generics, kind) = match i.kind {
AssocItemKind::Const(ref ty, ref expr) => {
let generics = self.lower_generics(&i.generics, ImplTraitContext::disallowed());
AssocItemKind::Static(ref ty, _, ref expr) | AssocItemKind::Const(ref ty, ref expr) => {
let ty = self.lower_ty(ty, ImplTraitContext::disallowed());
(
generics,
hir::Generics::empty(),
hir::ImplItemKind::Const(ty, self.lower_const_body(i.span, expr.as_deref())),
)
}
AssocItemKind::Fn(ref sig, ref body) => {
AssocItemKind::Fn(ref sig, ref generics, ref body) => {
self.current_item = Some(i.span);
let asyncness = sig.header.asyncness;
let body_id =
self.lower_maybe_async_body(i.span, &sig.decl, asyncness, body.as_deref());
let impl_trait_return_allow = !self.is_in_trait_impl;
let (generics, sig) = self.lower_method_sig(
&i.generics,
generics,
sig,
impl_item_def_id,
impl_trait_return_allow,
Expand All @@ -856,8 +859,8 @@ impl<'hir> LoweringContext<'_, 'hir> {

(generics, hir::ImplItemKind::Method(sig, body_id))
}
AssocItemKind::TyAlias(_, ref ty) => {
let generics = self.lower_generics(&i.generics, ImplTraitContext::disallowed());
AssocItemKind::TyAlias(ref generics, _, ref ty) => {
let generics = self.lower_generics(generics, ImplTraitContext::disallowed());
let kind = match ty {
None => {
let ty = self.arena.alloc(self.ty(i.span, hir::TyKind::Err));
Expand Down Expand Up @@ -901,14 +904,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
vis: self.lower_visibility(&i.vis, Some(i.id)),
defaultness: self.lower_defaultness(i.defaultness, true /* [1] */),
kind: match &i.kind {
AssocItemKind::Const(..) => hir::AssocItemKind::Const,
AssocItemKind::TyAlias(_, ty) => {
AssocItemKind::Static(..) // Let's pretend this is a `const` for recovery.
| AssocItemKind::Const(..) => hir::AssocItemKind::Const,
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
2 changes: 1 addition & 1 deletion src/librustc_ast_lowering/lib.rs
Original file line number Diff line number Diff line change
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
97 changes: 89 additions & 8 deletions src/librustc_ast_passes/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ use syntax::expand::is_proc_macro_attr;
use syntax::visit::{self, AssocCtxt, FnCtxt, FnKind, Visitor};
use syntax::walk_list;

const MORE_EXTERN: &str =
"for more information, visit https://doc.rust-lang.org/std/keyword.extern.html";

/// Is `self` allowed semantically as the first parameter in an `FnDecl`?
enum SelfSemantic {
Yes,
Expand Down Expand Up @@ -423,14 +426,62 @@ impl<'a> AstValidator<'a> {
}
}

fn check_impl_assoc_type_no_bounds(&self, bounds: &[GenericBound]) {
fn check_type_no_bounds(&self, bounds: &[GenericBound], ctx: &str) {
let span = match bounds {
[] => return,
[b0] => b0.span(),
[b0, .., bl] => b0.span().to(bl.span()),
};
self.err_handler()
.struct_span_err(span, "bounds on associated `type`s in `impl`s have no effect")
.struct_span_err(span, &format!("bounds on `type`s in {} have no effect", ctx))
.emit();
}

fn check_foreign_ty_genericless(&self, generics: &Generics) {
let cannot_have = |span, descr, remove_descr| {
self.err_handler()
.struct_span_err(
span,
&format!("`type`s inside `extern` blocks cannot have {}", descr),
)
.span_suggestion(
span,
&format!("remove the {}", remove_descr),
String::new(),
Applicability::MaybeIncorrect,
)
.span_label(self.current_extern_span(), "`extern` block begins here")
.note(MORE_EXTERN)
.emit();
};

if !generics.params.is_empty() {
cannot_have(generics.span, "generic parameters", "generic parameters");
}

if !generics.where_clause.predicates.is_empty() {
cannot_have(generics.where_clause.span, "`where` clauses", "`where` clause");
}
}

fn check_foreign_kind_bodyless(&self, ident: Ident, kind: &str, body: Option<Span>) {
let body = match body {
None => return,
Some(body) => body,
};
self.err_handler()
.struct_span_err(ident.span, &format!("incorrect `{}` inside `extern` block", kind))
.span_label(ident.span, "cannot have a body")
.span_label(body, "the invalid body")
.span_label(
self.current_extern_span(),
format!(
"`extern` blocks define existing foreign {0}s and {0}s \
inside of them cannot have a body",
kind
),
)
.note(MORE_EXTERN)
.emit();
}

Expand Down Expand Up @@ -458,7 +509,7 @@ impl<'a> AstValidator<'a> {
"`extern` blocks define existing foreign functions and functions \
inside of them cannot have a body",
)
.note("for more information, visit https://doc.rust-lang.org/std/keyword.extern.html")
.note(MORE_EXTERN)
.emit();
}

Expand Down Expand Up @@ -531,6 +582,16 @@ impl<'a> AstValidator<'a> {
}
}
}

fn check_item_named(&self, ident: Ident, kind: &str) {
if ident.name != kw::Underscore {
return;
}
self.err_handler()
.struct_span_err(ident.span, &format!("`{}` items in this context need a name", kind))
.span_label(ident.span, format!("`_` is not a valid name for this `{}` item", kind))
.emit();
}
}

enum GenericPosition {
Expand Down Expand Up @@ -900,6 +961,14 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
self.err_handler().span_err(item.span, "unions cannot have zero fields");
}
}
ItemKind::Const(.., None) => {
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>;");
}
_ => {}
}

Expand All @@ -912,7 +981,15 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
self.check_foreign_fn_bodyless(fi.ident, body.as_deref());
self.check_foreign_fn_headerless(fi.ident, fi.span, sig.header);
}
ForeignItemKind::Static(..) | ForeignItemKind::Ty | ForeignItemKind::Macro(..) => {}
ForeignItemKind::TyAlias(generics, bounds, body) => {
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);
}
ForeignItemKind::Static(_, _, body) => {
self.check_foreign_kind_bodyless(fi.ident, "static", body.as_ref().map(|b| b.span));
}
ForeignItemKind::Const(..) | ForeignItemKind::Macro(..) => {}
}

visit::walk_foreign_item(self, fi)
Expand Down Expand Up @@ -1154,25 +1231,29 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
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_impl_assoc_type_no_bounds(bounds);
self.check_type_no_bounds(bounds, "`impl`s");
}
_ => {}
}
}

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);
}
}

if let AssocItemKind::Const(..) = item.kind {
self.check_item_named(item.ident, "const");
}

self.with_in_trait_impl(false, |this| visit::walk_assoc_item(this, item, ctxt));
}
}
Expand Down
Loading

0 comments on commit b864d23

Please sign in to comment.