rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

Precise capturing

Open traviscross opened this issue 3 months ago • 19 comments

To fully stabilize, in Rust 2024, the Lifetime Capture Rules 2024 that we accepted in RFC 3498, we need to stabilize some means of precise capturing. This RFC provides that means.

This RFC adds use<..> syntax for specifying which generic parameters should be captured in an opaque RPIT-like impl Trait type, e.g. impl use<'t, T> Trait. This solves the problem of overcapturing and will allow the Lifetime Capture Rules 2024 to be fully stabilized for RPIT in Rust 2024.

One way to think about use<..> is that, in Rust use brings things into scope, and here we are bringing certain generic parameters into scope for the hidden type.

For some history about the progress toward this feature predating this RFC, see this comment.

Rendered

traviscross avatar Apr 24 '24 07:04 traviscross

@rfcbot fcp merge

We've tried hard to avoid an explicit syntax like this but I think it's clear by now that it will be useful and it unblocks important Edition work. We had a reasonably thorough deep dive into the syntactic options and I believe the RFC lays out the options pretty well and the tradeoffs around them. Personally while I have some minor qualms about overloading use, I think it's the best option overall.

Note that in this fcp I am explicitly wearing my @rust-lang/lang hat -- I think the @rust-lang/types team should (before stabilization) vet the overall semantics and our implementation thereof but that's not really a question to be answered in the RFC.

nikomatsakis avatar Apr 24 '24 14:04 nikomatsakis

Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged team members:

  • [x] @joshtriplett
  • [x] @nikomatsakis
  • [x] @pnkfelix
  • [ ] @scottmcm
  • [ ] @tmandry

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. See this document for info about what commands tagged team members can give me.

rfcbot avatar Apr 24 '24 14:04 rfcbot

:bell: This is now entering its final comment period, as per the review above. :bell:

rfcbot avatar Apr 24 '24 17:04 rfcbot

Since this use<..> thing is part of the type signature, could you clarify if the capture list is changed, when it will be considered breaking change or not?

From the caller's point of view, if a function's RPIT (in covariant position only) has a capture removed, the hidden type's potential shortest lifetime is lengthened, which is compatible with existing caller code. So I think semver should allow removing captures from covariant-position RPIT in a minor update.

- fn callee1<'a, 'b>(aaa: &'a u8, bbb: &'b u8) -> &'a u8 { &0 }
+ fn callee1<'a, 'b>(aaa: &'a u8, bbb: &'b u8) -> &'static u8 { &0 }

- fn callee2<'a, 'b>(aaa: &'a u8, bbb: &'b u8) -> impl use<'a> Sized { &0 }
+ fn callee2<'a, 'b>(aaa: &'a u8, bbb: &'b u8) -> impl use<> Sized { &0 }

Meanwhile for TAIT, ATPIT and RPITIT changing the capture list in either direction should be considered a major breaking change.

(For APIT the capture list is irrelevant.)

kennytm avatar Apr 25 '24 01:04 kennytm

So, just procedurally...

While I do think solving the underlying ergonomic and semantic issues here are quite important, I do have to note that this syntax and RFC seem to have moved quite fast. Even for me - who, while doesn't attend lang meetings, does try to keep up to date of active lang things - I could have almost missed this. To put some dates to this on typical milestones in our current processes:

I know we're on a time crunch because of the edition, but I worry about this all moving just a bit too fast. Of course, accepting this RFC doesn't necessarily mean that actually stabilizing this feature will also happen quickly, but I worry that the pace so far is a harbinger for that. I hate to sit here and be the one that say "wait, we're moving too fast", especially because not moving fast here puts edition work in jeopardy, but I'd rather this concern be voiced than ignored.

I do want to be clear that my above comments have little reflection on my thoughts on the contents of this RFC or semantics of this feature. My concerns here apply to even the best of features (to put it into perspective: let-else, which imo is a very clear win all around took about a month from RFC open to RFC merge, with a fair amount of prior discussion; and this is what I consider a "fast" RFC process).

jackh726 avatar Apr 25 '24 02:04 jackh726

So, just procedurally...

I know we're on a time crunch because of the edition, but I worry about this all moving just a bit too fast.

I agree. I'm delighted to see so much design activity happening over the past few months. However, it does feel like some RFCs coming from core team members have been accepted unusually quickly.

The current pace is probably just a sign of how well these RFCs are written and how much design work has happened already with the involvement of the team that votes on the RFC. However, the RFC is where many in the broader Rust community might first hear about a new proposal and chime in with suggestions and concerns. At least I would find it harder to speak up after the FCP has already started.

Perhaps we could pause the FCP, if possible in the edition timetable, and wait for e.g. one "This week in Rust" news cycle to have publicised the RFC?

