Implement `impl Trait` in return type position by anonymization. #35091

eddyb · 2016-07-28T16:21:04Z

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.:

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

rust-highfive · 2016-07-28T16:21:17Z

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

Ryman · 2016-07-28T21:01:52Z

$0 and $1 are resolved (to iter::Mapvec::Iter<T, closure> and the closure, respectively) and assigned to A and B, after checking the body of foo.

Will the #[must_use] attribute still cause a warning if the return value is unused in this case?

eddyb · 2016-07-28T21:10:53Z

@Ryman Not unless the type is normalized with Reveal::All in the lint, no.

bors · 2016-08-01T00:39:41Z

☔ The latest upstream changes (presumably #35143) made this pull request unmergeable. Please resolve the merge conflicts.

nikomatsakis · 2016-08-05T17:56:19Z

src/librustc/ty/outlives.rs

@@ -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)


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.

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. =)

bors · 2016-08-06T09:28:07Z

☔ The latest upstream changes (presumably #35267) made this pull request unmergeable. Please resolve the merge conflicts.

arielb1 · 2016-08-07T13:16:02Z

I don't like the "capture all lifetimes in the impl trait" approach and would prefer something more like lifetime elision.

bors · 2016-08-09T21:12:21Z

☔ The latest upstream changes (presumably #35166) made this pull request unmergeable. Please resolve the merge conflicts.

nikomatsakis · 2016-08-10T09:57:19Z

src/librustc_typeck/rscope.rs

+    /// 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.


Nit: I can't make heads or tails of this comment :) An example would be great

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).

@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.

nikomatsakis · 2016-08-10T14:22:25Z

nikomatsakis · 2016-08-10T15:11:56Z

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.)

nikomatsakis · 2016-08-11T18:27:26Z

@bors r+

bors · 2016-08-11T18:27:26Z

📌 Commit c0b7f97 has been approved by nikomatsakis

bors · 2016-08-12T02:44:46Z

