Convert `T::Foo` resolution to use where clauses, not bounds #20300

Closed
nikomatsakis opened this Issue Dec 29, 2014 · 7 comments

Comments

Projects
None yet
8 participants
@nikomatsakis
Contributor

nikomatsakis commented Dec 29, 2014

Currently referencing an associated type like T::Foo searches the bounds on T rather than the full set of where clauses. This is no good.

@alexcrichton alexcrichton referenced this issue Dec 29, 2014

Closed

Implement associated items #17307

23 of 26 tasks complete

@nikomatsakis nikomatsakis added this to the 1.0 alpha milestone Jan 6, 2015

@nikomatsakis

This comment has been minimized.

Show comment
Hide comment
@nikomatsakis

nikomatsakis Jan 6, 2015

Contributor

Nominating and placing on 1.0 alpha milestone. Ultimately PROBABLY a nice to have, but it's really surprising that this doesn't work.

Contributor

nikomatsakis commented Jan 6, 2015

Nominating and placing on 1.0 alpha milestone. Ultimately PROBABLY a nice to have, but it's really surprising that this doesn't work.

@nikomatsakis nikomatsakis changed the title from Convert `T::Foo` resolution to use where clauses, not bounds` to Convert `T::Foo` resolution to use where clauses, not bounds Jan 8, 2015

@nikomatsakis nikomatsakis modified the milestones: 1.0 beta, 1.0 alpha Jan 9, 2015

@jroesch

This comment has been minimized.

Show comment
Hide comment
@jroesch

jroesch Jan 10, 2015

Member

cc me

Member

jroesch commented Jan 10, 2015

cc me

@erickt

This comment has been minimized.

Show comment
Hide comment
@erickt

erickt Jan 10, 2015

Contributor

Until this is implemented, you can also use a qualified type path <T as Trait>::Item:

trait Trait {
    type Item;
}

struct Foo;

impl Foo {
    fn foo1<T: Trait>(_x: T::Item) {}

    fn foo2<T>(_x: <T as Trait>::Item) where T: Trait {}
}

struct Bar;

impl Trait for Bar {
    type Item = i32;
}

fn main() {
    Foo::foo1::<Bar>(5i32);
    Foo::foo2::<Bar>(5i32);
}
Contributor

erickt commented Jan 10, 2015

Until this is implemented, you can also use a qualified type path <T as Trait>::Item:

trait Trait {
    type Item;
}

struct Foo;

impl Foo {
    fn foo1<T: Trait>(_x: T::Item) {}

    fn foo2<T>(_x: <T as Trait>::Item) where T: Trait {}
}

struct Bar;

impl Trait for Bar {
    type Item = i32;
}

fn main() {
    Foo::foo1::<Bar>(5i32);
    Foo::foo2::<Bar>(5i32);
}

@flaper87 flaper87 assigned flaper87 and unassigned flaper87 Jan 13, 2015

@brson brson added P-backcompat-lang and removed I-nominated labels Jan 15, 2015

@carllerche

This comment has been minimized.

Show comment
Hide comment
@carllerche

carllerche Jan 22, 2015

Member

cc me

Member

carllerche commented Jan 22, 2015

cc me

@ahmedcharles

This comment has been minimized.

Show comment
Hide comment
@ahmedcharles

ahmedcharles Jan 23, 2015

Contributor

Would anyone mind mentoring this?

Contributor

ahmedcharles commented Jan 23, 2015

Would anyone mind mentoring this?

@nikomatsakis

This comment has been minimized.

Show comment
Hide comment
@nikomatsakis

nikomatsakis Feb 2, 2015

Contributor

@ahmedcharles a fix is in progress already, actually, but it turns out to be quite a tricky problem. Much trickier than it appeared at first.

Contributor

nikomatsakis commented Feb 2, 2015

@ahmedcharles a fix is in progress already, actually, but it turns out to be quite a tricky problem. Much trickier than it appeared at first.

@freebroccolo

This comment has been minimized.

Show comment
Hide comment
@freebroccolo

freebroccolo Feb 6, 2015

Contributor

cc me

Contributor

freebroccolo commented Feb 6, 2015

cc me

Manishearth added a commit to Manishearth/rust that referenced this issue Feb 24, 2015

Rollup merge of #22512 - nikomatsakis:issue-20300-where-clause-not-bo…
…unds, r=nrc

 This is a fix for #20300 though as a side-sweep it fixes a number of stack overflows because it integrates cycle detection into the conversion process. I didn't go through and retest everything.

The tricky part here is that in some cases we have to go find the information we need from the AST -- we can't use the converted form of the where-clauses because we often have to handle something like `T::Item` *while converting the where-clauses themselves*. Since this is also not a fixed-point process we can't just try and keep trying to find the best order. So instead I modified the `AstConv` interface to allow you to request the bounds for a type parameter; we'll then do a secondary scan of the where-clauses to figure out what we need. This may create a cycle in some cases, but we have code to catch that.

Another approach that is NOT taken by this PR would be to \"convert\" `T::Item` into a form that does not specify what trait it's using. This then kind of defers the problem of picking the trait till later. That might be a good idea, but it would make normalization and everything else much harder, so I'm holding off on that (and hoping to find a better way for handling things like `i32::T`).

This PR also removes \"most of\" the `bounds` struct from `TypeParameterDef`. Still a little ways to go before `ParamBounds` can be removed entirely -- it's used for supertraits, for example (though those really ought to be converted, I think, to a call to `get_type_parameter_bounds` on `Self` from within the trait definition).

cc @jroesch

Fixes #20300

Manishearth added a commit to Manishearth/rust that referenced this issue Feb 24, 2015

Rollup merge of #22512 - nikomatsakis:issue-20300-where-clause-not-bo…
…unds, r=nrc

 This is a fix for #20300 though as a side-sweep it fixes a number of stack overflows because it integrates cycle detection into the conversion process. I didn't go through and retest everything.

The tricky part here is that in some cases we have to go find the information we need from the AST -- we can't use the converted form of the where-clauses because we often have to handle something like `T::Item` *while converting the where-clauses themselves*. Since this is also not a fixed-point process we can't just try and keep trying to find the best order. So instead I modified the `AstConv` interface to allow you to request the bounds for a type parameter; we'll then do a secondary scan of the where-clauses to figure out what we need. This may create a cycle in some cases, but we have code to catch that.

Another approach that is NOT taken by this PR would be to \"convert\" `T::Item` into a form that does not specify what trait it's using. This then kind of defers the problem of picking the trait till later. That might be a good idea, but it would make normalization and everything else much harder, so I'm holding off on that (and hoping to find a better way for handling things like `i32::T`).

This PR also removes \"most of\" the `bounds` struct from `TypeParameterDef`. Still a little ways to go before `ParamBounds` can be removed entirely -- it's used for supertraits, for example (though those really ought to be converted, I think, to a call to `get_type_parameter_bounds` on `Self` from within the trait definition).

cc @jroesch

Fixes #20300

bors added a commit that referenced this issue Feb 25, 2015

Auto merge of #22512 - nikomatsakis:issue-20300-where-clause-not-boun…
…ds, r=nikomatsakis

This is a fix for #20300 though as a side-sweep it fixes a number of stack overflows because it integrates cycle detection into the conversion process. I didn't go through and retest everything.

The tricky part here is that in some cases we have to go find the information we need from the AST -- we can't use the converted form of the where-clauses because we often have to handle something like `T::Item` *while converting the where-clauses themselves*. Since this is also not a fixed-point process we can't just try and keep trying to find the best order. So instead I modified the `AstConv` interface to allow you to request the bounds for a type parameter; we'll then do a secondary scan of the where-clauses to figure out what we need. This may create a cycle in some cases, but we have code to catch that.

Another approach that is NOT taken by this PR would be to "convert" `T::Item` into a form that does not specify what trait it's using. This then kind of defers the problem of picking the trait till later. That might be a good idea, but it would make normalization and everything else much harder, so I'm holding off on that (and hoping to find a better way for handling things like `i32::T`).

This PR also removes "most of" the `bounds` struct from `TypeParameterDef`. Still a little ways to go before `ParamBounds` can be removed entirely -- it's used for supertraits, for example (though those really ought to be converted, I think, to a call to `get_type_parameter_bounds` on `Self` from within the trait definition).

cc @jroesch 

Fixes #20300

@bors bors closed this in #22512 Feb 25, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment