Support field-wise CoerceShared reborrows#157101
Closed
P8L1 wants to merge 3 commits into
Closed
Conversation
rustbot
added
S-waiting-on-author
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
Contributor
|
🔨 5 commits were squashed into b8e47e8. |
rust-bors
Bot
force-pushed
the
fix-reborrow-coerceshared-field-relations
branch
from
9713677 to
b8e47e8
Compare
This comment has been minimized.
This comment has been minimized.
P8L1
force-pushed
the
fix-reborrow-coerceshared-field-relations
branch
from
b8e47e8 to
83d9f22
Compare
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
|
Unclosed quote in argument. Run |
Contributor
Author
|
@bors squash msg="Support field-wise CoerceShared reborrows" |
This comment has been minimized.
This comment has been minimized.
Contributor
|
🔨 7 commits were squashed into 8987e4a. |
rust-bors
Bot
force-pushed
the
fix-reborrow-coerceshared-field-relations
branch
from
62ec49c to
8987e4a
Compare
Collaborator
rustbot
added
S-waiting-on-review
Collaborator
|
r? @folkertdev rustbot has assigned @folkertdev. Use Why was this reviewer chosen?The reviewer was selected based on:
|
Collaborator
|
|
Member
|
Why are you asking for my review here? I have nothing to do with this feature and don't have capacity to take on a new feature, especially not a massive PR like this. @rustbot reroll |
Contributor
Author
Thanks, that makes sense. I wrote up a PR-specific design note here please review it and share your thoughts so this PR can proceed! |
P8L1
commented
Jun 2, 2026
Contributor
Author
There was a problem hiding this comment.
Borrow-set gathering records MIR Reborrow loans.
This file now treats malformed generic reborrow shapes as delayed compiler bugs instead of hard panics.
P8L1
commented
Jun 2, 2026
Comment on lines
305
to
+330
| } else if let &mir::Rvalue::Reborrow(target, mutability, borrowed_place) = rvalue { | ||
| let borrowed_place_ty = borrowed_place.ty(self.body, self.tcx).ty; | ||
| let &ty::Adt(reborrowed_adt, _reborrowed_args) = borrowed_place_ty.kind() else { | ||
| unreachable!() | ||
| self.tcx.dcx().span_delayed_bug( | ||
| self.body.source_info(location).span, | ||
| format!("generic reborrow source is not an ADT: {borrowed_place_ty:?}"), | ||
| ); | ||
| return; | ||
| }; | ||
| let &ty::Adt(target_adt, assigned_args) = target.kind() else { | ||
| self.tcx.dcx().span_delayed_bug( | ||
| self.body.source_info(location).span, | ||
| format!("generic reborrow target is not an ADT: {target:?}"), | ||
| ); | ||
| return; | ||
| }; | ||
| let &ty::Adt(target_adt, assigned_args) = target.kind() else { unreachable!() }; | ||
| let Some(ty::GenericArgKind::Lifetime(region)) = assigned_args.get(0).map(|r| r.kind()) | ||
| else { | ||
| bug!( | ||
| "hir-typeck passed but {} does not have a lifetime argument", | ||
| if mutability == Mutability::Mut { "Reborrow" } else { "CoerceShared" } | ||
| self.tcx.dcx().span_delayed_bug( | ||
| self.body.source_info(location).span, | ||
| format!( | ||
| "hir-typeck passed but {} does not have a lifetime argument", | ||
| if mutability == Mutability::Mut { "Reborrow" } else { "CoerceShared" } | ||
| ), | ||
| ); | ||
| return; |
Contributor
Author
There was a problem hiding this comment.
This replaces unreachable!/bug! assumptions with delayed bugs and early returns.
The borrow-set visitor can now avoid constructing a loan from invalid generic reborrow MIR.
P8L1
commented
Jun 2, 2026
Contributor
Author
There was a problem hiding this comment.
Borrowck type checking is where generic reborrow MIR becomes region and type constraints.
The main change is replacing ad hoc one-field CoerceShared handling with recursive field-wise constraints.
P8L1
commented
Jun 2, 2026
Comment on lines
+2493
to
+2509
| let ty::Adt(_, dest_args) = dest_ty.kind() else { | ||
| span_mirbug!( | ||
| self, | ||
| borrowed_place, | ||
| "generic reborrow target is not an ADT: {dest_ty:?}" | ||
| ); | ||
| return; | ||
| }; | ||
| let Some(ty::GenericArgKind::Lifetime(dest_region)) = dest_args.get(0).map(|r| r.kind()) | ||
| else { | ||
| span_mirbug!( | ||
| self, | ||
| borrowed_place, | ||
| "generic reborrow target does not have a lifetime argument: {dest_ty:?}" | ||
| ); | ||
| return; | ||
| }; |
Contributor
Author
There was a problem hiding this comment.
This turns shape assumptions on the reborrow target into MIR bugs with early returns.
Borrowck still records liveness for the target lifetime, but only after proving the first generic arg is a lifetime.
P8L1
commented
Jun 2, 2026
Comment on lines
+2527
to
+2543
| // CoerceShared relates distinct ADTs field-wise. Impl validation guarantees that | ||
| // every target field has the corresponding source field and that each field relation | ||
| // is Copy-compatible, recursively CoerceShared-compatible, or `&mut T` to `&T`. | ||
| // The loan itself is still issued for `borrowed_place` as a whole, so source-only | ||
| // fields remain protected for the inferred target lifetime. Under the current | ||
| // single-lifetime impl model, that is the source lifetime. | ||
| if self | ||
| .add_generic_shared_reborrow_constraints( | ||
| location, | ||
| borrowed_place, | ||
| borrowed_ty, | ||
| dest_ty, | ||
| category, | ||
| ) | ||
| .is_err() | ||
| { | ||
| return; |
Contributor
Author
There was a problem hiding this comment.
The previous local name-based loop is replaced by a recursive helper call.
The key invariant is that validation, borrowck, and materialization all consume the same canonical field pairs.
Contributor
|
This PR currently does not correctly handle #![feature(reborrow, decl_macro)]
use std::marker::{CoerceShared, Reborrow};
macro my_macro($field:ident) {
pub struct MyMut<'a> {
$field: &'a i32,
field: &'a i64,
}
#[derive(Clone, Copy)]
pub struct MyRef<'a> {
$field: &'a i32,
field: &'a i64,
}
impl Reborrow for MyMut<'_> {}
impl<'a> CoerceShared<MyRef<'a>> for MyMut<'a> {}
}
my_macro!(field); |
P8L1
commented
Jun 2, 2026
Comment on lines
2546
to
+2564
| // Exclusive reborrow | ||
| self.relate_types( | ||
| if let Err(terr) = self.relate_types( | ||
| borrowed_ty, | ||
| ty::Variance::Covariant, | ||
| dest_ty, | ||
| location.to_locations(), | ||
| category, | ||
| ) | ||
| .unwrap(); | ||
| ) { | ||
| span_mirbug!( | ||
| self, | ||
| borrowed_place, | ||
| "bad generic exclusive reborrow relation ({:?}: {:?}): {:?}", | ||
| borrowed_ty, | ||
| dest_ty, | ||
| terr | ||
| ); | ||
| } | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Exclusive reborrows keep the ordinary same-type relation but now report a MIR bug instead of unwrapping.
This is defensive behavior for malformed MIR and does not change the success path.
| } | ||
| } | ||
|
|
||
| fn add_generic_shared_reborrow_constraints( |
Contributor
Author
There was a problem hiding this comment.
This helper recursively walks the validated CoerceShared field relation.
It normalizes field types in borrowck's inference context before deciding whether to relate leaves or recurse.
Comment on lines
+2583
to
+2616
| if let Err(terr) = self.relate_types_structurally_relating_aliases( | ||
| borrowed_ty.peel_refs(), | ||
| ty::Variance::Covariant, | ||
| dest_ty.peel_refs(), | ||
| location.to_locations(), | ||
| category, | ||
| ) { | ||
| span_mirbug!( | ||
| self, | ||
| borrowed_place, | ||
| "bad generic shared reborrow field relation ({:?}: {:?}): {:?}", | ||
| borrowed_ty, | ||
| dest_ty, | ||
| terr | ||
| ); | ||
| return Err(()); | ||
| } | ||
|
|
||
| self.constraints.outlives_constraints.push(OutlivesConstraint { | ||
| sup: ref_region.as_var(), | ||
| sub: borrow_region.as_var(), | ||
| locations: location.to_locations(), | ||
| span: location.to_locations().span(self.body), | ||
| category, | ||
| variance_info: ty::VarianceDiagInfo::default(), | ||
| from_closure: false, | ||
| }); | ||
| return Ok(()); | ||
| } | ||
|
|
||
| match (borrowed_ty.kind(), dest_ty.kind()) { | ||
| (ty::Adt(borrowed_adt, borrowed_args), ty::Adt(dest_adt, dest_args)) | ||
| if borrowed_adt.did() != dest_adt.did() => | ||
| { |
Contributor
Author
There was a problem hiding this comment.
The &mut T to &T leaf relates pointee types structurally and then emits the target-outlives-source constraint.
This mirrors built-in shared reborrowing while allowing aliases and projections in the pointee type.
Comment on lines
+2617
to
+2656
| let field_pairs = match reborrow::coerce_shared_field_pairs( | ||
| tcx, | ||
| *borrowed_adt, | ||
| *borrowed_args, | ||
| *dest_adt, | ||
| *dest_args, | ||
| ) { | ||
| Ok(field_pairs) => field_pairs, | ||
| Err(CoerceSharedFieldPairError::FieldStyleMismatch) => { | ||
| span_mirbug!( | ||
| self, | ||
| borrowed_place, | ||
| "generic shared reborrow has unsupported field structure: \ | ||
| {borrowed_ty:?} -> {dest_ty:?}" | ||
| ); | ||
| return Err(()); | ||
| } | ||
| Err(CoerceSharedFieldPairError::MissingSourceField { .. }) => { | ||
| span_mirbug!( | ||
| self, | ||
| borrowed_place, | ||
| "generic shared reborrow is missing a source field: \ | ||
| {borrowed_ty:?} -> {dest_ty:?}" | ||
| ); | ||
| return Err(()); | ||
| } | ||
| }; | ||
|
|
||
| for field_pair in field_pairs { | ||
| self.add_generic_shared_reborrow_constraints( | ||
| location, | ||
| borrowed_place, | ||
| field_pair.source.ty, | ||
| field_pair.target.ty, | ||
| category, | ||
| )?; | ||
| } | ||
|
|
||
| Ok(()) | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Distinct ADTs are expanded through canonical CoerceShared field pairs, then each field pair is related recursively.
This is the borrowck side of nested field-wise reborrows such as OuterMut to OuterRef.
Comment on lines
+2657
to
+2677
| _ => { | ||
| if let Err(terr) = self.relate_types_structurally_relating_aliases( | ||
| borrowed_ty, | ||
| ty::Variance::Covariant, | ||
| dest_ty, | ||
| location.to_locations(), | ||
| category, | ||
| ) { | ||
| span_mirbug!( | ||
| self, | ||
| borrowed_place, | ||
| "bad generic shared reborrow field relation ({:?}: {:?}): {:?}", | ||
| borrowed_ty, | ||
| dest_ty, | ||
| terr | ||
| ); | ||
| Err(()) | ||
| } else { | ||
| Ok(()) | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
The fallback relates non-recursive leaves structurally, including aliases, using the same variance as the old field loop.
This covers copied scalar fields and equal field types after normalization.
Contributor
Author
There was a problem hiding this comment.
HIR type checking creates the generic reborrow coercion adjustment before MIR borrowck sees it. This file adds a structural shape guard so only field-corresponding CoerceShared impls produce reborrow MIR.
| }; | ||
| use rustc_middle::ty::error::TypeError; | ||
| use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt, Unnormalized}; | ||
| use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt, Unnormalized, reborrow}; |
Contributor
Author
There was a problem hiding this comment.
HIR typeck imports the shared reborrow helper for a structural shape guard before creating a reborrow adjustment. This keeps obviously malformed CoerceShared shapes out of MIR.
| } | ||
|
|
||
| /// Applies generic exclusive reborrowing on type implementing `Reborrow`. | ||
| /// Applies generic shared reborrowing on types implementing `CoerceShared`. |
Contributor
Author
There was a problem hiding this comment.
The method comment is corrected to describe CoerceShared shared reborrowing, matching the code path below. The new shape guard rejects non-structs, missing lifetimes, and missing field correspondence before registering the trait obligation.
Comment on lines
+1008
to
+1012
| // This is only a shape guard for producing well-formed MIR adjustments. The field-type | ||
| // relation remains owned by the `CoerceShared` obligation and builtin impl validation. | ||
| if !self.coerce_shared_reborrow_structurally_well_formed(a, b) { | ||
| return Err(TypeError::Mismatch); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
This helper checks only ADT struct shape, exactly-one-lifetime presence, and successful canonical field pairing. It does not inspect per-field type compatibility beyond what the pair helper needs for field metadata.
| } | ||
| } | ||
|
|
||
| fn coerce_shared_reborrow_structurally_well_formed(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> bool { |
Contributor
Author
There was a problem hiding this comment.
This helper is the HIR typeck shape gate for producing ExprKind::Reborrow adjustments. It accepts only struct ADTs with one lifetime argument each and successful canonical field pairing.
Contributor
Author
Ok im going to fix this now then finish comments |
P8L1
commented
Jun 2, 2026
Contributor
Author
There was a problem hiding this comment.
This new helper module is the canonical home for Reborrow and CoerceShared field metadata: it extracts participating ADT fields, preserves projection indices and spans, filters PhantomData, and computes source-to-target field pairs.
| use crate::ty::{self, Ty, TyCtxt}; | ||
|
|
||
| #[derive(Clone, Copy, Debug)] | ||
| pub struct ReborrowField<'tcx> { |
Contributor
Author
There was a problem hiding this comment.
This struct is the data contract for one participating field. index preserves the real ADT field index for MIR projection, ident is needed for hygiene-aware named-field matching, name is kept for diagnostics, ty is the instantiated field type, and span lets validation point at the field that caused the problem.
The split between ident and name matters: name is useful for user-facing output, but matching named fields must not be reduced to raw textual-name equality.
| } | ||
|
|
||
| #[derive(Clone, Copy, Debug)] | ||
| pub struct CoerceSharedFieldPair<'tcx> { |
Contributor
Author
There was a problem hiding this comment.
CoerceSharedFieldPair is the normalized relation later compiler phases operate on: each target field has a specific source field. The error enum keeps correspondence failures field-local, distinguishing mismatched struct styles from a target field that has no valid source-side data field.
| } | ||
|
|
||
| /// Returns the instantiated non-`PhantomData` fields that participate in generic reborrows. | ||
| pub fn reborrow_data_fields<'tcx>( |
Contributor
Author
There was a problem hiding this comment.
This function extracts the instantiated non-PhantomData fields that actually participate in field-wise reborrowing. It deliberately preserves the original FieldIdx, because matching ignores marker fields but MIR / CTFE / codegen still need to project from the real ADT layout.
Comment on lines
+74
to
+78
| if source_variant.ctor_kind() != target_variant.ctor_kind() { | ||
| return Err(CoerceSharedFieldPairError::FieldStyleMismatch); | ||
| } | ||
|
|
||
| let by_position = matches!(target_variant.ctor_kind(), Some(CtorKind::Fn)); |
Contributor
Author
There was a problem hiding this comment.
This is the boundary between named-field and tuple-field correspondence. CoerceShared does not allow named structs and tuple structs to be matched against each other, because the meaning of “corresponding field” is different in the two styles.
| let target_fields = reborrow_data_fields(tcx, target_def, target_args); | ||
|
|
||
| if by_position { | ||
| target_fields |
Contributor
Author
There was a problem hiding this comment.
Tuple structs match by filtered data-field ordinal, not by original field index. This lets PhantomData be ignored for correspondence while still preserving each field’s original FieldIdx inside the returned pair for later projection.
| if by_position { | ||
| target_fields | ||
| .into_iter() | ||
| .zip(source_fields.into_iter().map(Some).chain(std::iter::repeat(None))) |
Contributor
Author
There was a problem hiding this comment.
The repeat(None) tail makes the target-driven failure mode explicit: every target data field must have a corresponding source data field. Extra source fields do not matter for constructing the shared target view, but missing source fields are rejected.
Comment on lines
+99
to
+101
| .find(|source| { | ||
| tcx.hygienic_eq(target.ident, source.ident, source_variant.def_id) | ||
| }) |
Contributor
Author
There was a problem hiding this comment.
This is the hygiene-sensitive part of named-field correspondence. Two fields can print with the same textual name under decl_macro while still being distinct identifiers, so this must use rustc’s hygienic field identity rather than raw Symbol equality. (The decl macro hygiene bug was caused by me just comparing names)
| tcx.hygienic_eq(target.ident, source.ident, source_variant.def_id) | ||
| }) | ||
| .ok_or(CoerceSharedFieldPairError::MissingSourceField { target })?; | ||
|
|
Contributor
Author
There was a problem hiding this comment.
The named-field branch is target-driven: each non-PhantomData target field must resolve to a hygienically corresponding source field. If no source field matches, the helper reports the target field as missing instead of silently omitting it or falling back to position.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Contributor
Author
|
Ok I would like to start off by saying sorry for all the pings. I have recently received valid criticism about the reviewability of this PR. Thus, I took my self-review notes, summarised them and posted them as review comments here. This was done to make it easier for reviewers to wrap their heads around the changes. I have also updated the Design Note to reflect the new decl_macro hygiene changes |
Contributor
Author
Thanks, both of these were bugs in the impl validation rather than intentional behavior. I pushed a fix in Validate CoerceShared field relations after normalization. CoerceShared field validation now structurally normalizes the field types before accepting equality or the built-in That fixes the associated-type case because |
Contributor
Author
|
Once again, I updated the design note to reflect the new "Validate CoerceShared field relations after normalization" changes |
P8L1
commented
Jun 2, 2026
| if !errors.is_empty() { Err(errors) } else { Ok(()) } | ||
| } | ||
|
|
||
| pub(crate) fn coerce_shared_info<'tcx>( |
Contributor
Author
There was a problem hiding this comment.
Validates a builtin CoerceShared impl. Source and target shapes that later reborrow phases cannot interpret are rejected here. Field-type legality is delegated to canonical field-pair validation below.
|
|
||
| // FIXME(#155345): This should return `Unnormalized` | ||
| fn collect_struct_data_fields<'tcx>( | ||
| fn validate_reborrow_field_access( |
Contributor
Author
There was a problem hiding this comment.
Rejects fields that later reborrow lowering cannot legally project. Field-wise validation must be able to name every participating field, so private, otherwise inaccessible, and non-exhaustive fields are rejected before canonical field-pair validation.
| Ok(()) | ||
| } | ||
|
|
||
| fn validate_coerce_shared_fields<'tcx>( |
Contributor
Author
There was a problem hiding this comment.
Builds and validates canonical CoerceShared source-to-target field pairs. Pair construction defines the correspondence used by validation and later phases: named fields use hygienic identity, while tuple fields use the ordinal among non-PhantomData data fields.
| Ok(()) | ||
| } | ||
|
|
||
| fn validate_coerce_shared_field<'tcx>( |
Contributor
Author
There was a problem hiding this comment.
Validates one corresponding CoerceShared field relation. The accepted cases are deliberately narrow: a builtin &mut T to &T shared-reference leaf, a copy-compatible equal field type, or a recursive SourceField: CoerceShared<TargetField> obligation. Successful paths must discharge generated obligations and region constraints before accepting.
| ocx.resolve_regions_and_report_errors(impl_did, param_env, []) | ||
| } | ||
|
|
||
| enum FieldRelation { |
Contributor
Author
There was a problem hiding this comment.
Field relation to prove after normalization in the impl-validation context. This keeps equality separate from the builtin shared-reference leaf. The MutRefToSharedRef case is not type equality: it normalizes both fields, strips exactly one &mut/& layer, and records the required lifetime constraint before relating the referents.
| } | ||
|
|
||
| enum FieldRelation { | ||
| Equal, |
Contributor
Author
There was a problem hiding this comment.
Require normalized source and target field types to be equal.
|
|
||
| enum FieldRelation { | ||
| Equal, | ||
| MutRefToSharedRef, |
Contributor
Author
There was a problem hiding this comment.
Require &mut T to relate to &T after adding the lifetime constraint.
| MutRefToSharedRef, | ||
| } | ||
|
|
||
| fn field_tys_satisfy_relation_after_normalization_and_resolution<'tcx>( |
Contributor
Author
There was a problem hiding this comment.
Normalizes, relates, and resolves a field-type relation as a single check. Alias and projection types are structurally normalized in the impl-validation context. Generated obligations are evaluated with ambiguity as failure, the normalized types are structurally related, and region constraints must resolve before this returns true.
| && ocx.resolve_regions(impl_did, param_env, []).is_empty() | ||
| } | ||
|
|
||
| fn assert_field_type_is_copy<'tcx>( |
Contributor
Author
There was a problem hiding this comment.
Verifies the copy-compatible leaf case after field equality is established. The equal-type field path may only accept a leaf when that field type is Copy, and the generated copy obligations and regions must also resolve.
Contributor
|
The following code causes an ICE with this PR: #![feature(reborrow)]
use std::marker::{CoerceShared, Reborrow};
pub trait Trait {
type Assoc;
}
impl<T> Trait for T {
type Assoc = T;
}
#[expect(dead_code)]
pub struct MyMut<'a, T>(&'a (), T);
#[expect(dead_code)]
#[derive(Copy, Clone)]
struct MyRef<'a, T>(&'a (), <T as Trait>::Assoc);
impl<T: Copy> Reborrow for MyMut<'_, T> {}
impl<'a, T: Copy> CoerceShared<MyRef<'a, T>> for MyMut<'a, T> {}
pub fn foo<T: Trait + Copy>(x: MyMut<'_, T>) {
let _: MyRef<'_, T> = x;
}Error outputI think that rust was unable to figure out that |
aapoalas
requested changes
Jun 4, 2026
Contributor
There was a problem hiding this comment.
Hey, first: thank you for the work you've done.
However, after some discussion with Xiang we've come to the conclusion that unfortunately this direction this PR is walking us is indeed a bit wrong. Here's what we'd want to do:
- Perform all well-formedness checking in the
coherence/builtin.rs: there shouldn't be any need to perform any checks relating to the correctness of the traits or field types in THIR building, MIR building, or the borrow checker. - Similarly to now, the THIR will emit a single reborrow adjustment that is used in borrow checking.
- An MIR pass will be added that lowers the single Rvalue::Reborrow into field-wise actions. This goes into
fn run_runtime_lowering_passes.
This gets us "both flavours of reborrowing": simple reborrowing in the MIR for borrow checking, and field-by-field reborrowing in the lowered MIR for code generation. Actual code generation backends shouldn't have to do all that much work for reborrowing to work at this point.
If you want to help, here's how:
- The tests you're adding here are good; adding them in a separate PR would be great. It's okay if some or all of them produce ICEs in that PR, there's a
tests/ui/crashesfolder for that and we can add in atests/ui/crashes/reborrowfolder specifically for those. - The work you've done on the well-formedness check (
coherence/builtin.rs) deserves a separate PR. We can then build on top of that to improve the checking to get to the desired state of the well-formedness check containing all the check logic. - A distant third is the MIR pass itself: this we'll be very deeply involved in so collaborating may be harder, but as we get into building the pass I can ping you or send an email and let you know if there is anything in particular you could help with.
On the communication front, I recommend using your own voice and words in the future: you're doing yourself a disfavour by using an LLM to translate and generate text for you. You do not need to communicate in perfect English - it's perfectly okay to communicate at the level that you're capable of, and nothing more or less.
Cheers.
rustbot
added
S-waiting-on-author
Collaborator
|
Reminder, once the PR becomes ready for a review, use |
aapoalas
reviewed
Jun 4, 2026
Contributor
There was a problem hiding this comment.
note: Seeing this file change is a sign that this PR puts us on the wrong path: there shouldn't really be any codegen-specific handling of reborrowing, or if there is then it should just be an extra way to generate a copy.
aapoalas
reviewed
Jun 4, 2026
| let ty::Adt(dest_adt, dest_args) = dest_ty.kind() else { bug!() }; | ||
| let [dest_arg, ..] = ***dest_args else { bug!() }; | ||
| let ty::GenericArgKind::Lifetime(dest_region) = dest_arg.kind() else { bug!() }; | ||
| let ty::Adt(_, dest_args) = dest_ty.kind() else { |
Contributor
There was a problem hiding this comment.
note: These changes are questionable. These paths should be unreachable, so why are we adding extra dead code?
This was referenced Jun 5, 2026
Contributor
Author
Hi, thanks so much. I appreciate it! I am unfortunately a bit caught up with work at the moment. Thus, I dont think I will be doing more work soon. I will email you if I get a breather and can resume work on Reborrow. I also think that the above approach sounds great. I have split this PR into #157490 and #157489 I am, however, uncertain if it is the exact split you want. Just say if I have to remove things |
rustbot
removed
the
S-waiting-on-author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
View all comments
This PR extends generic shared reborrows so that
CoerceSharedvalidates and lowers source-to-target structs field by field, rather than relying on a same-layout, transmute-like copy.The core change is a shared
rustc_middle::ty::reborrowhelper that computes the field correspondence used byCoerceSharedvalidation, borrow checking, const-eval, and codegen.The correspondence rules are:
PhantomDatadata fields by name.PhantomDatadata fields by position.PhantomDatafields are ignored.CoerceSharedimpl validation now checks each corresponding field relation. A field relation is accepted when the field is:CoerceShared-compatible, orThe validation also rejects mismatched field styles, missing source fields, mismatched reborrow lifetimes, and inaccessible fields with targeted diagnostics.
Borrowck now adds shared generic reborrow constraints recursively through the validated field relations, while still issuing the loan for the original source place as a whole. This ensures source-only fields remain protected for the inferred target lifetime when the target omits fields from the source.
Const-eval and codegen now lower shared generic reborrows recursively into the target fields instead of treating the operation as a transmute-like copy. This covers nested
CoerceSharedfields and layout-changing source/target pairs.Fixes
Fixes #156566.
Fixes #156309.
Fixes #156315.
PR scope
I did not split these into smaller PRs because the issues mostly share the same root cause:
CoerceSharedneeded a field-by-field validation and lowering model. So when making this, I just decided to write it in a way that will fix the issues anyway.Tracking
#145612
https://rust-lang.github.io/rust-project-goals/2025h2/autoreborrow-traits.html
CC. @aapoalas, @dingxiangfei2009
r? @RalfJung