generalize impl trait to permit multiple lifetime bounds #61775

nikomatsakis · 2019-06-12T15:38:47Z

Generalizes the region solver to support "pick constraints". These have the form:

pick R0 from [R1..Rn]

where R1..Rn are called the "option regions". The idea is that R0 must be equal to some region in the set R1..Rn. These constraints are then used to handle cases like this:

fn foo<'a, 'b>(...) -> impl Trait<'a, 'b> { .. }

The problem here is that every region R in the hidden type must be equal to either 'a or 'b (or 'static) -- in the past, the only kinds of constraints we had were outlives constraints, and since 'a and 'b are unrelated, there was no outlives constraint we could issue that would enforce that (R: 'a and R: 'b are both too strict, for example). But now we can issue a pick constraint: pick R from ['a, 'b].

In general, solving pick constraints is tricky. We integrate them into the solver as follows. In general, during the propagation phase, we are monotonically growing a set of inference regions. To handle a case like pick R from [O...], where O... represents the option regions, we do the following:

Look for all the lower bounds of the region R -- that is, every region LB such that R: LB must hold.
Look for all the upper bounds of the region R -- that is, every region UB such that UB: R must hold.
Let the viable options be each option region O such that UB: O and O: LB for each UB, LB bound.
Find the minimal viable option M, where O: M holds for every option region O.

If there is such a minimal viable option, then we make R: M. (This may in turn influence other bits of inference.) If there is no minimal viable option, either because all options were eliminated or because none of the remaining options are minimal, we do nothing. Ultimately, if the pick constraint is not satisfied, an error is reported.

For this logic, we currently require that the option regions O are always lifetime parameters. To determine the bounds, we walk the various outlives edges that were otherwise introduced.

r? @matthewjasper
cc @cramertj

Fixes #56238

TODO:

Error messages include region variable info sometimes, how to fix?
Tests for bare existential type and other impl Trait usage

nikomatsakis · 2019-06-12T15:42:32Z

One problem I realize I never solved: the error messages for display regions presently talk about the hidden region being a region variable... I'm not entirely sure how to do better there yet.

nikomatsakis · 2019-06-12T15:45:11Z

Another problem: We need some tests for this in conjunction with impl Trait used in other locations.

src/test/ui/async-await/async-fn-multiple-lifetimes.rs

src/librustc/infer/opaque_types/mod.rs

src/test/ui/impl-trait/multiple-lifetimes/ordinary-bounds-pick-original.stderr

src/librustc_mir/borrow_check/nll/region_infer/mod.rs

src/librustc/infer/lexical_region_resolve/mod.rs

src/test/ui/impl-trait/multiple-lifetimes/ordinary-bounds-pick-other.stderr

src/librustc/infer/lexical_region_resolve/mod.rs

src/librustc/infer/mod.rs

src/librustc/infer/opaque_types/mod.rs

src/librustc_data_structures/graph/vec_graph/test.rs

Centril · 2019-06-13T16:45:01Z

cc #61773

Centril · 2019-06-16T03:37:59Z

We discussed this on Thursday's language team meeting. The conclusion was that @nikomatsakis will write more elaborate descriptions and a section to the rustc guide / or an RFC at their option. Moreover, the bits for explicit impl Trait syntax should be feature gated initially so that we can ship the narrower subset for async fn.

eddyb · 2019-06-16T10:43:48Z

I am was confused about the naming "pick", how does it differ from this (which I've employed recently):

fn foo<'a: 'c, 'b: 'c, 'c>(...) -> impl Trait<'a, 'b> + 'c { .. }

Well, I just re-read the PR description and I think the implication there (that should probably be written out) is that the new "pick" constraint is used to type-check the body of foo, not its users.
That is, externally, there is no lifetime constraint, only the knowledge that 'a and 'b were captured.

The problem here is that every region R in the hidden type must be equal to either 'a or 'b

There's another implication here, that the hidden type starts out with region inference variables, which must be resolved somehow. The "pick" constraint guides resolution to use one of the captured lifetimes.

A couple more nits with the description:

"pick" is a bit too imperative to count as a "constraint" IMO (compared to, say, set membership)
- i.e. R0 ∈ {R1..Rn} instead of pick R0 from [R1..Rn]
"option" in the context of Rust seems ambiguous
- "choice" or "alternative" might be better? not sure

For this logic, we currently require that the option regions O are always lifetime parameters.

This gives me two ideas:

does this have to be a constraint? could the solving algorithm described "simply" be ran, without any side-effects, once the hidden type is known, and the resulting "exported version" unified back in?
do we have to restrict the lifetimes to the captured ones, or could we start with a version that is less specific and "simply" looks for a parameter? aka MustBeParam(R) or even Escapes(R)
- for e.g. fn foo<'a>(x: &'a T) -> impl Trait { x } this could find 'a and tell you to mention 'a in the return type, e.g. + Captures<'a> (or whatever solution we'll end up with)

