Skip to content

Support field-wise CoerceShared reborrows #157101

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.

Sign up for GitHub

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

Closed
P8L1 wants to merge 3 commits into from
Closed

Support field-wise CoerceShared reborrows #157101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8987e4a
Support field-wise CoerceShared reborrows
P8L1 Jun 1, 2026
b5e7354
Fix CoerceShared field matching under decl_macro hygiene
P8L1 Jun 2, 2026
bffca17
Validate CoerceShared field relations after normalization
P8L1 Jun 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions compiler/rustc_borrowck/src/borrow_set.rs
View file
Open in desktop
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

Borrow-set gathering records MIR Reborrow loans.
This file now treats malformed generic reborrow shapes as delayed compiler bugs instead of hard panics.

View changes since the review

Original file line number Diff line number Diff line change
Expand Up @@ -305,15 +305,29 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'tcx> {
} 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;
Comment on lines 305 to +330
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

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.

View changes since the review

};
let region = region.as_var();
let kind = if mutability == Mutability::Mut {
Expand Down
215 changes: 161 additions & 54 deletions compiler/rustc_borrowck/src/type_check/mod.rs
View file
Open in desktop
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

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.

View changes since the review

Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use rustc_middle::mir::*;
use rustc_middle::traits::query::NoSolution;
use rustc_middle::ty::adjustment::PointerCoercion;
use rustc_middle::ty::cast::CastTy;
use rustc_middle::ty::reborrow::{self, CoerceSharedFieldPairError};
use rustc_middle::ty::{
self, CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations, CoroutineArgsExt,
GenericArgsRef, Ty, TyCtxt, TypeVisitableExt, UserArgs, UserTypeAnnotationIndex, fold_regions,
Expand Down Expand Up @@ -2489,9 +2490,23 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {

let borrowed_ty = borrowed_place.ty(self.body, tcx).ty;

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 {
Copy link
Copy Markdown
Contributor

@aapoalas aapoalas Jun 4, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

note: These changes are questionable. These paths should be unreachable, so why are we adding extra dead code?

View changes since the review

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;
};
Comment on lines +2493 to +2509
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

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.

View changes since the review

constraints.liveness_constraints.add_location(dest_region.as_var(), location);

// In Polonius mode, we also push a `loan_issued_at` fact
Expand All @@ -2509,65 +2524,157 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
}

if mutability.is_not() {
// FIXME(reborrow): for CoerceShared we need to relate the types manually, field by
// field. We cannot just attempt to relate `T` and `<T as CoerceShared>::Target` by
// calling relate_types as they are (generally) two unrelated user-defined ADTs, such as
// `CustomMut<'a>` and `CustomRef<'a>`, or `CustomMut<'a, T>` and `CustomRef<'a, T>`.
// Field-by-field relate_types is expected to work based on the wf-checks that the
// CoerceShared trait performs.
let ty::Adt(borrowed_adt, borrowed_args) = borrowed_ty.kind() else { unreachable!() };
let borrowed_fields = borrowed_adt.all_fields().collect::<Vec<_>>();
for dest_field in dest_adt.all_fields() {
let Some(borrowed_field) =
borrowed_fields.iter().find(|f| f.name == dest_field.name)
else {
continue;
};
let dest_ty = dest_field.ty(tcx, dest_args).skip_norm_wip();
let borrowed_ty = borrowed_field.ty(tcx, borrowed_args).skip_norm_wip();
if let (
ty::Ref(borrow_region, _, Mutability::Mut),
ty::Ref(ref_region, _, Mutability::Not),
) = (borrowed_ty.kind(), dest_ty.kind())
{
self.relate_types(
borrowed_ty.peel_refs(),
ty::Variance::Covariant,
dest_ty.peel_refs(),
location.to_locations(),
category,
)
.unwrap();
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,
});
} else {
self.relate_types(
borrowed_ty,
ty::Variance::Covariant,
dest_ty,
location.to_locations(),
category,
)
.unwrap();
}
// 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;
Comment on lines +2527 to +2543
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

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.

View changes since the review

}
} else {
// 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
);
}
}
}
Comment on lines 2546 to +2564
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

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.

View changes since the review


fn add_generic_shared_reborrow_constraints(
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

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.

View changes since the review

&mut self,
location: Location,
borrowed_place: &Place<'tcx>,
borrowed_ty: Ty<'tcx>,
dest_ty: Ty<'tcx>,
category: ConstraintCategory<'tcx>,
) -> Result<(), ()> {
let tcx = self.tcx();
let borrowed_ty = self.normalize(ty::Unnormalized::new_wip(borrowed_ty), location);
let dest_ty = self.normalize(ty::Unnormalized::new_wip(dest_ty), location);

if let (
ty::Ref(borrow_region, _, Mutability::Mut),
ty::Ref(ref_region, _, Mutability::Not),
) = (borrowed_ty.kind(), dest_ty.kind())
{
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() =>
{
Comment on lines +2583 to +2616
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

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.

View changes since the review

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(())
}
Comment on lines +2617 to +2656
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

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.

View changes since the review

_ => {
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(())
}
}
Comment on lines +2657 to +2677
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

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.

View changes since the review

}
}

