Skip to content

Structurally normalize types as needed in projection_ty_core #141703

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.

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

Merged
merged 2 commits into from
May 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 8 additions & 7 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,17 +474,18 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
let projected_ty = curr_projected_ty.projection_ty_core(
tcx,
proj,
|this, field, ()| {
let ty = this.field_ty(tcx, field);
self.structurally_resolve(ty, locations)
},
|_, _| unreachable!(),
|ty| self.structurally_resolve(ty, locations),
|ty, variant_index, field, ()| PlaceTy::field_ty(tcx, ty, variant_index, field),
|_| unreachable!(),
);
curr_projected_ty = projected_ty;
}
trace!(?curr_projected_ty);

let ty = curr_projected_ty.ty;
// Need to renormalize `a` as typecheck may have failed to normalize
// higher-ranked aliases if normalization was ambiguous due to inference.
let a = self.normalize(a, locations);
let ty = self.normalize(curr_projected_ty.ty, locations);
Copy link
Member Author

Choose a reason for hiding this comment

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

We have to normalize the final output type only in the old solver, since it's no longer guaranteed to be normalized.

Copy link
Member Author

Choose a reason for hiding this comment

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

This normalize call actually makes it easier to hit #141708, which is why CI was failing on icu_provider.

The bug report demonstrates that it's still possible to hit today, and it's fixed by the second commit which always normalizes the type for a.

Unfortunate, but necessary unless we wanted to (e.g.) eagerly normalize all closure signatures in writeback, which presumably has its own issues.

self.relate_types(ty, v.xform(ty::Contravariant), a, locations, category)?;