juntyr avatar Apr 25 '24 04:04 juntyr

@jackh726: There's a long story on this particular feature going back at least nine months. We discussed it on the lang side prior to accepting the Lifetime Capture Rules 2024 (RFC 3498). There was discussion of whether we needed to do this at that time. This feature is discussed specifically in that RFC (and has been discussed periodically since).

In the triage meeting on 2024-03-27, we discussed how we may need to move forward on this. In the triage meeting on 2024-04-03, we discussed how we in fact do need to move forward on this, and we decided to discuss this in our planning meeting on that same day.

In that planning meeting on 2024-04-03, we discussed this feature for over an hour with all members present. We settled on a unanimous consensus that this was needed. There were no open semantic questions of any significance for how this should work. We then committed to and worked through a productive bikeshed to work out the syntax, and we scheduled a design meeting for 2024-04-24.

The next step was to write an RFC that would make a specific syntax proposal and lay out a justification for it along with an analysis of each of the alternatives (along with all the other detail and context that an RFC provides). This is that RFC. The syntax chosen here was the one that made everyone at least moderately happy (+0.5 or greater).

We then had a 90 minute design meeting on 2024-04-24 in which all members were present and reviewed this RFC in detail. We then discussed it in detail.

The RFC and the discussion was additionally informed by the experience gained by having implemented and landed the feature and migration lints on an experimental basis, and by having converted rustc to use it.

Given this extended period of collaboration and context and consensus building, it perhaps should not be surprising to see this particular RFC move quickly.

Hopefully this context is helpful. Lang did its due diligence here.

traviscross avatar Apr 25 '24 04:04 traviscross

Thanks for raising that concern @jackh726 and thanks @traviscross for cataloging the conversation trail. I'd say the discussion goes further back, in that the need for a way to explicit control captures has been debated on and off since impl Trait was first implemented back in 2015 or whatever (the hope at the time was that the compromise of "ignoring lifetimes that don't appear in the signature" would be good enough, and TAIT would be there to cover more extreme cases; but I think we've gained a lot more experience now and I see that this was not the right design).

I am sitting with the substance of @jackh726's point and feeling two conflicting things.

I agree it's good to give time for feedback and input. I don't want to move too quickly and wind up making mistakes, and feedback on the RFC thread can and is an important factor for consideration. I definitely want to be sure that we are not moving so fast that we don't get the input we need to make a responsible decision.

All that is true AND I also don't want to introduce delay for delay's sake. Put another way, the discussion on the RFC thread is only one of many sources of inputs that go into the design process -- and, frankly, often not the most important one, since it rarely captures a representative slice of Rust users. The real question is whether we are likely to learn things on the RFC that that we've not heard already. I think that is not likely in this case.

This is one of those cases where we wound up with a LOT of discussion before the actual RFC gets opened. This happens when we do early experimentation or blogging, for example. It is happening right now with return type notation, which has seen a lot of deep discussion, but we haven't gotten around to authoring the RFC yet. This isn't great, and I intend to fix it ASAP, but those things do happen and I think it's ok, as long as it's not the common case.

Put another way, I want us to make the right decisions, and I agree that haste and pressure can work against that -- but I also think that over-rotating on process can have the same impact, as people get too tired to pursue changes that make sense for fear of managing the RFC thread etc. It's a balance.

Anyway, I appreciate you raising the point @jackh726. I agree this RFC is moving faster than most and it's a good thing to call out. I also know that a lot of discussion amongst the lang team doesn't necessarily translate to visibility from the wider Rust community, so there is always the chance that there are perspectives we have not considered (part of why I still consider it important to author an RFC for all changes, no matter how much we've talked about it before). But I also want to acknowledge the nuance of the situation.

nikomatsakis avatar Apr 25 '24 14:04 nikomatsakis

At least I would find it harder to speak up after the FCP has already started.

This is unfortunate! The whole point of the FCP is to be sure we get input, so if it has a chilling effect, that seems unfortunate and maybe we can find a way to make its intent clearer.

Put another way, we added the FCP precisely so that (a) there was a 10-day period where people could raise concerns and (b) we were sure that pending decisions are going to show up in a noticeable place like TWIR etc.

nikomatsakis avatar Apr 25 '24 15:04 nikomatsakis

Thank you @traviscross and @nikomatsakis for all of the background on the long design road of how this RFC came to be and how the FCP brings additional visibility - I'm excited for the feature and the added level of control and expressiveness it brings to Rust

juntyr avatar Apr 25 '24 15:04 juntyr

Niko Matsakis wrote:

At least I would find it harder to speak up after the FCP has already started.

This is unfortunate! The whole point of the FCP is to be sure we get input, so if it has a chilling effect, that seems unfortunate and maybe we can find a way to make its intent clearer.

Put another way, we added the FCP precisely so that (a) there was a 10-day period where people could raise concerns and (b) we were sure that pending decisions are going to show up in a noticeable place like TWIR etc.

Exactly: the primary reason for the 10-day duration of an FCP is to ensure that anything occurring with a weekly cadence, including team meetings and TWiR, will take place at least once during the FCP.

The FCP is very much meant to be a comment period, and we take comments during that period seriously. For my part, whenever I'm considering merging an RFC, I look carefully at any comments that took place between entering FCP and the current time, to see if they've raised any new concerns that we need to consider. My threshold for turning those into blocking concerns and bringing them up in a meeting is pretty low, to ensure that we don't miss them.

I have definitely had the experience that some proposals can seem like they have a lot of momentum, and when that happens, it doesn't always feel like it's easy to raise potential concerns. We do want to make sure people always feel like they can raise concerns, or observations that might could come concerns, at any point, including during FCP, even if FCP is the first they've heard of the RFC.

Suggestions welcome for how we can make this more inviting, and make it clear that we are still receptive to information that might change our minds even during FCP.

joshtriplett avatar Apr 25 '24 15:04 joshtriplett

  1. Alternative to new keyword via we could add with instead.
  2. Backward compatibility: implicit rules (for use-less signatures) for Rust 2024 and Rust 2021 it is expected to be backward compatible since it is already in stable.

VitWW avatar Apr 25 '24 19:04 VitWW

As I explained in the lang team meeting, I prefer use<..> impl Trait over impl use<..> Trait. My points did not produce consensus in favor of changing the syntax: of the lang team members present, two preferred use-first, two preferred use-after, and one was agnostic. The RFC author preferred use-after, and I agreed not to block the RFC on the question.

The reasons for preferring this syntax are:

  1. Subjective (accessibility): The mnemonic users have for this feature is impl Trait. By putting more things between the impl and the Trait we make it less recognizable.
  2. Subjective (linguistic): This comes from english writing. It's simpler to say "using x, y, and z, I implement Foo" than "I implement – using x, y, and z – Foo". You would always prefer the first over the second.
  3. Technical: The comparison to impl for<'a> Foo<'a> + Bar and impl Foo + for<'a> Bar<'a> is informative. for<'a> applies to a particular bound in the set of bounds, while use<> applies to the entire impl Trait opaque type. Putting it out in front distinguishes it from for<> in this way.[^binder]
  4. Future consistency: The code sample for closures puts use in front of the || (for good reason, in my opinion). We should strive for consistency in the way such syntax is used. It would be somewhat unfortunate to have both of these in the language:
    • use<> || expr
    • impl use<> Trait
impl Trait for () {
    type Ty = impl Fn();
    fn define<T>(_: T) -> Self::Ty {
        use<> || ()
    //  ^^^^^^^^^^^
    //  ^ Captures no generic parameters.
    }
}

[^binder]: In my view, this goes against the idea, now expressed in the RFC, that use<> impl Trait "looks like a binder" ("binder" is jargon for for<'a>).

The arguments in favor of use-after were later added to the RFC.

Most of the discussion against revolved around the fact that it would require another migration to macro fragment specifiers (specifically ty) in 2024. But I think it's important to note that this would not prevent usage of this feature in un-migrated macros. Users could always put parentheses around these types when using macros which are not migrated to the new fragment specifiers.

// Before 2024:
my_unmigrated_macro! {
    fn foo<'t, T>(_: &'t (), x: T) -> impl Sized { x }
}

// After 2024 (note the parentheses):
my_unmigrated_macro! {
    fn foo<'t, T>(_: &'t (), x: T) -> (use<T> impl Sized) { x }
}

// After 2024, macro has been migrated to new matchers:
my_migrated_macro! {
    fn foo<'t, T>(_: &'t (), x: T) -> use<T> impl Sized { x }
}

After all the discussion I still find myself preferring use-before and wish we had more time to experiment with syntax. But this is the RFC, not stabilization, and I do not wish to block the RFC or the feature itself on this particular question.

tmandry avatar Apr 26 '24 23:04 tmandry

