don't apply temporary lifetime extension rules to non-extended super let

[dianne]

Reference PR: rust-lang/reference#1980

This changes the semantics for super let (and macros implemented in terms of it, such as pin!, format_args!, write!, and println!) as suggested by @theemathas in #145784 (comment), making super let initializers only count as extending expressions when the super let itself is within an extending block. Since super let initializers aren't temporary drop scopes, their temporaries outside of inner temporary scopes are effectively always extended, even when not in extending positions; this only affects two cases as far as I can tell:

• Block tail expressions in Rust 2024. This PR makes f(pin!({ &temp() })) drop temp() at the end of the block in Rust 2024, whereas previously it would live until after the call to f because syntactically the temp() was in an extending position as a result of super let in pin!'s expansion.
• super let nested within a non-extended super let is no longer extended. i.e. a normal let is required to treat super lets as extending (in which case nested super lets will also be extending).

Closes #145784

This is a breaking change. Both static and dynamic semantics are affected. The most likely breakage is for programs to stop compiling, but it's technically possible for drop order to silently change as well (as in #145784). Since this affects stable macros, it probably would need a crater run.

Nominating for discussion alongside #145784: @rustbot label +I-lang-nominated +I-libs-api-nominated

Tracking issue for super let: #139076

[rustbot]

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

[dianne]

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.

[dianne]

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.

[traviscross]

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

[theemathas]

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

Does this PR affect any edition 2021 code?

[dianne]

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.

[traviscross]

Suggested change

	// We have extending borrow expressions within the initializer
	// expression.
	// We have extending borrow expressions within a non-extending
	// expression within the initializer expression.

(Revising my earlier text here.)

[traviscross]

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

[rfcbot]

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.

[traviscross]

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

[dianne]

PRs opened to all 5 affected root crates:

• Adjust formatting macro usage for compatibility with future Rust compiler versions fadaaszhi/ambavia#9
• Adjust formatting macro usage for compatibility with future Rust compiler versions Koyin2005/Kay#1
• Adjust formatting macro usage for compatibility with future Rust compiler versions eyemotif/eyebot2#5
• Adjust formatting macro usage for compatibility with future Rust compiler versions kjuulh/noil#1
• Adjust formatting macro usage for compatibility with future Rust compiler versions musicodeXQQ/updownserv#1

[traviscross]

Not to hurry anyone, exactly -- we're all busy -- but looking at the calendar, we only have a couple more days for this to land before we'll need to ask for a beta backport for Rust 1.91.

Also, it would be convenient if this change could land at the same time as #145882, as the Reference update in rust-lang/reference#1980 assumes both.

Cc @petrochenkov (as reviewer on #145882).

[jackh726]

The change to making the match itself return a bool is nicer. Tests seem pretty well written.

View changes since this review

[jackh726]

r=me for this, whether you want to land this independently or alongside #145882.

[traviscross]

Thanks for the review. Let's go ahead and get this in so we can start with the beta nomination.

@bors r=jackh726,traviscross rollup

[bors]

📌 Commit d5b5a4a has been approved by jackh726,traviscross

It is now in the queue for this repository.

[apiraino]

Beta backport declined as per compiler team on Zulip and discussion here.

@rustbot label -beta-nominated

[kornelski]

(edited: it wasn't stable for as long as I thought)

I agree with the overall change, since the lifetime extension was already inconsistent, but I'm worried this may suddenly break legitimate code, and Rust isn't supposed to be doing that.

[traviscross]

On lang, we are and were aware this is a breaking change (of a behavior that we released in Rust 1.88 for pin! and Rust 1.89 for format_args!). That's why we did a crater run and ensured PRs were submitted to the affected projects.

That's also why we'd prefer to get this change released sooner rather than later. The longer that the other behavior is stable, the more people will fall into writing the thing we're breaking.

What went out in Rust 1.88 and Rust 1.89 amounted to an accidental stabilization. When we do those, we try to take it back, if we can. Even if we might later want to support something like this in the future, such as via @dianne's proposal in #146098, our practice has been to first fix the bug when possible. We then do the design work on the feature and later land it intentionally.

When deciding whether we can take back an accidental stabilization directly, we consider the degree and character of the breakage, among other factors. Here, we judged that as acceptable given the benefit and given that we're taking this back relatively quickly after releasing this bug.

---

Formatting strings don't support conditions, so use of a nested if cond { &format!("") } else { "default" } is justifiable, especially that it avoids another allocation when the else is a non-empty string. I've used this myself, since if { format_args!() } wasn't possible until recently (1.89.0).

Note in particular that the pattern we're breaking here,

println!("{:?}{:?}", (), if true { &format!("") } else { "" });

did not work until Rust 1.89. That code is an error in Rust 1.88 and all prior versions. Further, due to #145880,

println!("{:?}", if true { &format!("") } else { "" }); //~ ERROR

is still an error in Rust 1.89.

[cyqsimon]

Ran into this just now. Initially I feared this is going to be disastrous, but after reading through related posts and realising it only affects very new code (written with 1.89 and 1.90) I feel much better about this.

Perhaps it would help to make these affected versions explicit in the lint help text to make it less spooky (funny that it happens to be this time of year) for the user?

[LevitatingBusinessMan]

Formatting strings don't support conditions, so use of a nested if cond { &format!("") } else { "default" } is justifiable, especially that it avoids another allocation when the else is a non-empty string. I've used this myself, since if { format_args!() } wasn't possible until recently (1.89.0).

Note in particular that the pattern we're breaking here,

println!("{:?}{:?}", (), if true { &format!("") } else { "" });

did not work until Rust 1.89. That code is an error in Rust 1.88 and all prior versions. Further, due to #145880,

println!("{:?}", if true { &format!("") } else { "" }); //~ ERROR

is still an error in Rust 1.89.

I was using this pattern, what would be the proper equivalent?

[theemathas]

@LevitatingBusinessMan

let temp = if true { &format!("") } else { "" };
println!("{:?}", temp);