Ok(())
Expand Down Expand Up @@ -1852,7 +1853,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
| ProjectionElem::Downcast(..) => {}
ProjectionElem::Field(field, fty) => {
let fty = self.normalize(fty, location);
let ty = base_ty.field_ty(tcx, field);
let ty = PlaceTy::field_ty(tcx, base_ty.ty, base_ty.variant_index, field);
let ty = self.normalize(ty, location);
debug!(?fty, ?ty);

Expand Down
78 changes: 42 additions & 36 deletions compiler/rustc_middle/src/mir/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,26 +88,31 @@ impl<'tcx> PlaceTy<'tcx> {
///
/// Note that the resulting type has not been normalized.
#[instrument(level = "debug", skip(tcx), ret)]
pub fn field_ty(self, tcx: TyCtxt<'tcx>, f: FieldIdx) -> Ty<'tcx> {
if let Some(variant_index) = self.variant_index {
match *self.ty.kind() {
pub fn field_ty(
Copy link
Member Author

Choose a reason for hiding this comment

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

See comment below for why I split the self arg up into its fields.

tcx: TyCtxt<'tcx>,
self_ty: Ty<'tcx>,
variant_idx: Option<VariantIdx>,
f: FieldIdx,
) -> Ty<'tcx> {
if let Some(variant_index) = variant_idx {
match *self_ty.kind() {
ty::Adt(adt_def, args) if adt_def.is_enum() => {
adt_def.variant(variant_index).fields[f].ty(tcx, args)
}
ty::Coroutine(def_id, args) => {
let mut variants = args.as_coroutine().state_tys(def_id, tcx);
let Some(mut variant) = variants.nth(variant_index.into()) else {
bug!("variant {variant_index:?} of coroutine out of range: {self:?}");
bug!("variant {variant_index:?} of coroutine out of range: {self_ty:?}");
};

variant
.nth(f.index())
.unwrap_or_else(|| bug!("field {f:?} out of range: {self:?}"))
variant.nth(f.index()).unwrap_or_else(|| {
bug!("field {f:?} out of range of variant: {self_ty:?} {variant_idx:?}")
})
}
_ => bug!("can't downcast non-adt non-coroutine type: {self:?}"),
_ => bug!("can't downcast non-adt non-coroutine type: {self_ty:?}"),
}
} else {
match self.ty.kind() {
match self_ty.kind() {
ty::Adt(adt_def, args) if !adt_def.is_enum() => {
adt_def.non_enum_variant().fields[f].ty(tcx, args)
}
Expand All @@ -116,26 +121,25 @@ impl<'tcx> PlaceTy<'tcx> {
.upvar_tys()
.get(f.index())
.copied()
.unwrap_or_else(|| bug!("field {f:?} out of range: {self:?}")),
.unwrap_or_else(|| bug!("field {f:?} out of range: {self_ty:?}")),
ty::CoroutineClosure(_, args) => args
.as_coroutine_closure()
.upvar_tys()
.get(f.index())
.copied()
.unwrap_or_else(|| bug!("field {f:?} out of range: {self:?}")),
.unwrap_or_else(|| bug!("field {f:?} out of range: {self_ty:?}")),
// Only prefix fields (upvars and current state) are
// accessible without a variant index.
ty::Coroutine(_, args) => args
.as_coroutine()
.prefix_tys()
.get(f.index())
.copied()
.unwrap_or_else(|| bug!("field {f:?} out of range: {self:?}")),
ty::Coroutine(_, args) => {
args.as_coroutine().prefix_tys().get(f.index()).copied().unwrap_or_else(|| {
bug!("field {f:?} out of range of prefixes for {self_ty}")
})
}
ty::Tuple(tys) => tys
.get(f.index())
.copied()
.unwrap_or_else(|| bug!("field {f:?} out of range: {self:?}")),
_ => bug!("can't project out of {self:?}"),
.unwrap_or_else(|| bug!("field {f:?} out of range: {self_ty:?}")),
_ => bug!("can't project out of {self_ty:?}"),
}
}
}
Expand All @@ -148,11 +152,11 @@ impl<'tcx> PlaceTy<'tcx> {
elems.iter().fold(self, |place_ty, &elem| place_ty.projection_ty(tcx, elem))
}

/// Convenience wrapper around `projection_ty_core` for
/// `PlaceElem`, where we can just use the `Ty` that is already
/// stored inline on field projection elems.
/// Convenience wrapper around `projection_ty_core` for `PlaceElem`,
/// where we can just use the `Ty` that is already stored inline on
/// field projection elems.
pub fn projection_ty(self, tcx: TyCtxt<'tcx>, elem: PlaceElem<'tcx>) -> PlaceTy<'tcx> {
self.projection_ty_core(tcx, &elem, |_, _, ty| ty, |_, ty| ty)
self.projection_ty_core(tcx, &elem, |ty| ty, |_, _, _, ty| ty, |ty| ty)
}

/// `place_ty.projection_ty_core(tcx, elem, |...| { ... })`
Expand All @@ -164,8 +168,9 @@ impl<'tcx> PlaceTy<'tcx> {
self,
tcx: TyCtxt<'tcx>,
elem: &ProjectionElem<V, T>,
mut handle_field: impl FnMut(&Self, FieldIdx, T) -> Ty<'tcx>,
mut handle_opaque_cast_and_subtype: impl FnMut(&Self, T) -> Ty<'tcx>,
Copy link
Member Author

Choose a reason for hiding this comment

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

It's a bit awkward for these callbacks to take a PlaceTy, since then we can't structurally normalize it without mutating it in place or something.

So I blew up the struct into its fields, which is most of the changes in this PR.

mut structurally_normalize: impl FnMut(Ty<'tcx>) -> Ty<'tcx>,
mut handle_field: impl FnMut(Ty<'tcx>, Option<VariantIdx>, FieldIdx, T) -> Ty<'tcx>,
mut handle_opaque_cast_and_subtype: impl FnMut(T) -> Ty<'tcx>,
) -> PlaceTy<'tcx>
where
V: ::std::fmt::Debug,
Expand All @@ -176,16 +181,16 @@ impl<'tcx> PlaceTy<'tcx> {
}
let answer = match *elem {
ProjectionElem::Deref => {
let ty = self.ty.builtin_deref(true).unwrap_or_else(|| {
let ty = structurally_normalize(self.ty).builtin_deref(true).unwrap_or_else(|| {
bug!("deref projection of non-dereferenceable ty {:?}", self)
});
PlaceTy::from_ty(ty)
}
ProjectionElem::Index(_) | ProjectionElem::ConstantIndex { .. } => {
PlaceTy::from_ty(self.ty.builtin_index().unwrap())
PlaceTy::from_ty(structurally_normalize(self.ty).builtin_index().unwrap())
}
ProjectionElem::Subslice { from, to, from_end } => {
PlaceTy::from_ty(match self.ty.kind() {
PlaceTy::from_ty(match structurally_normalize(self.ty).kind() {
ty::Slice(..) => self.ty,
ty::Array(inner, _) if !from_end => Ty::new_array(tcx, *inner, to - from),
ty::Array(inner, size) if from_end => {
Expand All @@ -201,17 +206,18 @@ impl<'tcx> PlaceTy<'tcx> {
ProjectionElem::Downcast(_name, index) => {
PlaceTy { ty: self.ty, variant_index: Some(index) }
}
ProjectionElem::Field(f, fty) => PlaceTy::from_ty(handle_field(&self, f, fty)),
ProjectionElem::OpaqueCast(ty) => {
PlaceTy::from_ty(handle_opaque_cast_and_subtype(&self, ty))
}
ProjectionElem::Subtype(ty) => {
PlaceTy::from_ty(handle_opaque_cast_and_subtype(&self, ty))
}
ProjectionElem::Field(f, fty) => PlaceTy::from_ty(handle_field(
structurally_normalize(self.ty),
Copy link
Member Author

Choose a reason for hiding this comment

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

See above, here we'd need to do something like:

{
    self.ty = structurally_normalize(self.ty);
    PlaceTy::from(handle_field(self, f, fty))
}

which I think is kinda awkward.

self.variant_index,
f,
fty,
)),
ProjectionElem::OpaqueCast(ty) => PlaceTy::from_ty(handle_opaque_cast_and_subtype(ty)),
ProjectionElem::Subtype(ty) => PlaceTy::from_ty(handle_opaque_cast_and_subtype(ty)),

// FIXME(unsafe_binders): Rename `handle_opaque_cast_and_subtype` to be more general.
ProjectionElem::UnwrapUnsafeBinder(ty) => {
PlaceTy::from_ty(handle_opaque_cast_and_subtype(&self, ty))
PlaceTy::from_ty(handle_opaque_cast_and_subtype(ty))
}
};
debug!("projection_ty self: {:?} elem: {:?} yields: {:?}", self, elem, answer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,9 @@ impl<'a, 'tcx> ParseCtxt<'a, 'tcx> {
fn parse_place_inner(&self, expr_id: ExprId) -> PResult<(Place<'tcx>, PlaceTy<'tcx>)> {
let (parent, proj) = parse_by_kind!(self, expr_id, expr, "place",
@call(mir_field, args) => {
let (parent, ty) = self.parse_place_inner(args[0])?;
let (parent, place_ty) = self.parse_place_inner(args[0])?;
let field = FieldIdx::from_u32(self.parse_integer_literal(args[1])? as u32);
let field_ty = ty.field_ty(self.tcx, field);
let field_ty = PlaceTy::field_ty(self.tcx, place_ty.ty, place_ty.variant_index, field);
let proj = PlaceElem::Field(field, field_ty);
let place = parent.project_deeper(&[proj], self.tcx);
return Ok((place, PlaceTy::from_ty(field_ty)));
Expand Down
31 changes: 31 additions & 0 deletions tests/ui/nll/user-annotations/normalizing-user-annotation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//@ check-pass
//@ revisions: current next
//@ ignore-compare-mode-next-solver (explicit revisions)
//@[next] compile-flags: -Znext-solver

// Regression test for <https://github.com/rust-lang/rust/issues/141708>.

// See description in there; this has to do with fundamental limitations
// to the old trait solver surrounding higher-ranked aliases with infer
// vars. This always worked in the new trait solver, but I added a revision
// just for good measure.

trait Foo<'a> {
type Assoc;
}

impl Foo<'_> for i32 {
type Assoc = u32;
}

impl Foo<'_> for u32 {
type Assoc = u32;
}

fn foo<'b: 'b, T: for<'a> Foo<'a>, F: for<'a> Fn(<T as Foo<'a>>::Assoc)>(_: F) -> (T, F) {
todo!()
}

fn main() {
let (x, c): (i32, _) = foo::<'static, _, _>(|_| {});
}
32 changes: 32 additions & 0 deletions tests/ui/traits/next-solver/normalize/normalize-place-elem.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
//@ check-pass
//@ compile-flags: -Znext-solver

// Regression test for <https://github.com/rust-lang/trait-system-refactor-initiative/issues/221>.
// Ensure that we normalize after applying projection elems in MIR typeck.

use std::marker::PhantomData;

#[derive(Copy, Clone)]
struct Span;

trait AstKind {
type Inner;
}

struct WithSpan;
impl AstKind for WithSpan {
type Inner
= (i32,);
}

struct Expr<'a> { f: &'a <WithSpan as AstKind>::Inner }

impl Expr<'_> {
fn span(self) {
match self {
Self { f: (n,) } => {},
}
}
}

fn main() {}
Loading