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

Regression in new inference scheme #16870

Closed
bkoropoff opened this issue Aug 30, 2014 · 5 comments
Closed

Regression in new inference scheme #16870

bkoropoff opened this issue Aug 30, 2014 · 5 comments

Comments

@bkoropoff
Copy link
Contributor

I was having a great deal of difficulty getting my library (https://github.com/bkoropoff/rust-jlens) to build on newer versions of rustc, even after updating it to deal with generalized lifetime bounds. Reverting to a rustc prior to the new inference scheme resolved the issue. I came up with the following reduced test case:

struct Chain<'a:'b,'b> {
    x: &'a int,
    y: Option<&'b Chain<'a,'b>>
}

impl<'a,'b> Chain<'a,'b> {
    fn leaf(x: &'a int) -> Chain<'a,'b> { Chain { x: x, y: None } }
}

fn callme<'a,'b> (y: Chain<'a,'b>, x: &'a int, f: <'c>|Chain<'a,'c>|) {
    f(Chain { x: x, y: Some(&y) })
}

fn main() {
    let x = 5i;
    let y = Chain::leaf(&x);
    callme(y, &x, |_| {});
}

This seems to build fine prior to the new inference scheme landing and gives the following error after:

test.rs:12:30: 12:31 error: `y` does not live long enough
test.rs:12     f(Chain { x: x, y: Some(&y) })
                                        ^
test.rs:11:71: 13:2 note: reference must be valid for the lifetime 'b as defined on the block at 11:70...
test.rs:11 fn callme<'a,'b> (y: Chain<'a,'b>, x: &'a int, f: <'c>|Chain<'a,'c>|) {
test.rs:12     f(Chain { x: x, y: Some(&y) })
test.rs:13 }
test.rs:11:71: 13:2 note: ...but borrowed value is only valid for the block at 11:70
test.rs:11 fn callme<'a,'b> (y: Chain<'a,'b>, x: &'a int, f: <'c>|Chain<'a,'c>|) {
test.rs:12     f(Chain { x: x, y: Some(&y) })
test.rs:13 }

cc @nikomatsakis

@nikomatsakis
Copy link
Contributor

Without having investigated deeply, I think the problem is #3598. But I will investigate now to be sure.

@nikomatsakis
Copy link
Contributor

Preliminary investigation suggests that yes, the issue is #3598. In this case, the old type checker was erroneously accepting the program. The reason is that chain winds up being invariant w/r/t 'b because Option is invariant with respect to its type argument. I've often worked around this locally by not using Option and making a specialized type. Here is an example from the Rust code base:

https://github.com/rust-lang/rust/blob/master/src/librustc/middle/resolve_lifetime.rs#L58

That said, I am hoping to open an RFC about support covariance soon. I want to do some local experiments, but I think the codebase is now at a point where it should be easy to introduce.

@nikomatsakis
Copy link
Contributor

In my branch which fixes #3598, this test compiles.

@bkoropoff
Copy link
Contributor Author

@nikomatsakis Is your branch for issue #3598 published anywhere yet? It would be interesting to try building my library with it. In any case, I'll try your suggestion of using a specialized enum. Thanks for taking a look at this so quickly!

@bkoropoff
Copy link
Contributor Author

Using a specialized enum worked. Since this is not actually a regression but the result of an apparent soundness bug being fixed, closing issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants