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

Mismatched assumptions for interaction between associated type and lifetime in trait #24622

Closed
huonw opened this issue Apr 20, 2015 · 8 comments
Assignees
Labels
A-associated-items Area: Associated items (types, constants & functions) A-lifetimes Area: Lifetimes / regions A-trait-system Area: Trait system P-high High priority T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@huonw
Copy link
Member

huonw commented Apr 20, 2015

pub trait Foo<'a> {
    type Bar: std::fmt::Debug;
    fn baz(&'a self) -> Self::Bar;
}

pub struct X {
    name : String,
}

impl<'a> Foo<'a> for X {
    type Bar = &'a String;
    fn baz(&'a self) -> &'a String {
        &self.name
    }
}

pub fn foo<T: for<'a> Foo<'a>>(x: T) {
    let y = x.baz();
    drop(x);
    // for T = X, this is a &String pointing to dropped memory
    println!("{:?}", y); 
}

pub fn main() {
    foo(X { name: "foo".to_string() });
}

This segfaults.

The impl is allowing &'a String because 'a is in scope, but the use in foo assumes that Bar is independent of 'a.

(Noticed in http://stackoverflow.com/q/29745134/1256624 .)

@huonw huonw added I-wrong A-lifetimes Area: Lifetimes / regions A-trait-system Area: Trait system A-associated-items Area: Associated items (types, constants & functions) labels Apr 20, 2015
@huonw
Copy link
Member Author

huonw commented Apr 20, 2015

triage: P-high

cc @nikomatsakis

@rust-highfive rust-highfive added the P-medium Medium priority label Apr 20, 2015
@bluss
Copy link
Member

bluss commented Apr 20, 2015

petgraph uses this kind of trait to provide generic "neighbor vertex" iterators.

@arielb1
Copy link
Contributor

arielb1 commented Apr 21, 2015

I think a cleaner example is:

pub trait Foo<'a> {
    type Bar;
}

impl<'a, T> Foo<'a> for T { type Bar = &'a T; }

fn denormalise<'a, T>(t: &'a T) -> <T as Foo<'a>>::Bar {
    t
}

pub fn free_and_use<T: for<'a> Foo<'a>,
                    F: for<'a> FnOnce(<T as Foo<'a>>::Bar)>(x: T, f: F) {
    let y;
    'body: loop { // lifetime annotations added for clarity
        's: loop { y = denormalise(&x); break }
        drop(x);
        return f(y);
    }
}

pub fn main() {
    free_and_use("foo".to_string(), |s| println!("{}", s));
}

It is clear here that the declaration and implementation of Foo is not doing anything wrong, but free_and_use wrongly assumes that <T as Foo<'s>>::Bar : 'body, which does not make a lot of sense.

@nikomatsakis
Copy link
Contributor

Yes, I see the problem. It lies in some of the logic around type parameter outlives rules -- we assume type parameters outlive the current fn, and we assume the same of projections, which is clearly wrong. I've been wanting to revisit these rules about projections and how long they live (#23442) though not for soundness reasons.

@nikomatsakis
Copy link
Contributor

triage: P-high

@rust-highfive rust-highfive added P-high High priority and removed P-medium Medium priority labels Apr 24, 2015
@nikomatsakis nikomatsakis self-assigned this Apr 24, 2015
@nikomatsakis
Copy link
Contributor

Just FYI -- I've been working steadily on this. I'm going to try and writeup my thoughts "soon" (famous last words).

@brson brson added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jun 5, 2015
nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Jun 9, 2015
object types impose minimal constraints on their arguments.  Also modify
regionck to consider `<P0 as TraitRef<P1..Pn>>::Foo: 'r` to hold if `Pi:
'r` (for all `i`). This is a fallback, because in some cases we can
impose looser requirements. Currently the inference is sound but not
precise. Fixes rust-lang#24622.

Adapted cherry-pick of 45f93290969f19d8217a2e4dff7b12dc81957eb6.
nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Jul 6, 2015
object types impose minimal constraints on their arguments.  Also modify
regionck to consider `<P0 as TraitRef<P1..Pn>>::Foo: 'r` to hold if `Pi:
'r` (for all `i`). This is a fallback, because in some cases we can
impose looser requirements. Currently the inference is sound but not
precise. Fixes rust-lang#24622.

Adapted cherry-pick of 45f93290969f19d8217a2e4dff7b12dc81957eb6.
nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Jul 14, 2015
object types impose minimal constraints on their arguments.  Also modify
regionck to consider `<P0 as TraitRef<P1..Pn>>::Foo: 'r` to hold if `Pi:
'r` (for all `i`). This is a fallback, because in some cases we can
impose looser requirements. Currently the inference is sound but not
precise. Fixes rust-lang#24622.

Adapted cherry-pick of 45f93290969f19d8217a2e4dff7b12dc81957eb6.
nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Aug 5, 2015
object types impose minimal constraints on their arguments.  Also modify
regionck to consider `<P0 as TraitRef<P1..Pn>>::Foo: 'r` to hold if `Pi:
'r` (for all `i`). This is a fallback, because in some cases we can
impose looser requirements. Currently the inference is sound but not
precise. Fixes rust-lang#24622.

Adapted cherry-pick of 45f93290969f19d8217a2e4dff7b12dc81957eb6.
@nikomatsakis
Copy link
Contributor

This original example is now giving me an error on stable: http://is.gd/bidora

The question is, did we add a test?

@nikomatsakis
Copy link
Contributor

The test associated-types-outlives.rs claims to be a regression test for this issue. Closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-associated-items Area: Associated items (types, constants & functions) A-lifetimes Area: Lifetimes / regions A-trait-system Area: Trait system P-high High priority T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants