Fix const-eval of shared generic reborrows

[P8L1]

Rvalue::Reborrow promotion validation now applies the same reference-deref simplification as Rvalue::Ref, so generic reborrows are validated as reference-like copies during promotion.

This also fixes evaluation of shared generic reborrows produced by CoerceShared. Those reborrows intentionally copy from the source ADT into a distinct same-layout target ADT, so the interpreter must use the transmute-capable copy path for Mutability::Not.

Fixes #156313.
cc @aapoalas
Tracking: #145612
@rustbot label F-reborrow

[rustbot]

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

[rustbot]

r? @mejrs

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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler, mir, mir-opt
• compiler, mir, mir-opt expanded to 73 candidates
• Random selection from 18 candidates

[aapoalas]

r? @dingxiangfei2009

[RalfJung]

What does the MIR validator require for Reborrow expressions?

View changes since the review

[P8L1]

I checked the MIR validator. It does not currently have a dedicated Reborrow validity rule: rustc_mir_transform::validate leaves Rvalue::Ref(..) | Rvalue::Reborrow(..) alone in visit_rvalue, and the generic Assign check only verifies that rvalue.ty() can be assigned to the destination. For Reborrow, rvalue.ty() is the stored target type. The validator does not separately require the source place type to equal the destination type.

The stricter contract is elsewhere: the MIR comment says Mut reborrow copies the same ADT, while shared CoerceShared copies into the target ADT, which is currently required to be Copy and same-layout. Borrowck also assumes that shape: mutable reborrow is same ADT, shared CoerceShared is a distinct ADT, and borrowck relates the relevant fields/lifetimes.

So the use of copy_op_allow_transmute here is not because MIR validation permits arbitrary transmute-like Reborrow. It is because shared CoerceShared intentionally has different source and target ADT types, and CTFE's normal copy_op rejects that through the assignment-type compatibility check even though the current Reborrow contract says this is a same-layout bitwise copy.

[RalfJung]

I am kind of surprised that mutable and shared borrows behave so differently. But sure, we can do the matching thing in const-eval.

[RalfJung]

The fact that you have to touch this file makes me very concerned.
Why do custom reborrows have any interaction with promotion? Promotion is a mess, adding any new cases to it is a mistake in its current state IMO. I'd strongly prefer if we did not allow promotion of anything that involves user-defined borrows. Only built-in references should get promoted.

View changes since the review

[P8L1]

I checked the promotion code. This change does not add Reborrow as a promotion root: the collector still only pushes candidates for Rvalue::Ref, and promote_candidate still assumes the candidate statement is Rvalue::Ref. The change is in validation of temps that are already underneath one of those existing built-in reference promotion candidates.

So the interaction is narrower than "promotion now promotes user-defined borrows" in the candidate-collection sense. The concrete issue is that promotion already has a special case for built-in reborrows like &*r by validating the reference local instead of the dereferenced place; a temp produced via generic Reborrow can appear under that existing Rvalue::Ref path in the const-eval reproducer, and the old code treated it like a plain Copy(place).

[P8L1]

I think the remaining question is whether that narrower interaction is still too much for promotion.

The current PR does not make Rvalue::Reborrow itself a promotion candidate, but it does let promotion validation accept a temp produced by generic reborrow when that temp is underneath an already-collected Rvalue::Ref candidate.

Would you prefer that promotion reject any candidate whose temp graph contains a user-defined Rvalue::Reborrow, even when the promotion root is still a built-in Rvalue::Ref? If so, I can rework this so the CTFE/interpreter fix stays separate from promotion eligibility.

[RalfJung]

I don't think we should promote anything that contains a Reborrow anywhere. We certainly must not promote anything that could possibly invoke arbitrary user-defined code, that would be plain unsound.

[P8L1]

Got it, that answers the boundary question.

I’ll rework this so promotion rejects candidates that contain Rvalue::Reborrow anywhere in the promoted temp graph, rather than teaching promotion to validate those cases. Then the promotion side keeps the stricter rule that only built-in reference cases are promotable, while the CTFE/interpreter change remains limited to evaluating Reborrow MIR that was already produced through the normal MIR pipeline.

I’ll update the test accordingly so it no longer relies on promotion accepting a generic reborrow.