-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
impl-trait return type is bounded by all input type parameters, even when unnecessary #42940
Comments
From some experimentation, it seems to be because Thus in the original code In light of that, here's a smaller repro: #![feature(conservative_impl_trait)]
trait Tr { }
struct S;
impl Tr for S { }
fn foo<T>(_t: T) -> impl Tr {
S
}
struct S2;
fn main() {
let _bar = {
let s2 = S2;
foo(&s2)
};
} which complains that |
This is an intentional restriction. See RFC 1951 for the reasoning. |
Sure. So can there be a way that I can convince the compiler that The reasoning in the RFC is that eventually there will be syntax that provides control over which lifetime are and are not part of the returned existential ("Assumption 1"). But for the OP example where there already is an explicit lifetime and bound so it could be made to work today. |
Oh, I understand what you're saying now. Yes, if the return type contains an explicit lifetime bound, the compiler should be able to understand that the returned type outlives that lifetime bound. Currently, it cannot do that if there are type parameters involved. This should be fixed. Thanks for the report! |
This is fixed by The fact that existential types intentionally don't have the "generic lifetime inheriting" behavior that impl trait has is documented here. Updated 2023-10-09 for current syntax:
|
No, this is still a bug. The fact that e.g. this doesn't compile today and shouldn't: trait X {}
impl<T> X for T {}
fn foo<'a, T>(x: &'a u8, t: T) -> impl X + 'a {
(x, t)
} You have to explicitly add |
@cramertj Okay, so this is an issue with a lifetime inference, right? (Not actual bounds checking.) Would you mind writing up mentoring instructions so someone can tackle this? (Maybe even me.) |
@nikomatsakis Can we do something about this soon you think? :-) Seems kind of urgent to me, though perhaps this affects me more than most users. |
Just to check, this is the same issue right?
gives
(I also think the suggestion is misleading - won't adding a |
@aidanhs Yes to both (well, the answer to the second question is a bit more complicated, but "it won't work" is correct ;) ). |
Also encountered this issue with the following (minimised) code. Also found a workaround by specifying a dummy trait and moving the (static) lifetime there. Errorstrait Parser2 {
type Input;
type PartialState;
}
struct Test<I>(::std::marker::PhantomData<fn(I)>);
impl<I> Parser2 for Test<I> {
type Input = I;
type PartialState = ();
}
fn line<'a, I>() -> impl Parser2<Input = I, PartialState = impl Send + 'static> {
Test(::std::marker::PhantomData)
}
fn status<'a, I>() -> impl Parser2<Input = I, PartialState = impl Send + 'static> {
line()
}
fn main() {
} Workstrait Parser2 {
type Input;
type PartialState;
}
struct Test<I>(::std::marker::PhantomData<fn(I)>);
impl<I> Parser2 for Test<I> {
type Input = I;
type PartialState = ();
}
trait Static: Send + 'static {}
impl<T> Static for T where T: Send + 'static {}
fn line<'a, I>() -> impl Parser2<Input = I, PartialState = impl Static> {
Test(::std::marker::PhantomData)
}
fn status<'a, I>() -> impl Parser2<Input = I, PartialState = impl Static> {
line()
} |
@nikomatsakis Would be curious to get your thoughts on this along with the other not-too-dissimilar covariant lifetimes issue. |
I've often found it pretty useful to model the modelBasically an fn post<'a, B>(&'a self, _body: &B) -> impl Future + 'a { .. } can be modeled as if there were a "one-off" trait with a single impl: trait Post<'a, B> {
// ^^^^ these are the "captured" parameters, per RFC 1951
type Output: Future + 'a;
}
impl<'a, B> Post for () {
default type Output = /* the hidden type that compiler infers */;
} and the function were then declared like so: fn post<'a, B>(&'a self, _body: &B) -> <() as Post>::Output { .. } Why does this matter?Look at the next function, fn login<'a>(client: &'a Client, username: &str) -> impl Future + 'a { .. } this function winds up inferring that the hidden type The bug report here seems correct: we should be able to prove that How does this work for associated types?The compiler handles similar problems already for associated types. The rules were outlined in RFC 1214, along with some of the challenges. I think we should probably be able to apply the compiler's existing heuristics to this problem, but it might take a bit of work. The relevant function is here: rust/src/librustc/infer/outlives/obligations.rs Lines 357 to 362 in daa53a5
In particular, I believe this heuristic is the one that will help: rust/src/librustc/infer/outlives/obligations.rs Lines 445 to 459 in daa53a5
|
this stackoverflow question seems related + a funny error message from the compiler about the lifetime, that could be changed?! |
I also encountered this issue recently and would love to see it fixed, as it makes working with async harder. |
tagging with wg-async with hopes that it will be enqueued as a polish issue for a volunteer from that group to work on. @rustbot label: +wg-async |
This removes the need of `C: 'static` bounds. cf. <rust-lang/rust#42940>
The important thing to understand about this issue is that it doesn't have anything to do with the lifetime bounds. The lifetime bounds work exactly as you would expect that they would. Consider this minimized version of the issue: fn capture<'o, T>(_t: T) -> impl Send + 'o {}
fn outlives<'o, T: 'o>(_t: T) {}
fn test<'o>(x: ()) -> impl Send + 'o {
outlives::<'o, _>(capture::<'o, &'_ ()>(&x)); // OK.
capture::<'o, &'_ ()>(&x)
// ^^
// ERROR: Borrowed value does not live long enough.
} The line labeled "OK" compiles fine. This shows that the opaque type returned by However, Rust won't let a value of this type escape up the stack (without type erasure) because there's no way to give the captured lifetime a name that's valid outside of the function. A full analysis of this subtle issue is available here. |
Consider alias bounds when computing liveness in NLL # Background Right now, liveness analysis in NLL is a bit simplistic. It simply walks through all of the regions of a type and marks them as being live at points. This is problematic in the case of aliases, since it requires that we mark **all** of the regions in their args[^1] as live, leading to bugs like rust-lang#42940. In reality, we may be able to deduce that fewer regions are allowed to be present in the projected type (or "hidden type" for opaques) via item bounds or where clauses, and therefore ideally, we should be able to soundly require fewer regions to be live in the alias. For example: ```rust trait Captures<'a> {} impl<T> Captures<'_> for T {} fn capture<'o>(_: &'o mut ()) -> impl Sized + Captures<'o> + 'static {} fn test_two_mut(mut x: ()) { let _f1 = capture(&mut x); let _f2 = capture(&mut x); //~^ ERROR cannot borrow `x` as mutable more than once at a time } ``` In the example above, we should be able to deduce from the `'static` bound on `capture`'s opaque that even though `'o` is a captured region, it *can never* show up in the opaque's hidden type, and can soundly be ignored for liveness purposes. # The Fix We apply a simple version of RFC 1214's `OutlivesProjectionEnv` and `OutlivesProjectionTraitDef` rules to NLL's `make_all_regions_live` computation. Specifically, when we encounter an alias type, we: 1. Look for a unique outlives bound in the param-env or item bounds for that alias. If there is more than one unique region, bail, unless any of the outlives bound's regions is `'static`, and in that case, prefer `'static`. If we find such a unique region, we can mark that outlives region as live and skip walking through the args of the opaque. 2. Otherwise, walk through the alias's args recursively, as we do today. ## Additionally: Opaque Hidden Types A similar bug comes about when walking through the hidden type of an opaque to validate it doesn't capture lifetimes that aren't part of that opaque. For example: ```rust trait Captures<'a> {} impl<T> Captures<'_> for T {} fn a() -> impl Sized + 'static { b(&vec![]) } fn b<'o>(_: &'o Vec<i32>) -> impl Sized + Captures<'o> + 'static {} ``` The hidden type of `a::{opaque}` is inferred to be `b::<'?1>::{opaque}`, where `'?1` is a region local to the body of `a`. However, the `'static` bound on `b`'s opaque should allow us to deduce that the hidden type will never mention that region `'?1`, and we can ignore it (and replace it with `ReErased` in `b`'s opaque's hidden type). Similarly to liveness, we don't currently do anything other than recurse through aliases for their lifetime components. This PR ends up using the same region visitor as liveness for opaque type region captures. ## Limitations This approach has some limitations. Specifically, since liveness doesn't use the same type-test logic as outlives bounds do, we can't really try several options when we're faced with a choice. If we encounter two unique outlives regions in the param-env or bounds, we simply fall back to walking the opaque via its args. I expect this to be mostly mitigated by the special treatment of `'static`, and can be fixed in a forwards-compatible by a more sophisticated analysis in the future. ## Read more Context: rust-lang#42940 (comment) More context: rust-lang#115822 (comment) Fixes rust-lang#42940 [^1]: except for bivariant region args in opaques, which will become less relevant when we move onto edition 2024 capture semantics for opaques.
How can I tell Rust that no, the return type does not, in fact, capture any input lifetimes, and is entirely static? |
|
Sadly that doesn't work when I try it, it says the feature is unstable. |
If it was possible to use TAIT in stable this issue wouldn't still be open. |
video.webmWild that removing mut gets rid of the error Maybe someone can look into this, I'm relatively new to rust and I don't think Edit: on second thought maybe the pointer cast is causing it, not entirely sure Either way, it should work as all lifetimes are bounded to the impl trait, yet it doesn't |
…his-time-sound, r=aliemjay Consider alias bounds when computing liveness in NLL (but this time sound hopefully) This is a revival of rust-lang#116040, except removing the changes to opaque lifetime captures check to make sure that we're not triggering any unsoundness due to the lack of general existential regions and the currently-existing `ReErased` hack we use instead. r? `@aliemjay` -- I appreciate you pointing out the unsoundenss in the previous iteration of this PR, and I'd like to hear that you're happy with this iteration of this PR before this goes back into FCP :> Fixes rust-lang#116794 as well --- (mostly copied from rust-lang#116040 and reworked slightly) # Background Right now, liveness analysis in NLL is a bit simplistic. It simply walks through all of the regions of a type and marks them as being live at points. This is problematic in the case of aliases, since it requires that we mark **all** of the regions in their args[^1] as live, leading to bugs like rust-lang#42940. In reality, we may be able to deduce that fewer regions are allowed to be present in the projected type (or "hidden type" for opaques) via item bounds or where clauses, and therefore ideally, we should be able to soundly require fewer regions to be live in the alias. For example: ```rust trait Captures<'a> {} impl<T> Captures<'_> for T {} fn capture<'o>(_: &'o mut ()) -> impl Sized + Captures<'o> + 'static {} fn test_two_mut(mut x: ()) { let _f1 = capture(&mut x); let _f2 = capture(&mut x); //~^ ERROR cannot borrow `x` as mutable more than once at a time } ``` In the example above, we should be able to deduce from the `'static` bound on `capture`'s opaque that even though `'o` is a captured region, it *can never* show up in the opaque's hidden type, and can soundly be ignored for liveness purposes. # The Fix We apply a simple version of RFC 1214's `OutlivesProjectionEnv` and `OutlivesProjectionTraitDef` rules to NLL's `make_all_regions_live` computation. Specifically, when we encounter an alias type, we: 1. Look for a unique outlives bound in the param-env or item bounds for that alias. If there is more than one unique region, bail, unless any of the outlives bound's regions is `'static`, and in that case, prefer `'static`. If we find such a unique region, we can mark that outlives region as live and skip walking through the args of the opaque. 2. Otherwise, walk through the alias's args recursively, as we do today. ## Limitation: Multiple choices This approach has some limitations. Firstly, since liveness doesn't use the same type-test logic as outlives bounds do, we can't really try several options when we're faced with a choice. If we encounter two unique outlives regions in the param-env or bounds, we simply fall back to walking the opaque via its args. I expect this to be mostly mitigated by the special treatment of `'static`, and can be fixed in a forwards-compatible by a more sophisticated analysis in the future. ## Limitation: Opaque hidden types Secondly, we do not employ any of these rules when considering whether the regions captured by a hidden type are valid. That causes this code (cc rust-lang#42940) to fail: ```rust trait Captures<'a> {} impl<T> Captures<'_> for T {} fn a() -> impl Sized + 'static { b(&vec![]) } fn b<'o>(_: &'o Vec<i32>) -> impl Sized + Captures<'o> + 'static {} ``` We need to have existential regions to avoid [unsoundness](rust-lang#116040 (comment)) when an opaque captures a region which is not represented in its own substs but which outlives a region that does. ## Read more Context: rust-lang#115822 (comment) (for the liveness case) More context: rust-lang#42940 (comment) (for the opaque capture case, which this does not fix) [^1]: except for bivariant region args in opaques, which will become less relevant when we move onto edition 2024 capture semantics for opaques.
…sound, r=aliemjay Consider alias bounds when computing liveness in NLL (but this time sound hopefully) This is a revival of #116040, except removing the changes to opaque lifetime captures check to make sure that we're not triggering any unsoundness due to the lack of general existential regions and the currently-existing `ReErased` hack we use instead. r? `@aliemjay` -- I appreciate you pointing out the unsoundenss in the previous iteration of this PR, and I'd like to hear that you're happy with this iteration of this PR before this goes back into FCP :> Fixes #116794 as well --- (mostly copied from #116040 and reworked slightly) # Background Right now, liveness analysis in NLL is a bit simplistic. It simply walks through all of the regions of a type and marks them as being live at points. This is problematic in the case of aliases, since it requires that we mark **all** of the regions in their args[^1] as live, leading to bugs like #42940. In reality, we may be able to deduce that fewer regions are allowed to be present in the projected type (or "hidden type" for opaques) via item bounds or where clauses, and therefore ideally, we should be able to soundly require fewer regions to be live in the alias. For example: ```rust trait Captures<'a> {} impl<T> Captures<'_> for T {} fn capture<'o>(_: &'o mut ()) -> impl Sized + Captures<'o> + 'static {} fn test_two_mut(mut x: ()) { let _f1 = capture(&mut x); let _f2 = capture(&mut x); //~^ ERROR cannot borrow `x` as mutable more than once at a time } ``` In the example above, we should be able to deduce from the `'static` bound on `capture`'s opaque that even though `'o` is a captured region, it *can never* show up in the opaque's hidden type, and can soundly be ignored for liveness purposes. # The Fix We apply a simple version of RFC 1214's `OutlivesProjectionEnv` and `OutlivesProjectionTraitDef` rules to NLL's `make_all_regions_live` computation. Specifically, when we encounter an alias type, we: 1. Look for a unique outlives bound in the param-env or item bounds for that alias. If there is more than one unique region, bail, unless any of the outlives bound's regions is `'static`, and in that case, prefer `'static`. If we find such a unique region, we can mark that outlives region as live and skip walking through the args of the opaque. 2. Otherwise, walk through the alias's args recursively, as we do today. ## Limitation: Multiple choices This approach has some limitations. Firstly, since liveness doesn't use the same type-test logic as outlives bounds do, we can't really try several options when we're faced with a choice. If we encounter two unique outlives regions in the param-env or bounds, we simply fall back to walking the opaque via its args. I expect this to be mostly mitigated by the special treatment of `'static`, and can be fixed in a forwards-compatible by a more sophisticated analysis in the future. ## Limitation: Opaque hidden types Secondly, we do not employ any of these rules when considering whether the regions captured by a hidden type are valid. That causes this code (cc #42940) to fail: ```rust trait Captures<'a> {} impl<T> Captures<'_> for T {} fn a() -> impl Sized + 'static { b(&vec![]) } fn b<'o>(_: &'o Vec<i32>) -> impl Sized + Captures<'o> + 'static {} ``` We need to have existential regions to avoid [unsoundness](rust-lang/rust#116040 (comment)) when an opaque captures a region which is not represented in its own substs but which outlives a region that does. ## Read more Context: rust-lang/rust#115822 (comment) (for the liveness case) More context: rust-lang/rust#42940 (comment) (for the opaque capture case, which this does not fix) [^1]: except for bivariant region args in opaques, which will become less relevant when we move onto edition 2024 capture semantics for opaques.
…sound, r=aliemjay Consider alias bounds when computing liveness in NLL (but this time sound hopefully) This is a revival of #116040, except removing the changes to opaque lifetime captures check to make sure that we're not triggering any unsoundness due to the lack of general existential regions and the currently-existing `ReErased` hack we use instead. r? `@aliemjay` -- I appreciate you pointing out the unsoundenss in the previous iteration of this PR, and I'd like to hear that you're happy with this iteration of this PR before this goes back into FCP :> Fixes #116794 as well --- (mostly copied from #116040 and reworked slightly) # Background Right now, liveness analysis in NLL is a bit simplistic. It simply walks through all of the regions of a type and marks them as being live at points. This is problematic in the case of aliases, since it requires that we mark **all** of the regions in their args[^1] as live, leading to bugs like #42940. In reality, we may be able to deduce that fewer regions are allowed to be present in the projected type (or "hidden type" for opaques) via item bounds or where clauses, and therefore ideally, we should be able to soundly require fewer regions to be live in the alias. For example: ```rust trait Captures<'a> {} impl<T> Captures<'_> for T {} fn capture<'o>(_: &'o mut ()) -> impl Sized + Captures<'o> + 'static {} fn test_two_mut(mut x: ()) { let _f1 = capture(&mut x); let _f2 = capture(&mut x); //~^ ERROR cannot borrow `x` as mutable more than once at a time } ``` In the example above, we should be able to deduce from the `'static` bound on `capture`'s opaque that even though `'o` is a captured region, it *can never* show up in the opaque's hidden type, and can soundly be ignored for liveness purposes. # The Fix We apply a simple version of RFC 1214's `OutlivesProjectionEnv` and `OutlivesProjectionTraitDef` rules to NLL's `make_all_regions_live` computation. Specifically, when we encounter an alias type, we: 1. Look for a unique outlives bound in the param-env or item bounds for that alias. If there is more than one unique region, bail, unless any of the outlives bound's regions is `'static`, and in that case, prefer `'static`. If we find such a unique region, we can mark that outlives region as live and skip walking through the args of the opaque. 2. Otherwise, walk through the alias's args recursively, as we do today. ## Limitation: Multiple choices This approach has some limitations. Firstly, since liveness doesn't use the same type-test logic as outlives bounds do, we can't really try several options when we're faced with a choice. If we encounter two unique outlives regions in the param-env or bounds, we simply fall back to walking the opaque via its args. I expect this to be mostly mitigated by the special treatment of `'static`, and can be fixed in a forwards-compatible by a more sophisticated analysis in the future. ## Limitation: Opaque hidden types Secondly, we do not employ any of these rules when considering whether the regions captured by a hidden type are valid. That causes this code (cc #42940) to fail: ```rust trait Captures<'a> {} impl<T> Captures<'_> for T {} fn a() -> impl Sized + 'static { b(&vec![]) } fn b<'o>(_: &'o Vec<i32>) -> impl Sized + Captures<'o> + 'static {} ``` We need to have existential regions to avoid [unsoundness](rust-lang/rust#116040 (comment)) when an opaque captures a region which is not represented in its own substs but which outlives a region that does. ## Read more Context: rust-lang/rust#115822 (comment) (for the liveness case) More context: rust-lang/rust#42940 (comment) (for the opaque capture case, which this does not fix) [^1]: except for bivariant region args in opaques, which will become less relevant when we move onto edition 2024 capture semantics for opaques.
…sound, r=aliemjay Consider alias bounds when computing liveness in NLL (but this time sound hopefully) This is a revival of #116040, except removing the changes to opaque lifetime captures check to make sure that we're not triggering any unsoundness due to the lack of general existential regions and the currently-existing `ReErased` hack we use instead. r? `@aliemjay` -- I appreciate you pointing out the unsoundenss in the previous iteration of this PR, and I'd like to hear that you're happy with this iteration of this PR before this goes back into FCP :> Fixes #116794 as well --- (mostly copied from #116040 and reworked slightly) # Background Right now, liveness analysis in NLL is a bit simplistic. It simply walks through all of the regions of a type and marks them as being live at points. This is problematic in the case of aliases, since it requires that we mark **all** of the regions in their args[^1] as live, leading to bugs like #42940. In reality, we may be able to deduce that fewer regions are allowed to be present in the projected type (or "hidden type" for opaques) via item bounds or where clauses, and therefore ideally, we should be able to soundly require fewer regions to be live in the alias. For example: ```rust trait Captures<'a> {} impl<T> Captures<'_> for T {} fn capture<'o>(_: &'o mut ()) -> impl Sized + Captures<'o> + 'static {} fn test_two_mut(mut x: ()) { let _f1 = capture(&mut x); let _f2 = capture(&mut x); //~^ ERROR cannot borrow `x` as mutable more than once at a time } ``` In the example above, we should be able to deduce from the `'static` bound on `capture`'s opaque that even though `'o` is a captured region, it *can never* show up in the opaque's hidden type, and can soundly be ignored for liveness purposes. # The Fix We apply a simple version of RFC 1214's `OutlivesProjectionEnv` and `OutlivesProjectionTraitDef` rules to NLL's `make_all_regions_live` computation. Specifically, when we encounter an alias type, we: 1. Look for a unique outlives bound in the param-env or item bounds for that alias. If there is more than one unique region, bail, unless any of the outlives bound's regions is `'static`, and in that case, prefer `'static`. If we find such a unique region, we can mark that outlives region as live and skip walking through the args of the opaque. 2. Otherwise, walk through the alias's args recursively, as we do today. ## Limitation: Multiple choices This approach has some limitations. Firstly, since liveness doesn't use the same type-test logic as outlives bounds do, we can't really try several options when we're faced with a choice. If we encounter two unique outlives regions in the param-env or bounds, we simply fall back to walking the opaque via its args. I expect this to be mostly mitigated by the special treatment of `'static`, and can be fixed in a forwards-compatible by a more sophisticated analysis in the future. ## Limitation: Opaque hidden types Secondly, we do not employ any of these rules when considering whether the regions captured by a hidden type are valid. That causes this code (cc #42940) to fail: ```rust trait Captures<'a> {} impl<T> Captures<'_> for T {} fn a() -> impl Sized + 'static { b(&vec![]) } fn b<'o>(_: &'o Vec<i32>) -> impl Sized + Captures<'o> + 'static {} ``` We need to have existential regions to avoid [unsoundness](rust-lang/rust#116040 (comment)) when an opaque captures a region which is not represented in its own substs but which outlives a region that does. ## Read more Context: rust-lang/rust#115822 (comment) (for the liveness case) More context: rust-lang/rust#42940 (comment) (for the opaque capture case, which this does not fix) [^1]: except for bivariant region args in opaques, which will become less relevant when we move onto edition 2024 capture semantics for opaques.
Since
SomeFuture
borrows'a Client
, I'd expectimpl Future + 'a
to be the correct return type, but it gives this error:Changing
_body
to have an explicit lifetime like_body: &'b B
where'b
is independent of'a
or where'a: 'b
does not change the error. This and the original error make it seem that returning animpl trait
is somehow causing the_body
parameter to get the'a
lifetime, even though it's clearly unused, let alone used in a way that it would require the'a
lifetime.Changing
(1)
fromimpl Future + 'a
toSomeFuture<'a>
fixes it.Changing
(1)
fromimpl Future + 'a
toBox<Future + 'a>
and returningBox::new(SomeFuture(self))
fixes it.The text was updated successfully, but these errors were encountered: