-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Implement impl Trait
in return type position by anonymization.
#35091
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
Will the |
@Ryman Not unless the type is normalized with |
☔ The latest upstream changes (presumably #35143) made this pull request unmergeable. Please resolve the merge conflicts. |
506c6ff
to
6a3ee28
Compare
@@ -174,6 +174,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { | |||
ty::TyEnum(..) | // OutlivesNominalType | |||
ty::TyStruct(..) | // OutlivesNominalType | |||
ty::TyBox(..) | // OutlivesNominalType (ish) | |||
ty::TyAnon(..) | // OutlivesNominalType (ish) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment seems wrong. Perhaps "OutlivesProjectionComponents (ish)" -- really this merits a revising of the RFC 1214 rules to include OutlivesAnonType
or something, but that's ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably this code is doing the right thing, but it's a bit more conservative than it might otherwise be. Really there are choices here just like for projections. Consider for example if you had trait Foo<'a,'b>: 'a
, then you could in theory derive that impl Foo<'a,'b>: 'a
even if 'b: 'a
doesn't necessarily hold. Something similar could occur with trait Foo<'a, T>: 'a
, where impl Foo<'a, &'b T>: 'a
might hold even if 'b: 'a
does not hold.
It seems in a way like an odd rule, since the type name that the type system gives includes lifetimes (like 'b
) that are not necessarily in scope, but the same thing can occur with projections -- the point is that, after monomorphization, the normalized form of that type is known to outlive 'a
, even if all components pre-normalization do not.
/me has that itchy feeling of wanting to revisit a richer, formalized version of this, to try and have a good reference. But nothing here makes me too scared yet. =)
☔ The latest upstream changes (presumably #35267) made this pull request unmergeable. Please resolve the merge conflicts. |
I don't like the "capture all lifetimes in the |
☔ The latest upstream changes (presumably #35166) made this pull request unmergeable. Please resolve the merge conflicts. |
/// scope, that anonymized types will close over. This property is | ||
/// controlled by the region scope because it's fine-grained enough | ||
/// to allow restriction of anonymized types to the syntactical extent | ||
/// of a function's return type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I can't make heads or tails of this comment :) An example would be great
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you talking about allowing impl Trait
, or the "generics in scope"? The latter would be easy to provide an example from (<'a, T>
for fn foo<'a, T>(...) -> impl Trait
), but controlling whether impl Trait
is legal in a certain position is harder to explain.
For it to make more sense, rscope
would have to become one of several properties that get propagated recursively.
At the same time, the ItemContext
in typeck::collect
may be a better choice now for holding the "generics in scope", as it's the only AstConv
implementation which allows impl Trait
, but it's still not that uniform (only the return type of a function or inherent impl method supports it, not just any type in the signature of those items).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eddyb after I read below I figured out what the comment meant. I think something like this would have helped me:
If this scope allows anonymized types, return the generics in scope that anonymized types will close over. For example, if you have a function like:
fn foo<'a, T>() -> impl Trait { ... }
then, for the scope that is used when handling the return type, this function would return 'a
and T
. This property is controlled by the region scope because it's fine-grained enough to allow restriction of anonymized types to the syntactical extentof a function's return type.
I went through the RFC and made a minimal list of test cases I think would like to see:
|
Er, I posted this comment earlier, but in the wrong place: @eddyb ok so as discussed on IRC, I am in favor of landing this PR, on the assumption that we will address some of the thornier issues later. One thing I think is missing is negative tests. I'd like to see at least some tests that if you try to return incompatible types that will result in an error. Probably some (more?) tests targeting "Auto trait" inference -- and the limits, e.g. around specialization -- are warranted as well. One thing I have done in the past -- but didn't do yet for this PR -- is to make a list, based on the RFC, not the code, of things that might need to be tested, and then compare the code against that list. I'm happy to go through and do it, or you can and post it somewhere. :) (In case it wasn't clear, the code itself seems good to me! Nice work as usual.) |
c32f829
to
c0b7f97
Compare
@bors r+ |
📌 Commit c0b7f97 has been approved by |
☔ The latest upstream changes (presumably #35592) made this pull request unmergeable. Please resolve the merge conflicts. |
@bors r=nikomatsakis |
📌 Commit 23f0494 has been approved by |
Implement `impl Trait` in return type position by anonymization. This is the first step towards implementing `impl Trait` (cc #34511). `impl Trait` types are only allowed in function and inherent method return types, and capture all named lifetime and type parameters, being invariant over them. No lifetimes that are not explicitly named lifetime parameters are allowed to escape from the function body. The exposed traits are only those listed explicitly, i.e. `Foo` and `Clone` in `impl Foo + Clone`, with the exception of "auto traits" (like `Send` or `Sync`) which "leak" the actual contents. The implementation strategy is anonymization, i.e.: ```rust fn foo<T>(xs: Vec<T>) -> impl Iterator<Item=impl FnOnce() -> T> { xs.into_iter().map(|x| || x) } // is represented as: type A</*invariant over*/ T> where A<T>: Iterator<Item=B<T>>; type B</*invariant over*/ T> where B<T>: FnOnce() -> T; fn foo<T>(xs: Vec<T>) -> A<T> { xs.into_iter().map(|x| || x): $0 where $0: Iterator<Item=$1>, $1: FnOnce() -> T } ``` `$0` and `$1` are resolved (to `iter::Map<vec::Iter<T>, closure>` and the closure, respectively) and assigned to `A` and `B`, after checking the body of `foo`. `A` and `B` are *never* resolved for user-facing type equality (typeck), but always for the low-level representation and specialization (trans). The "auto traits" exception is implemented by collecting bounds like `impl Trait: Send` that have failed for the obscure `impl Trait` type (i.e. `A` or `B` above), pretending they succeeded within the function and trying them again after type-checking the whole crate, by replacing `impl Trait` with the real type. While passing around values which have explicit lifetime parameters (of the function with `-> impl Trait`) in their type *should* work, regionck appears to assign inference variables in *way* too many cases, and never properly resolving them to either explicit lifetime parameters, or `'static`. We might not be able to handle lifetime parameters in `impl Trait` without changes to lifetime inference, but type parameters can have arbitrary lifetimes in them from the caller, so most type-generic usecases (or not generic at all) should not run into this problem. cc @rust-lang/lang
This is the first step towards implementing
impl Trait
(cc #34511).impl Trait
types are only allowed in function and inherent method return types, and capture all named lifetime and type parameters, being invariant over them.No lifetimes that are not explicitly named lifetime parameters are allowed to escape from the function body.
The exposed traits are only those listed explicitly, i.e.
Foo
andClone
inimpl Foo + Clone
, with the exception of "auto traits" (likeSend
orSync
) which "leak" the actual contents.The implementation strategy is anonymization, i.e.:
$0
and$1
are resolved (toiter::Map<vec::Iter<T>, closure>
and the closure, respectively) and assigned toA
andB
, after checking the body offoo
.A
andB
are never resolved for user-facing type equality (typeck), but always for the low-level representation and specialization (trans).The "auto traits" exception is implemented by collecting bounds like
impl Trait: Send
that have failed for the obscureimpl Trait
type (i.e.A
orB
above), pretending they succeeded within the function and trying them again after type-checking the whole crate, by replacingimpl Trait
with the real type.While passing around values which have explicit lifetime parameters (of the function with
-> impl Trait
) in their type should work, regionck appears to assign inference variables in way too many cases, and never properly resolving them to either explicit lifetime parameters, or'static
.We might not be able to handle lifetime parameters in
impl Trait
without changes to lifetime inference, but type parameters can have arbitrary lifetimes in them from the caller, so most type-generic usecases (or not generic at all) should not run into this problem.cc @rust-lang/lang