r? @jackh726

rustbot has assigned @jackh726.
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

Copied from #145784 (comment), since I think this is a notable caveat of this PR and worth considering before approving it:

This comes with a bit of a gotcha in terms of temporary lifetimes: it might be strange that the temp() would live longer in

non_extending({ let x = { &temp() }; f(x) }); // ok

than in

non_extending({ super let x = { &temp() }; f(x) }); // error

Though the case for if let is similar: its scrutinee isn't a temporary scope and it doesn't have lifetime extension rules that can make block tail expressions' temporaries live longer.

Also copied since it motivates this PR: I think something like this may be necessary for the identity

&EXPR === { super let x = &EXPR; x }

to hold in both extending and non-extending contexts without changing how block tail expression scopes work. Substituting, e.g. { &temp() } in for EXPR, the identity only currently holds in extending contexts in Rust 2024. In non-extending contexts,

&{ &temp() }

drops temp() when leaving the block, but

{ super let x = &{ &temp() }; x }

would extend it to outlive x. This PR shortens the lifetime of temp() such that it's dropped at the end of the block in both cases. I haven't done any rigorous proof or extensive testing that this PR together with #145342 makes the identity always hold, however.

To confirm, with this PR, does this behavior hold (in Rust 2024)?:

fn f<T>(_: LogDrop<'_>, x: T) -> T { x }

// These two should be the same.
assert_drop_order(1..=3, |e| {
    let _v = f(e.log(2), &{ &raw const *&e.log(1) });
    drop(e.log(3));
});
assert_drop_order(1..=3, |e| {
    let _v = f(e.log(2), {
        super let v = &{ &raw const *&e.log(1) };
        v
    });
    drop(e.log(3));
});
// These two should be the same.
assert_drop_order(1..=3, |e| {
    let _v = f(e.log(1), &&raw const *&e.log(2));
    drop(e.log(3));
});
assert_drop_order(1..=3, |e| {
    let _v = f(e.log(1), {
        super let v = &&raw const *&e.log(2);
        v
    });
    drop(e.log(3));
});
// These two should be the same.
assert_drop_order(1..=2, |e| {
    let _v = &{ &raw const *&e.log(2) };
    drop(e.log(1));
});
assert_drop_order(1..=2, |e| {
    let _v = {
        super let v = &{ &raw const *&e.log(2) };
        v
    };
    drop(e.log(1));
});

Playground link

(If any of these are missing, please add them as tests.)

super let nested within a non-extended super let is no longer extended

Does this PR affect any edition 2021 code?

To confirm, with this PR, does this behavior hold (in Rust 2024)?:

Those all hold under this PR, yes. I've added them all as tests (with some additional versioning to account for the Edition-dependent drop order in the first one); thanks!

super let nested within a non-extended super let is no longer extended

Does this PR affect any edition 2021 code?

Not that I'm aware of. That detail matters in Edition 2024, since the nested super let could have an extending borrow operator within a block tail expression in its initializer, which would previously extend the borrowed temporary's lifetime to outlive the outer super let's binding. In Edition 2021, that temporary outlives the outer super let's binding regardless, since block tail expressions (and super let initializers) aren't temporary scopes.

This is correct, I believe. Let's propose to do it.

@rfcbot fcp merge

@dianne, let's document this in the Reference.

cc @rust-lang/lang-docs

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

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

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.

cc @m-ou-se @rust-lang/libs-api

I've reviewed 13 of the test failures in #145838 (comment) (top-to-bottom from Arxoto.sudoku.78ccf4297d22f37e62e8bf07ff19f7f2e7843139 to LegendarySaiyan.structures.f375d1f242c501951f7a3a6c49a09a7895f57326). All of them are flaky tests. Made a crater PR.

Saethlin told me that I should wait for the retry before checking the test failures.

🚧 Experiment pr-145838-1 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

🎉 Experiment pr-145838-1 is completed!
📊 697 regressed and 670 fixed (187804 total)
📊 580 spurious results on the retry-regessed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

At a glance, the "build compiler error" failures look real, following the pattern of using &format!(...) in a block of a conditional expression inside a format argument (alongside at least one other format argument). The rest I've peeked at look spurious. The "fixed" crates are almost certainly all noise, so I'm expecting a comparable number of spurious regressions. I'll try and review them more comprehensively later.

In the mean time, I've implemented my lifetime extension suggestion for block tails. It's not ready for review yet, but it lives at #146098 now for testing/discussion purposes.

Retrying since there's a lot of "cannot find directory"s.
@craterbot run mode=build-and-test crates=https://crater-reports.s3.amazonaws.com/pr-145838-1/retry-regressed-list.txt p=1

👌 Experiment pr-145838-2 created and queued.
🤖 Automatically detected try build b83b707
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

🚧 Experiment pr-145838-2 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

🎉 Experiment pr-145838-2 is completed!
📊 22 regressed and 18 fixed (1276 total)
📊 65 spurious results on the retry-regessed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

22 isn't too bad to sift through. I'll write up a list of what I find. As before, the build failures look real and the test failures I've seen so far look spurious.

Build failures (all real, same crates as in #145838 (comment), though the last failure there is missing):

• Random-Scientist.capuronii.9ac025e6ccfa700827de34ecf440a831e73c39eb: &format! in if branch in format! in a dependency: https://github.com/fadaaszhi/ambavia/blob/8f9b16a4cf3d44dba0764dedcf45e9fab4987869/parse/src/op.rs#L489-L501. The &format! in the block tail is still there in the current revision.
• https://github.com/Koyin2005/Kay/blob/09a84dc055c4b2152dba677aefd56855df69d6e7/src/types/format.rs#L54-L65: &format! in an if branch in format!. Crate has both a lib.rs and a main.rs.
• https://github.com/eyemotif/eyebot2/blob/5a3ad2f2854267603f09f421c32014859e956931/src/main.rs#L190-L199 &format! in an if branch in println!. Crate only has a main.rs.
• https://github.com/kjuulh/noil/blob/1b2a35509cb21429daaad7ef75bc8f2727a18d23/crates/noil/src/output.rs#L41-L63: &format! in an if branch in write!. Crate only has a main.rs.

For completeness, the last build failure from the first crater run:

• https://github.com/musicodeXQQ/updownserv/blob/2d89554ea1cb5a26205731b37f79b6466e105232/src/main.rs#L625: &format! in an if branch in format!. Crate only has a main.rs.

Test failures (all spurious):

• MathiasBjerggaard.3DCylinderSimulation.f30e72994f531da2b60879bb3c4344f07f1beac6 (log): nondeterministic (uses rand::thread_rng()).
• anatols.atomic_sibling_counter.aa389339e1557668b80609cc1246765d27c32aea (log): likely nondeterministic (parallel tests using shared mutable global state). The failing test reads the number of bytes allocated by the global allocator after allocating something, but it's lower than before. If I had to guess, another test's thread deallocated something at an unlucky time.
• aubex.pytron.89e241ae9b2cfefc4ceb5b5b0f99e7f349710f33 (log): nondeterministic (parallel tests). The failing test is marked #[serial], but it uses the same environment variable as other tests without a #[serial] or #[parallel] attribute, so one of them must have clobbered it.
• conradludgate.plunger.6d0237a3d6249e723033c49fa13a49bae21dd846 (log): nondeterministic (timing async).
• marziply.emotech-grpc.3bb9a8b87cd48366b7c5d401d09c231af41d37ff (log): tests all try to set up a server listening to the same address; fails when tests run concurrently.
• olegfomenko.valida-examples.36815fc5dd80b0bf715bd24103c576aa858e692d (log): I don't know what's going on here, but the test also fails without this PR. I can't get it to pass locally; it seems to require a custom toolchain and a specific target?
• deadpool-0.12.2 (outdated) (log): nondeterministic (timing). Timing-dependence is still present in the current version.
• exclave-0.2.6 (log): maybe synchronization issues or faulty IO? https://docs.rs/crate/exclave/0.2.6/source/src/units/test.rs#316-330 reads from a mutex before the expected result is written to it. It looks unrelated to this PR. The most recent version of the crate is from 2019, so it shouldn't be affected unless libraries it depends on have updated their old versions to Rust 2024 and one of them does something where runtime drop order is critical to correctness inside a super let-based macro (very unlikely).
• fides-4.3.5 (log) nondeterministic, fails without this PR. Likely relies on nondeterministic ordering/hashes.
• kaiser-0.1.3 (log): unsafe precondition(s) violated: slice::from_raw_parts requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`. Old crate; looks unrelated.
• music-player-settings-0.1.6 (log): nondeterministic (parallel tests). A couple tests use the same environment variable; one must have set it when the other expected it to be unset.
• nextera_jwt-0.1.10 (log) nondeterministic (parallel tests). This time, the failing test expects an environment variable to be set that it doesn't set itself; it's set by a different test though, so maybe it relies on that test setting it to pass?
• nu_plugin_ws-1.0.5 (log): probably nondeterministic (timing), judging by the r#"echo "test" | ws "{}" --max-time 2sec"#. The crate's Rust 2021, also.
• quanta-0.12.6 (log): nondeterministic (timing).
• rskin-1.5.0 (log): depends on filesystem layout, or at least that's where it errors for me when I try to reproduce it locally (and on the same two tests). It's Rust 2021 also.
• scirs2-autograd-0.1.0-alpha.6 (log): probably nondeterministic (parallel tests), given the failing test depending on global mutable state shared with other tests. I can't tell at a glance whether that's what's causing the failure, but it definitely makes the test unreliable. I'm pretty sure this is unrelated anyway, too. There's no reason that an automatic differentiation library would rely on the runtime drop order of super let, it's in Rust 2021 so the drop order wouldn't be affected anyway, and there's a comment on the test itself saying something's wrong.
• single_executor-0.4.1 (log): nondeterministic (timing).
• static-regular-grammar-2.0.2 (log): maybe flaky infra? The test runs just fine for me under a toolchain based on a stage 1 build of this PR, but github CI seems to have failed for the latest commit.

In case anyone else is wondering about the disappearance of https://github.com/musicodeXQQ/updownserv.
In creater run pr-145838-1 it ended up in the error category (master log, try log), resulting in it not being included on the retry-regressed-list.txt.

Beta backport declined as per compiler team on Zulip, we are pretty close to the new release and this patch wouldn't have time to "bake" properly.

@rustbot label -beta-nominated