☔ The latest upstream changes (presumably #35592) made this pull request unmergeable. Please resolve the merge conflicts.

…ey appear.

…d impl methods.

eddyb · 2016-08-12T04:13:11Z

@bors r=nikomatsakis

bors · 2016-08-12T04:13:12Z

📌 Commit 23f0494 has been approved by nikomatsakis

bors · 2016-08-12T08:26:12Z

⌛ Testing commit 23f0494 with merge f55ac69...

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

bors · 2016-08-12T11:30:46Z

rust-highfive assigned nikomatsakis Jul 28, 2016

eddyb force-pushed the impl-trait branch from 1a2bd4f to 8d9a6f7 Compare July 28, 2016 16:58

eddyb mentioned this pull request Jul 31, 2016

Tracking issue for impl Trait (RFC 1522, RFC 1951, RFC 2071) #34511

Closed

29 tasks

eddyb force-pushed the impl-trait branch 5 times, most recently from 506c6ff to 6a3ee28 Compare August 4, 2016 16:25

nikomatsakis reviewed Aug 5, 2016
View reviewed changes

eddyb force-pushed the impl-trait branch from 6a3ee28 to 6b420b7 Compare August 6, 2016 00:12

eddyb force-pushed the impl-trait branch from 6b420b7 to bc131a8 Compare August 6, 2016 21:50

nikomatsakis reviewed Aug 10, 2016
View reviewed changes

eddyb force-pushed the impl-trait branch 3 times, most recently from c32f829 to c0b7f97 Compare August 10, 2016 21:04

eddyb mentioned this pull request Aug 11, 2016

Remove the ParamSpace separation from formal and actual generics in rustc. #35605

Merged

eddyb added 9 commits August 12, 2016 06:43

rustc: rename ProjectionMode and its variant to be more memorable.

d1d16c9

rustc: always normalize projections in ty::layout regardless where th…

ab26dbb

…ey appear.

rustc: don't reveal specializable polymorphic projections.

c976e07

syntax: add anonymized type syntax, i.e. impl TraitA+TraitB.

f0baec6

rustc: add TyAnon (impl Trait) to the typesystem.

ef11d4e

typeck: disallow impl Trait outside of return types of functions an…

1ef7ddf

…d impl methods.

typeck: record impl Trait concrete resolutions.

d92e594

typeck: leak auto trait obligations through impl Trait.

08bf9f6

test: add more extensive tests for impl Trait.

23f0494

eddyb force-pushed the impl-trait branch from c0b7f97 to 23f0494 Compare August 12, 2016 04:12

bors merged commit 23f0494 into rust-lang:master Aug 12, 2016

ticki mentioned this pull request Aug 12, 2016

Detect situations where impl Trait is preferred. rust-lang/rust-clippy#1161

Open

eddyb deleted the impl-trait branch August 12, 2016 14:09

TimNN mentioned this pull request Aug 28, 2016

ICE: the type u8 has an unknown layout #36036

Closed

m4rw3r mentioned this pull request Aug 28, 2016

impl Trait and making Chomp require nightly m4rw3r/chomp#50

Closed

Marwes mentioned this pull request Aug 30, 2016

Example of higher order parser generating functions. Marwes/combine#66

Closed

kennytm mentioned this pull request Jan 28, 2018

Incorrect use of traits::SizedReturnType #47830

Closed

Centril mentioned this pull request Jul 28, 2019

Meta tracking issue for impl Trait #63066

Open

11 tasks

eddyb mentioned this pull request Dec 9, 2019

rustc: include ParamEnv in global trait select/eval cache keys. #66963

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `impl Trait` in return type position by anonymization. #35091

Implement `impl Trait` in return type position by anonymization. #35091

eddyb commented Jul 28, 2016 •

edited

rust-highfive commented Jul 28, 2016

Ryman commented Jul 28, 2016

eddyb commented Jul 28, 2016

bors commented Aug 1, 2016

nikomatsakis Aug 5, 2016

nikomatsakis Aug 5, 2016

bors commented Aug 6, 2016

arielb1 commented Aug 7, 2016

bors commented Aug 9, 2016

nikomatsakis Aug 10, 2016

eddyb Aug 10, 2016 •

edited

nikomatsakis Aug 10, 2016

nikomatsakis commented Aug 10, 2016 •

edited by eddyb

nikomatsakis commented Aug 10, 2016

nikomatsakis commented Aug 11, 2016

bors commented Aug 11, 2016

bors commented Aug 12, 2016

eddyb commented Aug 12, 2016

bors commented Aug 12, 2016

bors commented Aug 12, 2016

bors commented Aug 12, 2016

Implement impl Trait in return type position by anonymization. #35091

Implement impl Trait in return type position by anonymization. #35091

Conversation

eddyb commented Jul 28, 2016 • edited

rust-highfive commented Jul 28, 2016

Ryman commented Jul 28, 2016

eddyb commented Jul 28, 2016

bors commented Aug 1, 2016

nikomatsakis Aug 5, 2016

Choose a reason for hiding this comment

nikomatsakis Aug 5, 2016

Choose a reason for hiding this comment

bors commented Aug 6, 2016

arielb1 commented Aug 7, 2016

bors commented Aug 9, 2016

nikomatsakis Aug 10, 2016

Choose a reason for hiding this comment

eddyb Aug 10, 2016 • edited

Choose a reason for hiding this comment

nikomatsakis Aug 10, 2016

Choose a reason for hiding this comment

nikomatsakis commented Aug 10, 2016 • edited by eddyb

nikomatsakis commented Aug 10, 2016

nikomatsakis commented Aug 11, 2016

bors commented Aug 11, 2016

bors commented Aug 12, 2016

eddyb commented Aug 12, 2016

bors commented Aug 12, 2016

bors commented Aug 12, 2016

bors commented Aug 12, 2016

Implement `impl Trait` in return type position by anonymization. #35091

Implement `impl Trait` in return type position by anonymization. #35091

eddyb commented Jul 28, 2016 •

edited

eddyb Aug 10, 2016 •

edited

nikomatsakis commented Aug 10, 2016 •

edited by eddyb