RFC: Drop temporaries in tail expressions before local variables #3606
Conversation
m-ou-se
added
T-lang
| # Reference-level explanation | ||
|
|
||
| For blocks/bodies/arms whose `{}` tokens come from Rust 2024 code, | ||
| temporaries in the tail expression will be dropped *before* the locals of the block are dropped. |
There was a problem hiding this comment.
iiuc does this imply these drop orders in the following compositions?
| Code | Drop order? |
|---|---|
edition_2021! {
let a = ..;
edition_2021! {
let b = ..;
temp_1().result
}
} |
Current situation
|
edition_2024! {
let c = ..;
edition_2024! {
let d = ..;
temp_2().result
}
} |
This RFC
|
edition_2021! {
let e = ..;
edition_2024! {
let f = ..;
temp_3().result
}
} |
'24 block in '21 function
|
edition_2024! {
let g = ..;
edition_2021! {
let h = ..;
temp_4().result
}
} |
'21 block in '24 function (???)
|
There was a problem hiding this comment.
Correct!
I don't expect to see people mixing editions like that though ^^'. The point is that a block expanded from a macro will behave the way the macro author expected. The edition of whoever wrote the block is used.
|
An example of something great with this is that it makes |
|
Looking at the resulting breakage from this change I would personally love to just land this change by default in all editions instead of only in >=2024, treating it as "clarifying previously underspecified language semantics" (unless we formally stated the existing rules somewhere). This kind of subtle inconsistency between editions is likely to cause some confusion. I am also very much happy to also only have this behavior in the new edition but would like to at least mention this as an alternative. |
|
@rfcbot fcp merge After much exploration, we've decided to break apart the "temporary lifetimes" change into two smaller items. This one corrects the lifetimes in the tail expressions of blocks, a common source of surprising compiler errors. The other item ( Lang team, let's do this! |
|
Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged team members: 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. |
rfcbot
added
proposed-final-comment-period
I could be persuaded here, but I am reluctant to make changes in semantics without an edition, because it is difficult to know what code out there may be affected. |
|
Seems very well scoped and motivated for an edition change. Thanks for the effort that went into design and boiling it down to a simple RFC such as this. @rfcbot reviewed |
|
|
||
| # Reference-level explanation | ||
|
|
||
| For blocks/bodies/arms whose `{}` tokens come from Rust 2024 code, |
There was a problem hiding this comment.
Thanks for including exactly which tokens trigger this behaviour 👍
rfcbot
added
final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
|
I would like to formally register a concerning regarding the effect on unsafe code. The RFC says
However, for unsafe code implicitly relying on the current drop order, it's unclear to me whether that is possible. (AFAIK rfcbot support for t-lang-advisors has not been implemented so if someone on t-lang could tell the bot about this concern that would be great. :) |
|
@RalfJung Your concern was discussed in the lang triage meeting just now. Niko said that we can warn for this situation (and that that such patterns with pointers are error-prone regardless, so we probably want to warn for such patterns also without this change). The lang team consensus in the meeting was, given the expectation that this can be linted for, this doesn't have to be a blocking concern on the RFC. (But instead something to take into account before stabilization, when there is more data.) I will add it to the unresolved questions in the RFC. |
|
This seems like a great change, but I was interested in seeing if any real-world code is broken by this. Where can I find the crater run with this change applied to all editions? |
rfcbot
added
finished-final-comment-period
|
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. |
traviscross
removed
the
I-lang-nominated
The FCP has completed on RFC 3606, so let's prepare it to be merged. First, we'll shorten the name of the feature flag a bit; this should still be unambiguous. Second, we're going to remove the graphic from the summary. While it may be illustrative, the text and the other examples seem clear enough without it, and its benefits have to be weighed against the fact that we want the content in this repository to be easily editable and freestanding. Pulling in an SVG file from an outside host pulls against that. If we come to think the graphic is critical, we could always add it back in a separate PR that would add an editable version of this SVG file into the repository itself. Third, let's make the H1 title of the document a bit more clear.
|
The lang team has accepted this RFC and we've now merged it. Thanks to @m-ou-se and @dingxiangfei2009 for pushing this forward, and thanks to all those who reviewed this RFC and provided helpful feedback. For further updates on this work, follow the tracking issue: |
This is one of the results of the temporary lifetimes effort by @nikomatsakis @dingxiangfei2009 and me.
Originally, we were working on a much larger RFC with several changes, but decided to not block small things on big things.
This part is quite small, but requires an edition change.
Rendered