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

Jump to bottom

Implement associated existential types #52650

Merged
merged 4 commits into from
Jul 27, 2018
Merged

Implement associated existential types #52650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9130efd
Implement associated existential types
oli-obk Jul 23, 2018
93974cb
Fix associated existentials for generic traits
oli-obk Jul 25, 2018
01eacd8
Remove useless comment
oli-obk Jul 25, 2018
33712a8
Add type system canaries for potential future bugs
oli-obk Jul 25, 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
52 changes: 34 additions & 18 deletions src/librustc/infer/anon_types/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,25 +691,41 @@ impl<'a, 'gcx, 'tcx> Instantiator<'a, 'gcx, 'tcx> {
// }
// ```
if let Some(anon_node_id) = tcx.hir.as_local_node_id(def_id) {
let in_definition_scope = match tcx.hir.expect_item(anon_node_id).node {
// impl trait
hir::ItemKind::Existential(hir::ExistTy {
impl_trait_fn: Some(parent),
..
}) => parent == self.parent_def_id,
// named existential types
hir::ItemKind::Existential(hir::ExistTy {
impl_trait_fn: None,
..
}) => may_define_existential_type(
tcx,
self.parent_def_id,
anon_node_id,
),
_ => {
let anon_parent_node_id = tcx.hir.get_parent(anon_node_id);
self.parent_def_id == tcx.hir.local_def_id(anon_parent_node_id)
let parent_def_id = self.parent_def_id;
let def_scope_default = || {
let anon_parent_node_id = tcx.hir.get_parent(anon_node_id);
parent_def_id == tcx.hir.local_def_id(anon_parent_node_id)
};
let in_definition_scope = match tcx.hir.find(anon_node_id) {
Some(hir::map::NodeItem(item)) => match item.node {
// impl trait
hir::ItemKind::Existential(hir::ExistTy {
impl_trait_fn: Some(parent),
..
}) => parent == self.parent_def_id,
// named existential types
hir::ItemKind::Existential(hir::ExistTy {
impl_trait_fn: None,
..
}) => may_define_existential_type(
tcx,
self.parent_def_id,
anon_node_id,
),
_ => def_scope_default(),
},
Some(hir::map::NodeImplItem(item)) => match item.node {
hir::ImplItemKind::Existential(_) => may_define_existential_type(
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess one of the wacky -- but perhaps .. fine -- consequences of this is that one could do something like

impl Foo for Bar {
    existential type Item;

    fn method() {
        fn helper() -> <Bar as Foo>::Item { .. }
    }

and that helper fn might count as a defining use?

I feel like we should only allow other members of the impl to be a defining use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not allowing this kind of defining use would be inconsistent with how normal existential types work.

Copy link
Contributor

@nikomatsakis nikomatsakis Jul 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I don't see it as particularly inconsistent — depends on your POV, I suppose. It seems to me that the various "sugared forms" (e.g., impl Trait) don't desugar directly to an existential type "sibling item", because they have different sets of defining uses... in the case of a function, only the fn return type in question. In the case of an impl, we get to decide?

For that matter, I do wonder if we would rather have the syntax here be type Item = impl Foo.

(cc @cramertj on this particular question)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm ok landing either way, but I would want to open an unresolved question on this point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea the syntax is horrible on purpose. It's the only one everyone could agree on that we do not want.

Copy link
Contributor

@nikomatsakis nikomatsakis Jul 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I added two unresolved questions to the tracking issue. Dear god I hope they won't get lost. =)

Copy link
Member

Choose a reason for hiding this comment

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

IMO it's fine for helper to be a defining use of Item because it's "in the scope" of Item's definition (in this case, the trait impl).

Copy link
Contributor

Choose a reason for hiding this comment

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

Was there another discussion on syntax? Two points:

  • sometimes there are no constraints: but type Item; is of course ambiguous
  • "existential" is not really the correct term (mathematically it means "there exists at least one such ..."); really we just mean "indirectly specified"

So possibly indirect type Item; or unspec type Item; ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This implemented the placeholder syntax from the RFC. Please check out the discussion on the tracking issue and follow up RFCs. The syntax has been heavily discussed there and is currently being RFCed as type Item = impl Trait;. So if you disagree with the proposed syntax or otherwise have opinions on this topic, bring it up on that RFC.

tcx,
self.parent_def_id,
anon_node_id,
),
_ => def_scope_default(),
},
_ => bug!(
"expected (impl) item, found {}",
tcx.hir.node_to_string(anon_node_id),
),
};
if in_definition_scope {
return self.fold_anon_ty(ty, def_id, substs);
Expand Down
13 changes: 10 additions & 3 deletions src/librustc/traits/project.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1502,19 +1502,26 @@ fn confirm_impl_candidate<'cx, 'gcx, 'tcx>(
let param_env = obligation.param_env;
let assoc_ty = assoc_ty_def(selcx, impl_def_id, obligation.predicate.item_def_id);

let ty = if !assoc_ty.item.defaultness.has_value() {
if !assoc_ty.item.defaultness.has_value() {
// This means that the impl is missing a definition for the
// associated type. This error will be reported by the type
// checker method `check_impl_items_against_trait`, so here we
// just return TyError.
debug!("confirm_impl_candidate: no associated type {:?} for {:?}",
assoc_ty.item.ident,
obligation.predicate);
tcx.types.err
return Progress {
ty: tcx.types.err,
obligations: nested,
};
}
let substs = translate_substs(selcx.infcx(), param_env, impl_def_id, substs, assoc_ty.node);
let ty = if let ty::AssociatedKind::Existential = assoc_ty.item.kind {
let item_substs = Substs::identity_for_item(tcx, assoc_ty.item.def_id);
tcx.mk_anon(assoc_ty.item.def_id, item_substs)
} else {
tcx.type_of(assoc_ty.item.def_id)
};
let substs = translate_substs(selcx.infcx(), param_env, impl_def_id, substs, assoc_ty.node);
Progress {
ty: ty.subst(tcx, substs),

This comment was marked as resolved.

Sign in to view

obligations: nested,
Expand Down
16 changes: 13 additions & 3 deletions src/librustc/traits/specialize/specialization_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,9 +377,19 @@ impl<'a, 'gcx, 'tcx> Ancestors {
trait_def_id: DefId,
) -> impl Iterator<Item = NodeItem<ty::AssociatedItem>> + Captures<'gcx> + Captures<'tcx> + 'a {
self.flat_map(move |node| {
node.items(tcx).filter(move |impl_item| {
impl_item.kind == trait_item_kind &&
tcx.hygienic_eq(impl_item.ident, trait_item_name, trait_def_id)
use ty::AssociatedKind::*;
node.items(tcx).filter(move |impl_item| match (trait_item_kind, impl_item.kind) {
| (Const, Const)
| (Method, Method)
| (Type, Type)
| (Type, Existential)
=> tcx.hygienic_eq(impl_item.ident, trait_item_name, trait_def_id),

| (Const, _)
| (Method, _)
| (Type, _)
| (Existential, _)
=> false,
}).map(move |item| NodeItem { node: node, item: item })
})
}
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_typeck/check/wfcheck.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,7 @@ fn check_associated_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
}
}
ty::AssociatedKind::Existential => {
// FIXME(oli-obk) implement existential types in trait impls
unimplemented!()
// do nothing, existential types check themselves
}
}

Expand Down
28 changes: 24 additions & 4 deletions src/librustc_typeck/collect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1046,12 +1046,12 @@ fn type_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
tcx.mk_fn_def(def_id, substs)
}
ImplItemKind::Const(ref ty, _) => icx.to_ty(ty),
ImplItemKind::Existential(ref _bounds) => {
ImplItemKind::Existential(_) => {
if tcx.impl_trait_ref(tcx.hir.get_parent_did(node_id)).is_none() {
report_assoc_ty_on_inherent_impl(tcx, item.span);
}
// FIXME(oli-obk) implement existential types in trait impls
unimplemented!()

find_existential_constraints(tcx, def_id)
}
ImplItemKind::Type(ref ty) => {
if tcx.impl_trait_ref(tcx.hir.get_parent_did(node_id)).is_none() {
Expand Down Expand Up @@ -1186,8 +1186,10 @@ fn find_existential_constraints<'a, 'tcx>(
}
impl<'a, 'tcx> ConstraintLocator<'a, 'tcx> {
fn check(&mut self, def_id: DefId) {
trace!("checking {:?}", def_id);
// don't try to check items that cannot possibly constrain the type
if !self.tcx.has_typeck_tables(def_id) {
trace!("no typeck tables for {:?}", def_id);
return;
}
let ty = self
Expand Down Expand Up @@ -1244,9 +1246,11 @@ fn find_existential_constraints<'a, 'tcx>(
let mut locator = ConstraintLocator { def_id, tcx, found: None };
let node_id = tcx.hir.as_local_node_id(def_id).unwrap();
let parent = tcx.hir.get_parent(node_id);
trace!("parent_id: {:?}", parent);
if parent == ast::CRATE_NODE_ID {
intravisit::walk_crate(&mut locator, tcx.hir.krate());
} else {
trace!("parent: {:?}", tcx.hir.get(parent));
match tcx.hir.get(parent) {
NodeItem(ref it) => intravisit::walk_item(&mut locator, it),
NodeImplItem(ref it) => intravisit::walk_impl_item(&mut locator, it),
Expand Down Expand Up @@ -1485,7 +1489,23 @@ fn explicit_predicates_of<'a, 'tcx>(
&item.generics
}

NodeImplItem(item) => &item.generics,
NodeImplItem(item) => match item.node {
ImplItemKind::Existential(ref bounds) => {
let substs = Substs::identity_for_item(tcx, def_id);
let anon_ty = tcx.mk_anon(def_id, substs);

// Collect the bounds, i.e. the `A+B+'c` in `impl A+B+'c`.
let bounds = compute_bounds(&icx,
anon_ty,
bounds,
SizedByDefault::Yes,
tcx.def_span(def_id));

predicates.extend(bounds.predicates(tcx, anon_ty));
&item.generics
},
_ => &item.generics,
}

NodeItem(item) => {
match item.node {
Expand Down
40 changes: 40 additions & 0 deletions src/test/ui/impl-trait/associated-existential-type-generic-trait.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2018 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(existential_type)]
// compile-pass

trait Bar {}
struct Dummy<U>(U);
impl<V> Bar for Dummy<V> {}

trait Foo<T> {
type Assoc: Bar;
fn foo(t: T) -> Self::Assoc;
}

impl<W> Foo<W> for i32 {
existential type Assoc: Bar;
fn foo(w: W) -> Self::Assoc {
Dummy(w)
}
}

struct NonGeneric;
impl Bar for NonGeneric {}

impl<W> Foo<W> for u32 {
existential type Assoc: Bar;
fn foo(_: W) -> Self::Assoc {
NonGeneric
}
}

fn main() {}
30 changes: 30 additions & 0 deletions src/test/ui/impl-trait/associated-existential-type-trivial.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2018 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(existential_type)]
// compile-pass

trait Bar {}
struct Dummy;
impl Bar for Dummy {}

trait Foo {
type Assoc: Bar;
fn foo() -> Self::Assoc;
}

impl Foo for i32 {
existential type Assoc: Bar;
fn foo() -> Self::Assoc {
Dummy
}
}

fn main() {}
34 changes: 34 additions & 0 deletions src/test/ui/impl-trait/associated-existential-type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright 2018 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(existential_type)]
// compile-pass

trait Bar {}
struct Dummy;
impl Bar for Dummy {}

trait Foo {
type Assoc: Bar;
fn foo() -> Self::Assoc;
fn bar() -> Self::Assoc;
}

impl Foo for i32 {
existential type Assoc: Bar;
fn foo() -> Self::Assoc {
Dummy
}
fn bar() -> Self::Assoc {
Dummy
}
}

fn main() {}