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

Generic Associated Types Parsing & Name Resolution #45904

Merged
merged 19 commits into from
Dec 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
143 changes: 70 additions & 73 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -734,8 +734,8 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> {
FnKind::ItemFn(..) => {
ItemRibKind
}
FnKind::Method(_, sig, _, _) => {
MethodRibKind(!sig.decl.has_self())
FnKind::Method(_, _, _, _) => {
TraitOrImplItemRibKind
}
FnKind::Closure(_) => ClosureRibKind(node_id),
};
Expand Down Expand Up @@ -823,12 +823,10 @@ enum RibKind<'a> {
ClosureRibKind(NodeId /* func id */),

// We passed through an impl or trait and are now in one of its
// methods. Allow references to ty params that impl or trait
// methods or associated types. Allow references to ty params that impl or trait
// binds. Disallow any other upvars (including other ty params that are
// upvars).
//
// The boolean value represents the fact that this method is static or not.
MethodRibKind(bool),
TraitOrImplItemRibKind,

// We passed through an item scope. Disallow upvars.
ItemRibKind,
Expand Down Expand Up @@ -1888,34 +1886,33 @@ impl<'a> Resolver<'a> {
for trait_item in trait_items {
this.check_proc_macro_attrs(&trait_item.attrs);

match trait_item.node {
TraitItemKind::Const(ref ty, ref default) => {
this.visit_ty(ty);

// Only impose the restrictions of
// ConstRibKind for an actual constant
// expression in a provided default.
if let Some(ref expr) = *default{
this.with_constant_rib(|this| {
this.visit_expr(expr);
});
let type_parameters = HasTypeParameters(&trait_item.generics,
TraitOrImplItemRibKind);
this.with_type_parameter_rib(type_parameters, |this| {
match trait_item.node {
TraitItemKind::Const(ref ty, ref default) => {
this.visit_ty(ty);

// Only impose the restrictions of
// ConstRibKind for an actual constant
// expression in a provided default.
if let Some(ref expr) = *default{
this.with_constant_rib(|this| {
this.visit_expr(expr);
});
}
}
}
TraitItemKind::Method(ref sig, _) => {
let type_parameters =
HasTypeParameters(&trait_item.generics,
MethodRibKind(!sig.decl.has_self()));
this.with_type_parameter_rib(type_parameters, |this| {
TraitItemKind::Method(_, _) => {
visit::walk_trait_item(this, trait_item)
});
}
TraitItemKind::Type(..) => {
this.with_type_parameter_rib(NoTypeParameters, |this| {
}
TraitItemKind::Type(..) => {
visit::walk_trait_item(this, trait_item)
});
}
TraitItemKind::Macro(_) => panic!("unexpanded macro in resolve!"),
};
}
TraitItemKind::Macro(_) => {
panic!("unexpanded macro in resolve!")
}
};
});
}
});
});
Expand Down Expand Up @@ -2123,48 +2120,48 @@ impl<'a> Resolver<'a> {
for impl_item in impl_items {
this.check_proc_macro_attrs(&impl_item.attrs);
this.resolve_visibility(&impl_item.vis);
match impl_item.node {
ImplItemKind::Const(..) => {
// If this is a trait impl, ensure the const
// exists in trait
this.check_trait_item(impl_item.ident,
ValueNS,
impl_item.span,
|n, s| ResolutionError::ConstNotMemberOfTrait(n, s));
this.with_constant_rib(|this|
visit::walk_impl_item(this, impl_item)
);
}
ImplItemKind::Method(ref sig, _) => {
// If this is a trait impl, ensure the method
// exists in trait
this.check_trait_item(impl_item.ident,
ValueNS,
impl_item.span,
|n, s| ResolutionError::MethodNotMemberOfTrait(n, s));

// We also need a new scope for the method-
// specific type parameters.
let type_parameters =
HasTypeParameters(&impl_item.generics,
MethodRibKind(!sig.decl.has_self()));
this.with_type_parameter_rib(type_parameters, |this| {
visit::walk_impl_item(this, impl_item);
});
}
ImplItemKind::Type(ref ty) => {
// If this is a trait impl, ensure the type
// exists in trait
this.check_trait_item(impl_item.ident,
TypeNS,
impl_item.span,
|n, s| ResolutionError::TypeNotMemberOfTrait(n, s));

this.visit_ty(ty);
// We also need a new scope for the impl item type parameters.
let type_parameters = HasTypeParameters(&impl_item.generics,
TraitOrImplItemRibKind);
this.with_type_parameter_rib(type_parameters, |this| {
use self::ResolutionError::*;
match impl_item.node {
ImplItemKind::Const(..) => {
// If this is a trait impl, ensure the const
// exists in trait
this.check_trait_item(impl_item.ident,
ValueNS,
impl_item.span,
|n, s| ConstNotMemberOfTrait(n, s));
this.with_constant_rib(|this|
visit::walk_impl_item(this, impl_item)
);
}
ImplItemKind::Method(_, _) => {
// If this is a trait impl, ensure the method
// exists in trait
this.check_trait_item(impl_item.ident,
ValueNS,
impl_item.span,
|n, s| MethodNotMemberOfTrait(n, s));

visit::walk_impl_item(this, impl_item);
}
ImplItemKind::Type(ref ty) => {
// If this is a trait impl, ensure the type
// exists in trait
this.check_trait_item(impl_item.ident,
TypeNS,
impl_item.span,
|n, s| TypeNotMemberOfTrait(n, s));

this.visit_ty(ty);
}
ImplItemKind::Macro(_) =>
panic!("unexpanded macro in resolve!"),
}
ImplItemKind::Macro(_) =>
panic!("unexpanded macro in resolve!"),
}
});
}
});
});
Expand Down Expand Up @@ -3100,7 +3097,7 @@ impl<'a> Resolver<'a> {
seen.insert(node_id, depth);
}
}
ItemRibKind | MethodRibKind(_) => {
ItemRibKind | TraitOrImplItemRibKind => {
// This was an attempt to access an upvar inside a
// named function item. This is not allowed, so we
// report an error.
Expand All @@ -3124,7 +3121,7 @@ impl<'a> Resolver<'a> {
Def::TyParam(..) | Def::SelfTy(..) => {
for rib in ribs {
match rib.kind {
NormalRibKind | MethodRibKind(_) | ClosureRibKind(..) |
NormalRibKind | TraitOrImplItemRibKind | ClosureRibKind(..) |
ModuleRibKind(..) | MacroDefinition(..) | ForwardTyParamBanRibKind |
ConstantItemRibKind => {
// Nothing to do. Continue.
Expand Down
21 changes: 18 additions & 3 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,9 @@ declare_features! (

// Nested groups in `use` (RFC 2128)
(active, use_nested_groups, "1.23.0", Some(44494)),

// generic associated types (RFC 1598)
(active, generic_associated_types, "1.23.0", Some(44265)),
);

declare_features! (
Expand Down Expand Up @@ -1614,9 +1617,17 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
gate_feature_post!(&self, const_fn, ti.span, "const fn is unstable");
}
}
ast::TraitItemKind::Type(_, Some(_)) => {
gate_feature_post!(&self, associated_type_defaults, ti.span,
"associated type defaults are unstable");
ast::TraitItemKind::Type(_, ref default) => {
// We use two if statements instead of something like match guards so that both
// of these errors can be emitted if both cases apply.
if default.is_some() {
gate_feature_post!(&self, associated_type_defaults, ti.span,
"associated type defaults are unstable");
}
if ti.generics.is_parameterized() {
gate_feature_post!(&self, generic_associated_types, ti.span,
"generic associated types are unstable");
}
}
_ => {}
}
Expand All @@ -1636,6 +1647,10 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
gate_feature_post!(&self, const_fn, ii.span, "const fn is unstable");
}
}
ast::ImplItemKind::Type(_) if ii.generics.is_parameterized() => {
gate_feature_post!(&self, generic_associated_types, ii.span,
"generic associated types are unstable");
}
_ => {}
}
visit::walk_impl_item(self, ii);
Expand Down
47 changes: 43 additions & 4 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1293,9 +1293,9 @@ impl<'a> Parser<'a> {
let lo = self.span;

let (name, node, generics) = if self.eat_keyword(keywords::Type) {
let TyParam {ident, bounds, default, ..} = self.parse_ty_param(vec![])?;
self.expect(&token::Semi)?;
(ident, TraitItemKind::Type(bounds, default), ast::Generics::default())
let (generics, TyParam {ident, bounds, default, ..}) =
self.parse_trait_item_assoc_ty(vec![])?;
(ident, TraitItemKind::Type(bounds, default), generics)
} else if self.is_const_item() {
self.expect_keyword(keywords::Const)?;
let ident = self.parse_ident()?;
Expand Down Expand Up @@ -4442,6 +4442,39 @@ impl<'a> Parser<'a> {
})
}

/// Parses the following grammar:
/// TraitItemAssocTy = Ident ["<"...">"] [":" [TyParamBounds]] ["where" ...] ["=" Ty]
fn parse_trait_item_assoc_ty(&mut self, preceding_attrs: Vec<Attribute>)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikomatsakis Is there a reason why parse_trait_item_assoc_ty doesn't consume the semicolon? I think it really could given that it's only used in one place and it wouldn't make any sense to parse all of this without the semicolon at the end. The only invocation of this function immediately expects a semicolon right after it.

(added this question again because GitHub removed it on the latest diff)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't make a big difference either way. A lot of times it's good to leave separators and terminators out from functions like this, since it makes them more re-usable, but I think in this case the ; logically "belongs" to the trait item, so it would make sense for this function to consume it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the latest code, the semicolon is consumed.

-> PResult<'a, (ast::Generics, TyParam)> {
let span = self.span;
let ident = self.parse_ident()?;
let mut generics = self.parse_generics()?;

// Parse optional colon and param bounds.
let bounds = if self.eat(&token::Colon) {
self.parse_ty_param_bounds()?
} else {
Vec::new()
};
generics.where_clause = self.parse_where_clause()?;

let default = if self.eat(&token::Eq) {
Some(self.parse_ty()?)
} else {
None
};
self.expect(&token::Semi)?;

Ok((generics, TyParam {
attrs: preceding_attrs.into(),
ident,
id: ast::DUMMY_NODE_ID,
bounds,
default,
span,
}))
}

/// Parses (possibly empty) list of lifetime and type parameters, possibly including
/// trailing comma and erroneous trailing attributes.
pub fn parse_generic_params(&mut self) -> PResult<'a, (Vec<LifetimeDef>, Vec<TyParam>)> {
Expand Down Expand Up @@ -4983,12 +5016,18 @@ impl<'a> Parser<'a> {
let vis = self.parse_visibility(false)?;
let defaultness = self.parse_defaultness()?;
let (name, node, generics) = if self.eat_keyword(keywords::Type) {
// This parses the grammar:
// ImplItemAssocTy = Ident ["<"...">"] ["where" ...] "=" Ty ";"
let name = self.parse_ident()?;
let mut generics = self.parse_generics()?;
generics.where_clause = self.parse_where_clause()?;
self.expect(&token::Eq)?;
let typ = self.parse_ty()?;
self.expect(&token::Semi)?;
(name, ast::ImplItemKind::Type(typ), ast::Generics::default())
(name, ast::ImplItemKind::Type(typ), generics)
} else if self.is_const_item() {
// This parses the grammar:
// ImplItemConst = "const" Ident ":" Ty "=" Expr ";"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment isn't relevant to what I'm adding, but I added it because @nikomatsakis mentioned that it's good to document the grammar in places like this

self.expect_keyword(keywords::Const)?;
let name = self.parse_ident()?;
self.expect(&token::Colon)?;
Expand Down
28 changes: 28 additions & 0 deletions src/test/compile-fail/feature-gate-generic_associated_types.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::ops::Deref;

trait PointerFamily<U> {
type Pointer<T>: Deref<Target = T>;
//~^ ERROR generic associated types are unstable
type Pointer2<T>: Deref<Target = T> where T: Clone, U: Clone;
//~^ ERROR generic associated types are unstable
}

struct Foo;
impl PointerFamily<u32> for Foo {
type Pointer<usize> = Box<usize>;
//~^ ERROR generic associated types are unstable
type Pointer2<u32> = Box<u32>;
//~^ ERROR generic associated types are unstable
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(generic_associated_types)]

//FIXME(#44265): "undeclared lifetime" errors will be addressed in a follow-up PR

trait Foo {
type Bar<'a, 'b>;
}

trait Baz {
type Quux<'a>;
}

impl<T> Baz for T where T: Foo {
type Quux<'a> = <T as Foo>::Bar<'a, 'static>;
//~^ ERROR undeclared lifetime
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error[E0261]: use of undeclared lifetime name `'a`
--> $DIR/construct_with_other_type.rs:24:37
|
24 | type Quux<'a> = <T as Foo>::Bar<'a, 'static>;
| ^^ undeclared lifetime

error: aborting due to previous error

18 changes: 18 additions & 0 deletions src/test/ui/rfc1598-generic-associated-types/empty_generics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(generic_associated_types)]

trait Foo {
type Bar<,>;
//~^ ERROR expected one of `>`, identifier, or lifetime, found `,`
}

fn main() {}
Loading