use<..> impl Trait is also more greppable than impl use<..> Trait. (That being said, I don't know that grepping for impl Trait is a task one would need to perform often.)

Jules-Bertholet avatar Apr 27 '24 01:04 Jules-Bertholet

Thanks @tmandry for writing that up. I've now greatly extended that section of the document to incorporate these[^2] and other points in favor of use<..> impl Trait[^1].

See in particular the discussion of the fundamental tension here. In short, the RFC lays out two intuitions for use<..>:

  • Intuition 1: use<..> applies generic arguments to the opaque type.
  • Intuition 2: use<..> brings generic parameters into scope for the hidden type.

These intuitions are both true, but they might suggest two different syntaxes, and this may be related to why the choice here is challenging.

[^1]: I've also removed the bit you mentioned in your footnote about it perhaps looking like a binder, as this indeed didn't seem a strong point to me either.

[^2]: I did not include the bits you mentioned about invoking macros across editions as the document already did not put much weight on and did not go into much detail on the macro fragment specifier migration. I did add further language to suggest that this migration cost is not major.

traviscross avatar Apr 27 '24 09:04 traviscross

This RFC does not specify what an RPITIT/ATPIT in a trait impl is allowed to capture in order for it to be compatible with the trait definition.

Here is an example of multiple trait implementations, the validity of which are not clear from the RFC.

// RPITIT only allowed to capture Y.
trait Trait<X, Y> {
    fn test() -> impl use<Y> Sized;
}


// Y = (A, B).
// Are we allowed to capture A?
// Is it considered a refinement to not capture B?
impl<A, B> Trait<(), (A, B)> for i8 {
    fn test() -> impl use<A> Sized {}
}

// Y = <A as Iterator>::Item.
// Are we allowed to capture A now that it appears in a projection type?
// I don't think so since projections do not constrain their parameters.
impl<A: Iterator> Trait<A, A::Item> for u8 {
    fn test() -> impl use<A> Sized {}
}

// Y = &'a str.
// The case for lifetimes and consts should be made clear.
impl<'a> Trait<(), &'a str> for i16 {
    fn test() -> impl use<'a> Sized {}
}

// Y = &'a str.
// RPITIT can't capture `'b` even though `'a == 'b`.
// Lifetime comparison is syntactic.
impl<'a: 'b, 'b: 'a> Trait<&'b str, &'a str> for u16 {
    fn test() -> impl use<'b> Sized {}
}

aliemjay avatar Apr 27 '24 17:04 aliemjay

// Y = <A as Iterator>::Item.
// Are we allowed to capture A now that it appears in a projection type?
// I don't think so since projections do not constrain their parameters.
impl<A: Iterator> Trait<A, A::Item> for u8 {
    fn test() -> impl use<A> Sized {}
}

Unlike the Captures<Y> trick you can't write impl use<A::Item> Sized :thinking: And what is meant by capturing A::Item for the impl anyway

It certainly cannot return impl use<A> Sized, otherwise <u8 as Trait<Chars<'b>, char>>::test() would be unsound by allowing 'b to escape.

OTOH returning impl use<> Sized would disallow use of 'a in <u8 as Trait<&'a [u16], &'a u16>>::test().

Basically you have to do refinement unless the capture list is relaxed to accept arbitrary GenericArgs.

kennytm avatar Apr 27 '24 20:04 kennytm

use<..> impl Trait is also more greppable than impl use<..> Trait.

Hmm, do we have impl for<'a> Trait<'a> already?

scottmcm avatar Apr 28 '24 09:04 scottmcm

We do

fmease avatar Apr 28 '24 09:04 fmease

Would it be fair to say that use<...> Type might be a future possibility?

For instance, say you have a type called Flux that changes over time with a method that returns it at a specific Moment. The Moment is a transformation of the original value and requires no references, but to prevent confusion you may want to make a Flux immutable if one of its Moments is live. You can use PhantomData, but this could be a kind of syntax sugar for that:

use std::time::Duration;

struct Flux {
    value: f64,
    rate: f64,
}

struct Moment {
    value: f64,
}

impl Flux {
    fn at<'a>(&'a self, secs: Duration) -> use<'a> Moment {
        Moment {
            value: self.value + self.rate * secs.as_secs_f64(),
        }
    }
}

fn main() {
    let mut flux = Flux {
        value: 0.0,
        rate: 1.0,
    };
    let moment = flux.at(Duration::from_secs(1));
    
    flux.value = 2.0; // <- error: cannot assign to `flux.value` because it is borrowed
    
    println!("{}", moment.value);
}

I imagine use<...> T would be considered a different type from T - maybe one that automatically dereferences to T. Either way, the point is that this could be a future possibility; one that might also favor use before impl.

Edit: Maybe it could just capture parameters like how associated types do, but idk if that works

Yokinman avatar May 02 '24 03:05 Yokinman

@Yokinman that's an interesting idea. It's definitely true that use<T> impl syntax gives us more room to generalize it to further contexts.

nikomatsakis avatar May 02 '24 09:05 nikomatsakis

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

rfcbot avatar May 04 '24 17:05 rfcbot