Expand Down
48 changes: 43 additions & 5 deletions compiler/rustc_borrowck/src/type_check/relate_tys.rs
View file
Open in desktop
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

This relation helper is used by borrowck when relating MIR-place and reborrow field types.
The PR adds an opt-in path for structurally relating aliases, needed when a CoerceShared field uses an alias or projection.

View changes since the review

Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,35 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
locations: Locations,
category: ConstraintCategory<'tcx>,
) -> Result<(), NoSolution> {
NllTypeRelating::new(self, locations, category, UniverseInfo::relate(a, b), v)
.relate(a, b)?;
NllTypeRelating::new(
self,
locations,
category,
UniverseInfo::relate(a, b),
v,
StructurallyRelateAliases::No,
)
.relate(a, b)?;
Ok(())
}

pub(super) fn relate_types_structurally_relating_aliases(
&mut self,
a: Ty<'tcx>,
v: ty::Variance,
b: Ty<'tcx>,
locations: Locations,
category: ConstraintCategory<'tcx>,
) -> Result<(), NoSolution> {
NllTypeRelating::new(
self,
locations,
category,
UniverseInfo::relate(a, b),
v,
StructurallyRelateAliases::Yes,
)
.relate(a, b)?;
Comment on lines +43 to +71
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

The ordinary relate_types path explicitly keeps StructurallyRelateAliases::No.
A new helper enables structural alias relating only for field-wise CoerceShared constraints.

View changes since the review

Ok(())
}

Expand All @@ -53,8 +80,15 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
locations: Locations,
category: ConstraintCategory<'tcx>,
) -> Result<(), NoSolution> {
NllTypeRelating::new(self, locations, category, UniverseInfo::other(), ty::Invariant)
.relate(a, b)?;
NllTypeRelating::new(
self,
locations,
category,
UniverseInfo::other(),
ty::Invariant,
StructurallyRelateAliases::No,
)
.relate(a, b)?;
Comment on lines +83 to +91
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

The invariant helper is updated for the new constructor parameter but keeps alias structural relation disabled.
This protects existing borrowck relations from adopting the new alias behavior transitively.

View changes since the review

Ok(())
}
}
Expand All @@ -81,6 +115,8 @@ struct NllTypeRelating<'a, 'b, 'tcx> {
ambient_variance: ty::Variance,

ambient_variance_info: ty::VarianceDiagInfo<TyCtxt<'tcx>>,

structurally_relate_aliases: StructurallyRelateAliases,
Comment on lines +118 to +119
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

NllTypeRelating now carries the alias-relating mode chosen by the caller.
The relation trait method returns that stored mode instead of hard-coding No.

View changes since the review

}

impl<'a, 'b, 'tcx> NllTypeRelating<'a, 'b, 'tcx> {
Expand All @@ -90,6 +126,7 @@ impl<'a, 'b, 'tcx> NllTypeRelating<'a, 'b, 'tcx> {
category: ConstraintCategory<'tcx>,
universe_info: UniverseInfo<'tcx>,
ambient_variance: ty::Variance,
structurally_relate_aliases: StructurallyRelateAliases,
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

This extends the relation constructor signature so each caller chooses the alias-relating mode explicitly.
The invariant is that ordinary relations keep the old behavior while CoerceShared field leaves can opt in.

View changes since the review

) -> Self {
Self {
type_checker,
Expand All @@ -98,6 +135,7 @@ impl<'a, 'b, 'tcx> NllTypeRelating<'a, 'b, 'tcx> {
universe_info,
ambient_variance,
ambient_variance_info: ty::VarianceDiagInfo::default(),
structurally_relate_aliases,
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

The selected alias mode is stored in the relation state next to variance and universe context.
This makes the later trait hook reflect caller intent without recomputing from the related types.

View changes since the review

}
}

Expand Down Expand Up @@ -553,7 +591,7 @@ impl<'b, 'tcx> PredicateEmittingRelation<InferCtxt<'tcx>> for NllTypeRelating<'_
}

fn structurally_relate_aliases(&self) -> StructurallyRelateAliases {
StructurallyRelateAliases::No
self.structurally_relate_aliases
Copy link
Copy Markdown
Contributor Author

@P8L1 P8L1 Jun 2, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

The relation hook now returns the stored structural-alias mode instead of hard-coding No.
This is what lets borrowck field leaves relate aliases and projections when CoerceShared validation permits them.

View changes since the review

}

fn param_env(&self) -> ty::ParamEnv<'tcx> {
Expand Down
Loading
Loading