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.

Sign up for GitHub

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

Nest the impl Trait existential item inside the return type #54741

Merged
merged 2 commits into from
Oct 5, 2018
Merged

Nest the impl Trait existential item inside the return type #54741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a1ea44
Nest the `impl Trait` existential item inside the return type
oli-obk Oct 2, 2018
c3151c6
Fix rustdoc
oli-obk Oct 2, 2018
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
4 changes: 4 additions & 0 deletions src/librustc/hir/intravisit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,10 @@ pub fn walk_ty<'v, V: Visitor<'v>>(visitor: &mut V, typ: &'v Ty) {
TyKind::Path(ref qpath) => {
visitor.visit_qpath(qpath, typ.hir_id, typ.span);
}
TyKind::Def(item_id, ref lifetimes) => {
visitor.visit_nested_item(item_id);
walk_list!(visitor, visit_generic_arg, lifetimes);
}
TyKind::Array(ref ty, ref length) => {
visitor.visit_ty(ty);
visitor.visit_anon_const(length)
Expand Down
48 changes: 3 additions & 45 deletions src/librustc/hir/lowering.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1352,20 +1352,7 @@ impl<'a> LoweringContext<'a> {
lctx.items.insert(exist_ty_id.node_id, exist_ty_item);

// `impl Trait` now just becomes `Foo<'a, 'b, ..>`
let path = P(hir::Path {
span: exist_ty_span,
def: Def::Existential(DefId::local(exist_ty_def_index)),
segments: hir_vec![hir::PathSegment {
infer_types: false,
ident: Ident::new(keywords::Invalid.name(), exist_ty_span),
args: Some(P(hir::GenericArgs {
parenthesized: false,
bindings: HirVec::new(),
args: lifetimes,
}))
}],
});
hir::TyKind::Path(hir::QPath::Resolved(None, path))
hir::TyKind::Def(hir::ItemId { id: exist_ty_id.node_id }, lifetimes)
})
}

Expand Down Expand Up @@ -3207,23 +3194,6 @@ impl<'a> LoweringContext<'a> {
}
}

/// Lowers `impl Trait` items for a function and appends them to the list
fn lower_fn_impl_trait_ids(
&mut self,
decl: &FnDecl,
header: &FnHeader,
ids: &mut SmallVec<[hir::ItemId; 1]>,
) {
if let Some(id) = header.asyncness.opt_return_id() {
ids.push(hir::ItemId { id });
}
let mut visitor = ImplTraitTypeIdVisitor { ids };
match decl.output {
FunctionRetTy::Default(_) => {},
FunctionRetTy::Ty(ref ty) => visitor.visit_ty(ty),
}
}

