Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Place tail expression behind terminating scope #125293

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dingxiangfei2009
Copy link
Contributor

This PR implements #123739 so that we can do further experiments in nightly.

A little rewrite has been applied to for await lowering. It was previously unsafe { Pin::unchecked_new(into_async_iter(..)) }. Under the edition 2024 rule, however, into_async_iter gets dropped at the end of the unsafe block. This presumably the first Edition 2024 migration rule goes by hoisting into_async_iter(..) into match one level above, so it now looks like the following.

match into_async_iter($iter_expr) {
  ref mut iter => match unsafe { Pin::unchecked_new(iter) } {
    ...
  }
}

@rustbot
Copy link
Collaborator

rustbot commented May 19, 2024

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 19, 2024
@dingxiangfei2009
Copy link
Contributor Author

dingxiangfei2009 commented May 19, 2024

cc @traviscross for edition 2024 update

cc @eholk for context who has worked on async iterator

@dingxiangfei2009 dingxiangfei2009 changed the title Edition 2024: place tail expression behind terminating scope Place tail expression behind terminating scope May 19, 2024
@petrochenkov
Copy link
Contributor

r? compiler

@rustbot rustbot assigned davidtwco and unassigned petrochenkov May 22, 2024
@bors

This comment was marked as resolved.


fn main() {
let _ = f();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many tests should be added. Such as

  1. code will still panic, but leave the mutex unpoisoned
fn f(m: &Mutex<i32>) -> i32 {
    let _x = PanicOnDrop;
    *m.lock().unwrap()
}
  1. mutable reference
fn main() {
    let c = std::cell::RefCell::new(123);

    if let Ok(mut b) = c.try_borrow_mut() {
        *b = 321;
    }
}
  1. Explicit drop order
    https://github.com/rust-lang/rfcs/pull/3606/files#r1548272644

  2. injects a borrow to a temporary into an already existing local variable that borrows it on drop

fn why_would_you_do_this() -> bool {
    let mut x = None;
    // Make a temporary `RefCell` and put a `Ref` that borrows it in `x`.
    x.replace(RefCell::new(123).borrow()).is_some()
}
  1. block could be a subexpression of a larger expression
    let zero = { String::new().as_str() }.len();

Considering this is a breaking feature, more tests would be better.

@dingxiangfei2009 dingxiangfei2009 force-pushed the tail-expr-temp-lifetime branch 2 times, most recently from 17a40e2 to 75d038e Compare June 1, 2024 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants