Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

parse: fuse associated and extern items up to defaultness #69194

Merged
merged 16 commits into from
Feb 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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