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

Lifetime bound error when using the indexing operator with an associated type containing a lifetime #32382

Closed
koute opened this issue Mar 20, 2016 · 12 comments
Labels
A-lifetimes Area: Lifetimes / regions A-type-system Area: Type system C-bug Category: This is a bug. fixed-by-NLL Bugs fixed, but only when NLL is enabled. P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@koute
Copy link
Member

koute commented Mar 20, 2016

Please consider the following code:

use std::marker::PhantomData;
use std::ops::Index;

pub trait Context: Clone {
    type Container: ?Sized;
    fn foobar_1( container: &Self::Container ) -> &str;
    fn foobar_2( container: &Self::Container ) -> &str;
    fn foobar_3( container: &Self::Container ) -> &str;
}

#[derive(Clone)]
struct Foobar<'a> {
    phantom: PhantomData<&'a ()>
}

impl<'a> Context for Foobar<'a> {
    type Container = [&'a str];

    fn foobar_1<'r>( container: &'r [&'a str] ) -> &'r str {
        // This compiles fine.
        container[0]
    }

    fn foobar_2<'r>( container: &'r Self::Container ) -> &'r str {
        // This also compiles fine.
        container.index( 0 )
    }

    fn foobar_3<'r>( container: &'r Self::Container ) -> &'r str {
        // error: `*container[..]` does not live long enough
        container[0]
    }
}

fn main() {}

which fails with the following error:

test.rs:31:9: 31:21 error: `*container[..]` does not live long enough
test.rs:31         container[0]
                   ^~~~~~~~~~~~
test.rs:29:66: 32:6 note: reference must be valid for the lifetime 'r as defined on the block at 29:65...
test.rs:29     fn foobar_3<'r>( container: &'r Self::Container ) -> &'r str {
test.rs:30         // error: `*container[..]` does not live long enough
test.rs:31         container[0]
test.rs:32     }
test.rs:29:66: 32:6 note: ...but borrowed value is only valid for the lifetime 'a as defined on the block at 29:65
test.rs:29     fn foobar_3<'r>( container: &'r Self::Container ) -> &'r str {
test.rs:30         // error: `*container[..]` does not live long enough
test.rs:31         container[0]
test.rs:32     }
error: aborting due to previous error

So if I use .index() and use an associated type it compiles fine. If I use [] and replace the associated type with the underlying type it also compiles fine. Logic would have it that if I use [] and I'll leave the associated type as-is it should also compile fine, but alas, I get an error, which doesn't really make any sense.

Rust version: rustc 1.9.0-nightly (b12b4e4 2016-03-17)

This is a regression; this code used to compile on at least 1.4, 1.5 and 1.6; it broke at 1.7.

@alexcrichton
Copy link
Member

cc @nikomatsakis

@alexcrichton
Copy link
Member

also @rust-lang/compiler

@alexcrichton alexcrichton added the regression-from-stable-to-stable Performance or correctness regression from one stable version to another. label Mar 21, 2016
@brson brson added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. I-nominated labels Jun 23, 2016
@pnkfelix pnkfelix self-assigned this Jun 23, 2016
@pnkfelix
Copy link
Member

I will look at this.

@pnkfelix
Copy link
Member

marking as P-medium nonetheless, since I am not convinced this needs to high priority...

@pnkfelix pnkfelix added P-medium Medium priority and removed I-nominated labels Jun 23, 2016
@brson brson added A-type-system Area: Type system A-borrow-checker Area: The borrow checker and removed A-borrow-checker Area: The borrow checker labels Sep 13, 2016
@nikomatsakis nikomatsakis added the A-lifetimes Area: Lifetimes / regions label Sep 13, 2016
@nikomatsakis
Copy link
Contributor

OK, so, I think this is related to the implied bounds that we compute. We must be somehow failing to add the implied 'a: 'r bound because it is obscured by the Self::Contained (which must not be normalized at the right time or something). I'll have to dig a bit in the code to see why this is the case though.

@nikomatsakis
Copy link
Contributor

Well, that doesn't quite explain why the manual call to index works, I guess. Maybe my hypothesis is wrong. I would guess though there's a problem where using the [] operator must be inducing some invariance perhaps?

@Mark-Simulacrum
Copy link
Member

@pnkfelix You are currently assigned to this, are you still investigating?

@eddyb
Copy link
Member

eddyb commented May 13, 2017

cc @nikomatsakis as well

@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 24, 2017
@JelteF
Copy link
Contributor

JelteF commented Apr 2, 2018

I'm running into this issue as well when trying to convert get().unwrap() to an index operation on this line:
https://github.com/JelteF/derive_more/blob/2e37e7f8181ba333df9503088222772b77c83d91/src/from.rs#L103

It indeed works fine when manually calling index().

@pnkfelix pnkfelix added the fixed-by-NLL Bugs fixed, but only when NLL is enabled. label Oct 4, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Oct 4, 2018

I just tried this out with the 2018 edition and it works there. Or on the nightly compiler, one can add #![feature(nll)] to get the fix.

We should add a regression test before closing though.

@pnkfelix pnkfelix added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Oct 4, 2018
pnkfelix added a commit to pnkfelix/rust that referenced this issue Oct 4, 2018
pietroalbini added a commit to pietroalbini/rust that referenced this issue Oct 5, 2018
…pe-with-lifetime, r=nikomatsakis

Regression test for rust-lang#32382.
bors added a commit that referenced this issue Oct 6, 2018
Rollup of 11 pull requests

Successful merges:

 - #54078 (Expand the documentation for the `std::sync` module)
 - #54717 (Cleanup rustc/ty part 1)
 - #54781 (Add examples to `TyKind::FnDef` and `TyKind::FnPtr` docs)
 - #54787 (Only warn about unused `mut` in user-written code)
 - #54804 (add suggestion for inverted function parameters)
 - #54812 (Regression test for #32382.)
 - #54833 (make `Parser::parse_foreign_item()` return a foreign item or error)
 - #54834 (rustdoc: overflow:auto doesn't work nicely on small screens)
 - #54838 (Fix typo in src/libsyntax/parse/parser.rs)
 - #54851 (Fix a regression in 1.30 by reverting #53564)
 - #54853 (Remove unneccessary error from test, revealing NLL error.)

Failed merges:

r? @ghost
@pnkfelix
Copy link
Member

pnkfelix commented Nov 9, 2018

#54812 added the test.

@pnkfelix pnkfelix removed the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Nov 9, 2018
@pnkfelix pnkfelix removed their assignment Nov 9, 2018
@pnkfelix
Copy link
Member

NLL (migrate mode) is enabled in all editions as of PR #59114. Verified that test case compiled in Nightly 2015 edition; closing as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lifetimes Area: Lifetimes / regions A-type-system Area: Type system C-bug Category: This is a bug. fixed-by-NLL Bugs fixed, but only when NLL is enabled. P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. 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

8 participants