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

Associated type lifetime bounds are underconstrained #22246

Closed
aturon opened this issue Feb 12, 2015 · 6 comments
Closed

Associated type lifetime bounds are underconstrained #22246

aturon opened this issue Feb 12, 2015 · 6 comments
Assignees
Labels
A-borrow-checker Area: The borrow checker A-type-system Area: Type system

Comments

@aturon
Copy link
Member

aturon commented Feb 12, 2015

The following code:

use std::ops::Deref;

pub trait ToOwned {
    type Owned: Borrow<Self>;
    fn to_owned(&self) -> Self::Owned;
}

pub enum Cow<'a, B: ?Sized + 'a> where B: ToOwned {
    Borrowed(&'a B),
    Owned(<B as ToOwned>::Owned)
}

impl<'a, B: ?Sized> Deref for Cow<'a, B> where B: ToOwned {
    type Target = B;

    fn deref(&self) -> &B {
        match *self {
            Cow::Borrowed(borrowed) => borrowed,
            Cow::Owned(ref owned) => owned.borrow()
        }
    }
}

pub trait Borrow<Borrowed: ?Sized> {
    fn borrow(&self) -> &Borrowed;
}

fails with:

<anon>:19:24: 19:33 error: the associated type `<B as ToOwned>::Owned` may not live long enough [E0311]
<anon>:19             Cow::Owned(ref owned) => owned.borrow()
                                 ^~~~~~~~~
<anon>:19:24: 19:33 help: consider adding an explicit lifetime bound for `<B as ToOwned>::Owned`
<anon>:19             Cow::Owned(ref owned) => owned.borrow()
                                 ^~~~~~~~~
<anon>:16:27: 21:6 note: the associated type `<B as ToOwned>::Owned` must be valid for the anonymous lifetime #1 defined on the block at 16:26...
<anon>:16     fn deref(&self) -> &B {
<anon>:17         match *self {
<anon>:18             Cow::Borrowed(borrowed) => borrowed,
<anon>:19             Cow::Owned(ref owned) => owned.borrow()
<anon>:20         }
<anon>:21     }
<anon>:19:24: 19:33 note: ...so that the reference type `&<B as ToOwned>::Owned` does not outlive the data it points at
<anon>:19             Cow::Owned(ref owned) => owned.borrow()

The same code not using associated types does not have this problem.

@aturon aturon added A-type-system Area: Type system A-borrow-checker Area: The borrow checker labels Feb 12, 2015
@aturon
Copy link
Member Author

aturon commented Feb 12, 2015

cc @japaric

@nikomatsakis
Copy link
Contributor

The problem arises from the "implied region bounds" rules. In the older formulation (or an equivalent one, like http://is.gd/L4uIcH), you wind up with the owned type O as a type parameter. Therefore, the type of self is &'a Cow<B,O>, from which we infer that B:'a and O:'a holds. This determination is based on rules that consider the variance of each type parameter (here, B and O). Unfortunately, when O is moved to an associated type, the type is just &'a Cow<B>, from which we derive that B:'a but not B::Owned : 'a.

I see a couple of ways to fix this:

  1. We could modify the WF rules to trawl the contents of the enum/structure, potentially. This would allow us to determine that &'a Cow<B> implies B::Owned : 'a because B::Owned is contained within one of the variants.
  2. We could modify the WF rules so that if you have a type parameter B with the bound B:Borrow, then we ensure that all associated types of Borrow must also outlive the surrounding region. Hence &'a Cow<B> would be imply B:'a and B::Owned : 'a.

The difference between 1 and 2 is that 2 doesn't require inspecting the contents of the struct, just the where-clauses declared on the struct. I have a mild preference for this -- I originally designed the rules to use variance for the same reason, basically that trawling through the contents feels like it exposes more of the guts than I would like, and raises other questions.

The downside of option 2 is that if there are associated types that don't represent data reachable from Self, then they will wind up with stricter lifetime requirements than necessary. Lacking variance information on associated types, we can't really make this determination. It's unclear how big a problem this would be in practice; conceivably we could eventually add some keyword (like in type X) to declare "input" types that don't represent data reachable from self.

Anyway, just some in-progress thoughts. I think mocking up option 2 wouldn't be all that hard.

@nikomatsakis
Copy link
Contributor

Somewhat simplified test case:

use std::ops::Deref;

pub trait ToOwned {
    type Owned: Borrow<Self>;
    fn to_owned(&self) -> Self::Owned;
}

pub trait Borrow<Borrowed> {
    fn borrow(&self) -> &Borrowed;
}

pub struct Foo<B:ToOwned> {
    owned: B::Owned
}

fn foo<B:ToOwned>(this: &Foo<B>) -> &B {
    this.owned.borrow()
}

fn main() { }

@nikomatsakis
Copy link
Contributor

I've implemented option 2, which I think is the right call. Basically the idea is that if you have a struct with where clauses, we will require that any associated types of traits that appear in those where clauses must outlive the struct itself (as they could potentially be embedded within the struct).

So if you have:

trait Foo { type T; ... }
struct Bar<U:Foo> { ... }

then if some instance of Bar<U> which appears in an location with lifetime 'a, U::T must outlive 'a. Similarly &'a Bar<U> is only legal if U::T : 'a (and, most importantly, a function that takes &'a Bar<U> can assume that U::T : 'a).

nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Feb 18, 2015
that it produces "outlives" relations for associated types. Add
several tests relating to rust-lang#22246.
bors added a commit that referenced this issue Feb 18, 2015
…soc-types, r=nikomatsakis

Take 2. This PR includes a bunch of refactoring that was part of an experimental branch implementing [implied bounds]. That particular idea isn't ready to go yet, but the refactoring proved useful for fixing #22246. The implied bounds branch also exposed #22110 so a simple fix for that is included here. I still think some more refactoring would be a good idea here -- in particular I think most of the code in wf.rs is kind of duplicating the logic in implicator and should go, but I decided to post this PR and call it a day before diving into that. I'll write a bit more details about the solutions I adopted in the various bugs. I patched the two issues I was concerned about, which was the handling of supertraits and HRTB (the latter turned out to be fine, so I added a comment explaining why.)

r? @pnkfelix (for now, anyway)
cc @aturon 

[implied bounds]: http://smallcultfollowing.com/babysteps/blog/2014/07/06/implied-bounds/
alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 19, 2015
…imes-of-assoc-types

Take 2. This PR includes a bunch of refactoring that was part of an experimental branch implementing [implied bounds]. That particular idea isn't ready to go yet, but the refactoring proved useful for fixing rust-lang#22246. The implied bounds branch also exposed rust-lang#22110 so a simple fix for that is included here. I still think some more refactoring would be a good idea here -- in particular I think most of the code in wf.rs is kind of duplicating the logic in implicator and should go, but I decided to post this PR and call it a day before diving into that. I'll write a bit more details about the solutions I adopted in the various bugs. I patched the two issues I was concerned about, which was the handling of supertraits and HRTB (the latter turned out to be fine, so I added a comment explaining why.)

r? @pnkfelix (for now, anyway)
cc @aturon

[implied bounds]: http://smallcultfollowing.com/babysteps/blog/2014/07/06/implied-bounds/
@edwardw
Copy link
Contributor

edwardw commented Feb 21, 2015

This can be closed since #22436 has fixed it.

@alexcrichton
Copy link
Member

Thanks @edwardw!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker A-type-system Area: Type system
Projects
None yet
Development

No branches or pull requests

4 participants