fn lower_item_id(&mut self, i: &Item) -> SmallVec<[hir::ItemId; 1]> {
match i.node {
ItemKind::Use(ref use_tree) => {
Expand All @@ -3232,20 +3202,8 @@ impl<'a> LoweringContext<'a> {
vec
}
ItemKind::MacroDef(..) => SmallVec::new(),
ItemKind::Fn(ref decl, ref header, ..) => {
let mut ids = smallvec![hir::ItemId { id: i.id }];
self.lower_fn_impl_trait_ids(decl, header, &mut ids);
ids
},
ItemKind::Impl(.., None, _, ref items) => {
let mut ids = smallvec![hir::ItemId { id: i.id }];
for item in items {
if let ImplItemKind::Method(ref sig, _) = item.node {
self.lower_fn_impl_trait_ids(&sig.decl, &sig.header, &mut ids);
}
}
ids
},
ItemKind::Fn(..) |
ItemKind::Impl(.., None, _, _) => smallvec![hir::ItemId { id: i.id }],
ItemKind::Static(ref ty, ..) => {
let mut ids = smallvec![hir::ItemId { id: i.id }];
if self.sess.features_untracked().impl_trait_in_bindings {
Expand Down
6 changes: 6 additions & 0 deletions src/librustc/hir/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1723,6 +1723,12 @@ pub enum TyKind {
///
/// Type parameters may be stored in each `PathSegment`.
Path(QPath),
/// A type definition itself. This is currently only used for the `existential type`
/// item that `impl Trait` in return position desugars to.
///
/// The generic arg list are the lifetimes (and in the future possibly parameters) that are
/// actually bound on the `impl Trait`.
Def(ItemId, HirVec<GenericArg>),
Copy link
Member

Choose a reason for hiding this comment

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

I want something like this in general, maybe {...} block syntax for types?
Then -> impl Trait would be -> { existential type X: Trait; X }.
cc @Centril @nikomatsakis

Copy link
Contributor

Choose a reason for hiding this comment

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

In other words this would be legal?

type Foo = { u8 };

Not sure why it is useful tho...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isn't that essentially generic modules? (but unnamed)

Copy link
Member

Choose a reason for hiding this comment

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

@oli-obk I guess only if you can depend on parameters in scope.

@Centril for the same reason we have it for expressions :P.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess only if you can depend on parameters in scope.

which impl trait always does

Copy link
Contributor

Choose a reason for hiding this comment

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

@eddyb in exprs it exists for some sort of encapsulation purposes... I suppose you could do some sort of encapsulation of type aliases... but we should tread carefully here, there might be ambiguity in store.

PS: we should move this conversation elsewhere into a fresh issue / internals instead of comments on a merged PR (totally not discoverable...).

Copy link
Member

Choose a reason for hiding this comment

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

@Centril

I suppose you could do some sort of encapsulation of type aliases

Module-based encapsulation is how existential type X: Trait; works today.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cramertj I meant something like this:

type Foo = {
    type ComplexType = Bar<Baz<u8>>;
    Vec<ComplexType>
};

/// A trait object type `Bound1 + Bound2 + Bound3`
/// where `Bound` is a trait or a lifetime.
TraitObject(HirVec<PolyTraitRef>, Lifetime),
Expand Down
1 change: 1 addition & 0 deletions src/librustc/hir/print.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ impl<'a> State<'a> {
self.print_ty_fn(f.abi, f.unsafety, &f.decl, None, &f.generic_params,
&f.arg_names[..])?;
}
hir::TyKind::Def(..) => {},
hir::TyKind::Path(ref qpath) => {
self.print_qpath(qpath, false)?
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc/ich/impls_hir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ impl_stable_hash_for!(enum hir::TyKind {
Never,
Tup(ts),
Path(qpath),
Def(it, lt),
TraitObject(trait_refs, lifetime),
Typeof(body_id),
Err,
Expand Down
211 changes: 103 additions & 108 deletions src/librustc/middle/resolve_lifetime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,140 +621,135 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
};
self.with(scope, |_, this| this.visit_ty(&mt.ty));
}
hir::TyKind::Path(hir::QPath::Resolved(None, ref path)) => {
if let Def::Existential(exist_ty_did) = path.def {
let id = self.tcx.hir.as_local_node_id(exist_ty_did).unwrap();

// Resolve the lifetimes in the bounds to the lifetime defs in the generics.
// `fn foo<'a>() -> impl MyTrait<'a> { ... }` desugars to
// `abstract type MyAnonTy<'b>: MyTrait<'b>;`
// ^ ^ this gets resolved in the scope of
// the exist_ty generics
let (generics, bounds) = match self.tcx.hir.expect_item(id).node {
// named existential types don't need these hacks
hir::ItemKind::Existential(hir::ExistTy{ impl_trait_fn: None, .. }) => {
intravisit::walk_ty(self, ty);
return;
},
hir::ItemKind::Existential(hir::ExistTy{
ref generics,
ref bounds,
..
}) => (
generics,
bounds,
),
ref i => bug!("impl Trait pointed to non-existential type?? {:#?}", i),
};
hir::TyKind::Def(item_id, ref lifetimes) => {
// Resolve the lifetimes in the bounds to the lifetime defs in the generics.
// `fn foo<'a>() -> impl MyTrait<'a> { ... }` desugars to
// `abstract type MyAnonTy<'b>: MyTrait<'b>;`
// ^ ^ this gets resolved in the scope of
// the exist_ty generics
let (generics, bounds) = match self.tcx.hir.expect_item(item_id.id).node {
// named existential types are reached via TyKind::Path
// this arm is for `impl Trait` in the types of statics, constants and locals
hir::ItemKind::Existential(hir::ExistTy{ impl_trait_fn: None, .. }) => {
intravisit::walk_ty(self, ty);
return;
},
// RPIT (return position impl trait)
hir::ItemKind::Existential(hir::ExistTy{
ref generics,
ref bounds,
..
}) => (
generics,
bounds,
),
ref i => bug!("impl Trait pointed to non-existential type?? {:#?}", i),
};

// Resolve the lifetimes that are applied to the existential type.
// These are resolved in the current scope.
// `fn foo<'a>() -> impl MyTrait<'a> { ... }` desugars to
// `fn foo<'a>() -> MyAnonTy<'a> { ... }`
// ^ ^this gets resolved in the current scope
for lifetime in lifetimes {
if let hir::GenericArg::Lifetime(lifetime) = lifetime {
self.visit_lifetime(lifetime);

assert!(exist_ty_did.is_local());
// Resolve the lifetimes that are applied to the existential type.
// These are resolved in the current scope.
// `fn foo<'a>() -> impl MyTrait<'a> { ... }` desugars to
// `fn foo<'a>() -> MyAnonTy<'a> { ... }`
// ^ ^this gets resolved in the current scope
for lifetime in &path.segments[0].args.as_ref().unwrap().args {
if let hir::GenericArg::Lifetime(lifetime) = lifetime {
self.visit_lifetime(lifetime);

// Check for predicates like `impl for<'a> Trait<impl OtherTrait<'a>>`
// and ban them. Type variables instantiated inside binders aren't
// well-supported at the moment, so this doesn't work.
// In the future, this should be fixed and this error should be removed.
let def = self.map.defs.get(&lifetime.id).cloned();
if let Some(Region::LateBound(_, def_id, _)) = def {
if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) {
// Ensure that the parent of the def is an item, not HRTB
let parent_id = self.tcx.hir.get_parent_node(node_id);
let parent_impl_id = hir::ImplItemId { node_id: parent_id };
let parent_trait_id = hir::TraitItemId { node_id: parent_id };
let krate = self.tcx.hir.forest.krate();
if !(krate.items.contains_key(&parent_id)
|| krate.impl_items.contains_key(&parent_impl_id)
|| krate.trait_items.contains_key(&parent_trait_id))
{
span_err!(
self.tcx.sess,
lifetime.span,
E0657,
"`impl Trait` can only capture lifetimes \
bound at the fn or impl level"
);
self.uninsert_lifetime_on_error(lifetime, def.unwrap());
}
// Check for predicates like `impl for<'a> Trait<impl OtherTrait<'a>>`
// and ban them. Type variables instantiated inside binders aren't
// well-supported at the moment, so this doesn't work.
// In the future, this should be fixed and this error should be removed.
let def = self.map.defs.get(&lifetime.id).cloned();
if let Some(Region::LateBound(_, def_id, _)) = def {
if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) {
// Ensure that the parent of the def is an item, not HRTB
let parent_id = self.tcx.hir.get_parent_node(node_id);
let parent_impl_id = hir::ImplItemId { node_id: parent_id };
let parent_trait_id = hir::TraitItemId { node_id: parent_id };
let krate = self.tcx.hir.forest.krate();
if !(krate.items.contains_key(&parent_id)
|| krate.impl_items.contains_key(&parent_impl_id)
|| krate.trait_items.contains_key(&parent_trait_id))
{
span_err!(
self.tcx.sess,
lifetime.span,
E0657,
"`impl Trait` can only capture lifetimes \
bound at the fn or impl level"
);
self.uninsert_lifetime_on_error(lifetime, def.unwrap());
}
}
}
}
}

// We want to start our early-bound indices at the end of the parent scope,
// not including any parent `impl Trait`s.
let mut index = self.next_early_index_for_abstract_type();
debug!("visit_ty: index = {}", index);
// We want to start our early-bound indices at the end of the parent scope,
// not including any parent `impl Trait`s.
let mut index = self.next_early_index_for_abstract_type();
debug!("visit_ty: index = {}", index);

let mut elision = None;
let mut lifetimes = FxHashMap();
let mut type_count = 0;
for param in &generics.params {
match param.kind {
GenericParamKind::Lifetime { .. } => {
let (name, reg) = Region::early(&self.tcx.hir, &mut index, &param);
if let hir::ParamName::Plain(param_name) = name {
if param_name.name == keywords::UnderscoreLifetime.name() {
// Pick the elided lifetime "definition" if one exists
// and use it to make an elision scope.
elision = Some(reg);
} else {
lifetimes.insert(name, reg);
}
let mut elision = None;
let mut lifetimes = FxHashMap();
let mut type_count = 0;
for param in &generics.params {
match param.kind {
GenericParamKind::Lifetime { .. } => {
let (name, reg) = Region::early(&self.tcx.hir, &mut index, &param);
if let hir::ParamName::Plain(param_name) = name {
if param_name.name == keywords::UnderscoreLifetime.name() {
// Pick the elided lifetime "definition" if one exists
// and use it to make an elision scope.
elision = Some(reg);
} else {
lifetimes.insert(name, reg);
}
}
GenericParamKind::Type { .. } => {
type_count += 1;
} else {
lifetimes.insert(name, reg);
}
}
GenericParamKind::Type { .. } => {
type_count += 1;
}
}
let next_early_index = index + type_count;
}
let next_early_index = index + type_count;

if let Some(elision_region) = elision {
let scope = Scope::Elision {
elide: Elide::Exact(elision_region),
s: self.scope,
};
self.with(scope, |_old_scope, this| {
let scope = Scope::Binder {
lifetimes,
next_early_index,
s: this.scope,
track_lifetime_uses: true,
abstract_type_parent: false,
};
this.with(scope, |_old_scope, this| {
this.visit_generics(generics);
for bound in bounds {
this.visit_param_bound(bound);
}
});
});
} else {
if let Some(elision_region) = elision {
let scope = Scope::Elision {
elide: Elide::Exact(elision_region),
s: self.scope,
};
self.with(scope, |_old_scope, this| {
let scope = Scope::Binder {
lifetimes,
next_early_index,
s: self.scope,
s: this.scope,
track_lifetime_uses: true,
abstract_type_parent: false,
};
self.with(scope, |_old_scope, this| {
this.with(scope, |_old_scope, this| {
this.visit_generics(generics);
for bound in bounds {
this.visit_param_bound(bound);
}
});
}
});
} else {
intravisit::walk_ty(self, ty)
let scope = Scope::Binder {
lifetimes,
next_early_index,
s: self.scope,
track_lifetime_uses: true,
abstract_type_parent: false,
};
self.with(scope, |_old_scope, this| {
this.visit_generics(generics);
for bound in bounds {
this.visit_param_bound(bound);
}
});
}
}
_ => intravisit::walk_ty(self, ty),
Expand Down
9 changes: 5 additions & 4 deletions src/librustc_typeck/astconv.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1338,10 +1338,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
match path.def {
Def::Existential(did) => {
// check for desugared impl trait
if ty::is_impl_trait_defn(tcx, did).is_some() {
let lifetimes = &path.segments[0].args.as_ref().unwrap().args;
return self.impl_trait_ty_to_ty(did, lifetimes);
}
assert!(ty::is_impl_trait_defn(tcx, did).is_none());
let item_segment = path.segments.split_last().unwrap();
self.prohibit_generics(item_segment.1);
let substs = self.ast_path_substs_for_ty(span, did, item_segment.0);
Expand Down Expand Up @@ -1462,6 +1459,10 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx>+'o {
});
self.def_to_ty(opt_self_ty, path, false)
}
hir::TyKind::Def(item_id, ref lifetimes) => {
let did = tcx.hir.local_def_id(item_id.id);
self.impl_trait_ty_to_ty(did, lifetimes)
},
hir::TyKind::Path(hir::QPath::TypeRelative(ref qself, ref segment)) => {
debug!("ast_ty_to_ty: qself={:?} segment={:?}", qself, segment);
let ty = self.ast_ty_to_ty(qself);
Expand Down
Loading