nikomatsakis · 2019-06-17T13:29:25Z

@eddyb

Well, I just re-read the PR description and I think the implication there (that should probably be written out) is that the new "pick" constraint is used to type-check the body of foo, not its users.

I plan to write out a more complete description, I'll try to make that clear.

"pick" is a bit too imperative to count as a "constraint" IMO (compared to, say, set membership)

"option" in the context of Rust seems ambiguous

I initially called them "in constraints", i.e., the region must be "in" this set, but I found it very hard to write about. I suppose "member" constraints would be ok. I like the term pick because it is relatively unique. I'll consider changing to 'm member of ['c...] where 'c are the choice regions.

does this have to be a constraint? could the solving algorithm described "simply" be ran, without any side-effects, once the hidden type is known, and the resulting "exported version" unified back in?

I don't really know what this means. But the reason it is expressed as a constraint is because it affects region variables -- those variables could be related to other variables, and so when we apply the pick constraint, we might affect other regions that do not directly appear in the hidden type.

I think one way to think of it is that we are dynamically growing the set of "outlives" constraints, but we can't tell (yet) which outlives constraint to apply -- we have to get part-way through solving before we can see that.

do we have to restrict the lifetimes to the captured ones, could we start with a version that is less specific and "simply" looks for a parameter? aka MustBeParam(R) or even Escapes(R)

It is not enough to just capture "some" parameter, that might still yield errors. It would probably suffice for simple async fn examples, though, as there is only one reasonable choice, and all lifetime parameters are captured. It wouldn't simplify the code that much I don't think, though -- I guess a little. (It would also be less useful as a starting point for let x: impl Trait<'_>, I imagine.)

for e.g. fn foo<'a>(x: &'a T) -> impl Trait { x } this could find 'a and tell you to mention 'a in the return type

we do try to give useful errors of this kind already, I think, but that seems somewhat separable.

nikomatsakis · 2019-06-17T14:20:40Z

@eddyb

Regarding this example, a few more thoughts. It is true that a workaround like this works as well:

fn foo<'a: 'c, 'b: 'c, 'c>(...) -> impl Trait<'a, 'b> + 'c { .. }

I think that this workaround will work in all cases, but it does require changing the function declaration. As you noted, the approach in this PR is internal to the function, and so it doesn't affect the "public interface". However, as such it is also less general than the workaround, since it can't handle cases like this (which I noted in a comment somewhere);

fn foo<'a, 'b>(x: &'a u32, y: &'b u32) -> impl Foo<'a, 'b> { if something { a } else { b } }

The problem here is that the resulting lifetime cannot be either 'a or 'b, but is rather their "intersection". Translating to the format you suggested gives a name to that intersection ('c) and hence works.

It would be possible to introduce such an "intersection lifetime" when desugaring async fn. We considered that approach but it also wouldn't help any other impl Trait users -- I'm also not sure how difficult it would be to do. Sad to say that our desugaring infrastructure doesn't usually make that sort of thing easy, but maybe things have improved.

I don't believe we can do such a workaround internally -- or at least, not on the current infrastructure. I had hoped is that, longer term, we could support the example above by extending the grammar of regions -- e.g., in polonius, where regions are sets of loans, it would be (I think) relatively natural to permit the compiler to have regions of the form a|b (i.e., the set-union of a and b). We could then infer the hidden type above to be &(a|b) u32. I think this would flow relatively naturally from Polonius's setup, but I confess I've not given it a lot of deep thought. We did talk a bit on Zulip about what it would take to support that in the current solver, and it seemed like it would be hard. It seems a bit surprising to me that it would be fundamentally easier in Polonius, so it's probably worth thinking that through a bit to see if I'm .. um .. missing something (it's easy to get confused).

(Of course, there has also been discussion of extending the type system with "existential regions". Then the idea would be that the hidden type would be inferred to something like exists<'c> where ('a: 'c, 'b: 'c) &'c u32. Plausibly something like that could be made sound but it is a pretty subtle extension so I'm not inclined to think about it now.)

In my view, the current approach is fairly conservative (i.e., it doesn't affect the type system in any deep way, though it does influence the inferencer deeply) and yet far-reaching (it affects not only async fn but other impl Trait users as well). It is not the most conservative thing we could do, though, which would probably be to introduce a dummy intersection lifetime for async fn desugaring.

nikomatsakis · 2019-06-18T14:59:11Z

I started working on more documentation in rust-lang/rustc-dev-guide#344 but I didn't get that far yet.

eddyb · 2019-06-18T15:30:57Z

We could then infer the hidden type above to be &(a|b) u32.

Yeah, that's what I thought this was, before realizing it's internal to the body and not leaking into the signature. And I was going to ask "why put it in a constraint and not in the lifetimes themselves".

I don't really know what this means. But the reason it is expressed as a constraint is because it affects region variables -- those variables could be related to other variables, and so when we apply the pick constraint, we might affect other regions that do not directly appear in the hidden type.

I think one way to think of it is that we are dynamically growing the set of "outlives" constraints, but we can't tell (yet) which outlives constraint to apply -- we have to get part-way through solving before we can see that.

I guess it would make sense if the pick constraint needs to "stay around" in some sort of "partially solved" state - either way, it's probably safer to have it as a constraint even if a hackier solution might exist.

Minimum rustc bump required for rust-lang/rust#61775

@cramertj

…amertj Stabilize `async_await` in Rust 1.39.0 Here we stabilize: - free and inherent `async fn`s, - the `<expr>.await` expression form, - and the `async move? { ... }` block form. Closes rust-lang#62149. Closes rust-lang#50547. All the blockers are now closed. <details> - [x] FCP in rust-lang#62149 - [x] rust-lang#61949; PR in rust-lang#62849. - [x] rust-lang#62517; PR in rust-lang#63376. - [x] rust-lang#63225; PR in rust-lang#63501 - [x] rust-lang#63388; PR in rust-lang#63499 - [x] rust-lang#63500; PR in rust-lang#63501 - [x] rust-lang#62121 (comment) - [x] Some tests for control flow (PR rust-lang#63387): - `?` - `return` in `async` blocks - `break` - [x] rust-lang#61775 (comment), i.e. tests for rust-lang#60944 with `async fn`s instead). PR in rust-lang#63383 </details> r? @cramertj

Minimum rustc bump required for rust-lang/rust#61775

…raints-61997, r=jackh726 stabilize member constraints Stabilizes the use of "member constraints" in solving `impl Trait` bindings. This is a step towards stabilizing a "MVP" of "named impl Trait". # Member constraint stabilization report | Info | | | --- | --- | | Tracking issue | [rust-lang#61997](rust-lang#61997) | | Implementation history | [rust-lang#61775] | | rustc-dev-guide coverage | [link](https://rustc-dev-guide.rust-lang.org/borrow_check/region_inference/member_constraints.html) | | Complications | [rust-lang#61773] | [rust-lang#61775]: rust-lang#61775 [rust-lang#61773]: rust-lang#61773 ## Background Member constraints are an extension to our region solver that was introduced to make async fn region solving tractable. There are used in situations like the following: ```rust fn foo<'a, 'b>(...) -> impl Trait<'a, 'b> { .. } ``` The problem here is that every region R in the hidden type must be equal to *either* `'a` *or* `'b` (or `'static`). This cannot be expressed simply via 'outlives constriants' like `R: 'a`. Therefore, we introduce a 'member constraint' `R member of ['a, 'b]`. These constraints were introduced in [rust-lang#61775]. At the time, we kept them feature gated and used them only for `impl Trait` return types that are derived from `async fn`. The intention, however, was always to support them in other contexts once we had time to gain more experience with them. **In the time since their introduction, we have encountered no surprises or bugs due to these member constraints.** They are tested extensively as part of every async function that involves multiple unrelated lifetimes in its arguments. ## Tests The behavior of member constraints is covered by the following tests: * [`src/test/ui/async-await/multiple-lifetimes`](https://github.com/rust-lang/rust/tree/20e032e65007ff1376e8480c1fbdb0a5068028fa/src/test/ui/async-await/multiple-lifetimes) -- tests using the async await, which are mostly already stabilized * [`src/test/ui/impl-trait/multiple-lifetimes.rs`](https://github.com/rust-lang/rust/blob/20e032e65007ff1376e8480c1fbdb0a5068028fa/src/test/ui/impl-trait/multiple-lifetimes.rs) * [`src/test/ui/impl-trait/multiple-lifetimes/ordinary-bounds-unsuited.rs`](https://github.com/rust-lang/rust/blob/20e032e65007ff1376e8480c1fbdb0a5068028fa/src/test/ui/impl-trait/multiple-lifetimes/ordinary-bounds-unsuited.rs) * [`src/test/ui/async-await/multiple-lifetimes/ret-impl-trait-fg.rs`](https://github.com/rust-lang/rust/blob/20e032e65007ff1376e8480c1fbdb0a5068028fa/src/test/ui/async-await/multiple-lifetimes/ret-impl-trait-fg.rs) * [`src/test/ui/async-await/multiple-lifetimes/ret-impl-trait-one.rs`](https://github.com/rust-lang/rust/blob/20e032e65007ff1376e8480c1fbdb0a5068028fa/src/test/ui/async-await/multiple-lifetimes/ret-impl-trait-one.rs) These tests cover a number of scenarios: * `-> implTrait<'a, 'b>` with unrelated lifetimes `'a` and `'b`, as described above * `async fn` that returns an `impl Trait` like the previous case, which desugars to a kind of "nested" impl trait like `impl Future<Output = impl Trait<'a, 'b>>` ## Potential concerns There is a potential interaction with `impl Trait` on local variables, described in [rust-lang#61773]. The challenge is that if you have a program like: ```rust= trait Foo<'_> { } impl Foo<'_> for &u32 { } fn bar() { let x: impl Foo<'_> = &44; // let's call the region variable for `'_` `'1` } ``` then we would wind up with `'0 member of ['1, 'static]`, where `'0` is the region variable in the hidden type (`&'0 u32`) and `'1` is the region variable in the bounds `Foo<'1>`. This is tricky because both `'0` and `'1` are being inferred -- so making them equal may have other repercussions. That said, `impl Trait` in bindings are not stable, and the implementation is pretty far from stabilization. Moreover, the difficulty highlighted here is not due to the presence of member constraints -- it's inherent to the design of the language. In other words, stabilizing member constraints does not actually cause us to accept anything that would make this problem any harder. So I don't see this as a blocker to stabilization of member constraints; it is potentially a blocker to stablization of `impl trait` in let bindings.

rust-highfive assigned matthewjasper Jun 12, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 12, 2019