Methods in impls allowed more restrictive lifetime bounds than in the trait.

[eddyb]

trait Foo {
    fn foo<T>(self) -> u64;
}
impl Foo for () {
    fn foo<T: 'static>(self) -> u64 {
        //    ^^^^^^^ this should cause an error, the bound is missing from the
        // method definition in the trait, and calls are checked against that.
        std::intrinsics::TypeId::of::<&'static T>().hash()
    }
}
fn main() {
    println!("{}", ().foo::<&()>());
}
// error: internal compiler error: non-static region found when hashing a type

cc @nikomatsakis

[JustAPerson]

tl;dr: this bug still reproduces but no longer causes an ICE
Updated for current Rust (on playpen as of today, 29 March 2015):

Hopefully I updated @eddyb's code properly. This compiles and executes fine.

#![feature(core, hash)]
use std::hash::{hash, SipHasher};

trait Foo {
    fn foo<T>(self) -> u64;
}
impl Foo for () {
    fn foo<T: 'static>(self) -> u64 {
        //    ^^^^^^^ this should cause an error, the bound is missing from the
        // method definition in the trait, and calls are checked against that.
        hash::<_, SipHasher>(unsafe{ &std::intrinsics::type_id::<&'static T>() })
    }
}
fn main() {
    println!("{}", ().foo::<&()>());
}

16958441930439044241

[ghost]

@JustAPerson Thanks! Marking as E-needstest.

[nikomatsakis]

triage: P-backcompat-lang (1.0)

I'm removing the E-needstest label because there is still a bug here -- actually ICEing is better than accepting an illegal program. That said, this may be a dup of #22779 -- although I suspect it is somewhat different. I'm going to triage it as 1.0 and try to fix it.

[arielb1]

Still broken after the fix of #22779 (http://is.gd/rZ0MgK)

[nikomatsakis]

So I know what the problem is here, but fixing it is a bit annoying due to the way our contexts are structured (we're not processing the fulfillment context's "region obligations" list). I've been wanting to do some restructuring here that would help, I'll look into that. (The very existence of the "region obligations" list is actually a demonstration of the problem.)

[jroesch]

cc me

[stevenblenkinsop]

Ran into an example that creates unsoundness and doesn't use unsafe. I think it's ultimately the same issue:

https://play.rust-lang.org/?gist=25b7461083e5ee9b3415&version=nightly

[vacavaca]

Is it planned to be fixed in stable?

PS just another example with use after free in safe code from dup issue:
https://play.rust-lang.org/?gist=e95ac4f83f16b062ee8b7c53ed52d5e2&version=stable&backtrace=0

[arielb1]

@nikomatsakis said he will look at this when he comes back from vacation.

[brson]

@rust-lang/compiler Can you reaffirm this is P-high and somebody is on it? Maybe this can be mentored?

[brson]

@sanxiyn Says they may be able to do this with some mentoring!

[alexcrichton]

I've just closed #35730 as a dupe of this which has another example of a segfault related to this we believe.

Renominating as this came up again, unfortunately...

[nikomatsakis]

triage: P-high

[nikomatsakis]

OK, so @jroesch and I implemented an initial fix for this (finally). I did a crater run with the following results:

• 5819 crates tested: 4167 working / 1605 broken / 12 regressed / 3 fixed / 32 unknown.

https://gist.github.com/nikomatsakis/cba00297c8659f71740b8e45dad07137

That's not too bad. I want to dig into those results some more, but I'm hoping we can get away w/o a warning period on this one -- it would be a real pain to support!

[nikomatsakis]

(It may also be worth investing a bit of energy in the error messages, which do not currently hint at the fact that the impl is imposing more requirements than the trait.)

[arielb1]

This appears to be broken by the fix:

pub trait Leak<T : ?Sized> {
    fn leak<'a>(self) -> &'a T where T: 'a;
}

impl<T> Leak<[T]> for Vec<T> {
    fn leak<'a>(mut self) -> &'a [T] where [T]: 'a {
        let r: *mut [T] = &mut self[..];
        std::mem::forget(self);
        unsafe { &mut *r }
    }
}
fn main() {
    println!("Hello, world!");
}

error[E0309]: the parameter type `T` may not live long enough
  --> src/lib.rs:45:5
   |
45 |     fn leak<'a>(mut self) -> &'a [T] where [T]: 'a {
   |     ^
   |
   = help: consider adding an explicit lifetime bound `T: 'a`...
note: ...so that the type `[T]` will meet its required lifetime bounds
  --> src/lib.rs:45:5
   |
45 |     fn leak<'a>(mut self) -> &'a [T] where [T]: 'a {
   |     ^

[nikomatsakis]

@arielb1 I feel that is an orthogonal issue. It is true that this code no longer compiles, and that it would be nice and safe if it did compile, but it is not true that it should compile with the current state of rustc's reasoning. That is, I think we should indeed deduce that if [T]: 'a than (e.g.) T: 'a must hold and so forth, but we fail to do so in other contexts too, and I'm not sure why we should go out of our way to work around it in this context vs solving that more generally.

Now, one could argue -- and I might agree -- that we should try to fix that oversight at the same time as applying this path. I think it'd be fairly straightforward to do, really, though it probably wants an RFC. I think @alexcrichton ran into this limitation in futures in some other setting.

[pnkfelix]

@nikomatsakis do you think you two are still making fwd progress here, or should we delegate?

[nikomatsakis]

Got distracted but I now have a local branch that I think both rejects the original case but accepts this case. I have to press on a bit more. The key change though is to expand on things like &'a T: 'b and use that to conclude that, indeed, 'a: 'b and T: 'b. (I did this in a fairly conservative fashion for now.)

[nikomatsakis]

Argh. This is frustrating. So I put in some work into the error messages, which now look like this:

lunch-box. rustc --stage1 region-unrelated.rs
error[E0276]: impl has stricter requirements than trait
  --> region-unrelated.rs:21:5
   |
16 |     fn foo() where T: 'a;
   |     --------------------- definition of `foo` from trait
...
21 |     fn foo() where V: 'a { }
   |     ^^^^^^^^^^^^^^^^^^^^^^^^ impl has extra requirement `V: 'a`

error: aborting due to previous error

I am trying to turn these into future-compatibility lints, but the current infrastructure (e.g., add_lint) is lacking (it only lets you give a string). I have to see if I can improve it easily, I guess (there is struct_span_lint, I suppose...).

[nikomatsakis]

Update: I now have the lints mostly working, though I'm still refactoring the changes I made to that end.

[brson]

Pr enqueud #37167