1

RFC: Drop temporaries in tail expressions before local variables by m-ou-se · Pu...

 4 weeks ago
source link: https://github.com/rust-lang/rfcs/pull/3606
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.

RFC: Drop temporaries in tail expressions before local variables #3606

Conversation

Member

@m-ou-se m-ou-se

commented

Apr 2, 2024

•

edited by rustbot

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

davidhewitt, burdges, dev-ardi, PatchMixolydic, kalkronline, kennytm, Aloso, zachs18, samanpa, solson, and 9 more reacted with thumbs up emojia1phyr, Veykril, bjorn3, scottmcm, jieyouxu, GrayJack, marziply, Yokinman, astrale-sharp, and felix91gr reacted with heart emojiJules-Bertholet, Veykril, and GrayJack reacted with rocket emojishepmaster and GrayJack reacted with eyes emoji

m-ou-se

added T-lang Relevant to the language team, which will review and decide on the RFC. A-drop Proposals relating to the Drop trait or drop semantics A-ergonomics

I-lang-nominated Indicates that an issue has been nominated for prioritizing at the next lang team meeting. A-edition-2024 Area: The 2024 edition

labels

Apr 2, 2024

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

iiuc does this imply these drop orders in the following compositions?

CodeDrop order?
edition_2021! {
    let a = ..;
    edition_2021! {
        let b = ..;
        temp_1().result
    }
}

Current situation

  1. temp_1()
edition_2024! {
    let c = ..;
    edition_2024! {
        let d = ..;
        temp_2().result
    }
}

This RFC

  1. temp_2()
edition_2021! {
    let e = ..;
    edition_2024! {
        let f = ..;
        temp_3().result
    }
}

'24 block in '21 function

  1. temp_3()
edition_2024! {
    let g = ..;
    edition_2021! {
        let h = ..;
        temp_4().result
    }
}

'21 block in '24 function (???)

  1. temp_4()

Member

Author

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 tracing's spans more accurate and less surprising !
m-ou-se, kennytm, GrayJack, and matthieu-m reacted with thumbs up emojim-ou-se, GrayJack, and teor2345 reacted with hooray emoji

Contributor

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.
kennytm, dlight, GrayJack, and matthieu-m reacted with heart emoji

Contributor

@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 (super let) will be pursued in a separate RFC. We have decided NOT to propose a change to temporary lifetimes in match expressions — which was predicted to affect a lot more existing code — and are exploring alternatives (e.g., starting with a lint to help detect the buggy cases).

Lang team, let's do this!
m-ou-se reacted with heart emoji

Collaborator

rfcbot

commented

Apr 4, 2024

•

edited by scottmcm

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.
See this document for info about what commands tagged team members can give me.

rfcbot

added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it.

labels

Apr 4, 2024

Contributor

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 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.
m-ou-se reacted with thumbs up emoji

Member

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
m-ou-se reacted with heart emoji

# Reference-level explanation

For blocks/bodies/arms whose `{}` tokens come from Rust 2024 code,

Thanks for including exactly which tokens trigger this behaviour 👍

m-ou-se reacted with heart emoji

rfcbot

added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised.

and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period.

labels

Apr 4, 2024

Collaborator

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

Member

I would like to formally register a concerning regarding the effect on unsafe code. The RFC says

There's a very small chance this breaks existing code in a very subtle way. However, we can detect these cases and issue warnings.

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

Member

Author

@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.
kennytm and GrayJack reacted with eyes emoji

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 is finished for this RFC. to-announce

and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised.

labels

Apr 14, 2024

Collaborator

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.
marziply and bjorn3 reacted with hooray emojim-ou-se reacted with rocket emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

kennytm

kennytm left review comments

RalfJung

RalfJung left review comments

clarfonthey

clarfonthey left review comments

scottmcm

scottmcm left review comments

nikomatsakis

nikomatsakis approved these changes
Assignees

No one assigned

Labels

A-drop Proposals relating to the Drop trait or drop semantics A-edition-2024 Area: The 2024 edition A-ergonomics disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. I-lang-nominated Indicates that an issue has been nominated for prioritizing at the next lang team meeting. T-lang Relevant to the language team, which will review and decide on the RFC. to-announce

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

None yet

11 participants

report

Recommend

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK