Skip to content

Support generic params in Lift_Generic #156956

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

Merged
rust-bors merged 5 commits into from
Jun 4, 2026
Merged

Support generic params in Lift_Generic #156956

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bb5f1c8
`Lift_Generic` can lift generic parameters
Jamesbarford May 26, 2026
2837932
Add comment explaining why we need a `Lift` bound for all fields
Jamesbarford May 26, 2026
91a328f
Can lift structs with a field of `PhantomData`
Jamesbarford May 27, 2026
a55c892
Simplify generic Lift bounds
Jamesbarford Jun 3, 2026
57da2f6
Add documentation comment for `lift_derive`, move `PhantomData` check…
Jamesbarford Jun 4, 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
19 changes: 1 addition & 18 deletions compiler/rustc_type_ir/src/binder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::{self as ty, DebruijnIndex, Interner, UniverseIndex, Unnormalized};
///
/// `Decodable` and `Encodable` are implemented for `Binder<T>` using the `impl_binder_encode_decode!` macro.
#[derive_where(Clone, Copy, Hash, PartialEq, Debug; I: Interner, T)]
#[derive(GenericTypeVisitable)]
#[derive(GenericTypeVisitable, Lift_Generic)]
#[cfg_attr(feature = "nightly", derive(StableHash_NoContext))]
pub struct Binder<I: Interner, T> {
value: T,
Expand All @@ -36,23 +36,6 @@ pub struct Binder<I: Interner, T> {

impl<I: Interner, T: Eq> Eq for Binder<I, T> {}

// FIXME: We manually derive `Lift` because the `derive(Lift_Generic)` doesn't
// understand how to turn `T` to `T::Lifted` in the output `type Lifted`.
impl<I: Interner, U: Interner, T> Lift<U> for Binder<I, T>
where
T: Lift<U>,
I::BoundVarKinds: Lift<U, Lifted = U::BoundVarKinds>,
{
type Lifted = Binder<U, T::Lifted>;

fn lift_to_interner(self, cx: U) -> Self::Lifted {
Binder {
value: self.value.lift_to_interner(cx),
bound_vars: self.bound_vars.lift_to_interner(cx),
}
}
}

#[cfg(feature = "nightly")]
macro_rules! impl_binder_encode_decode {
($($t:ty),+ $(,)?) => {
Expand Down
97 changes: 82 additions & 15 deletions compiler/rustc_type_ir_macros/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,18 @@ decl_derive!(
[TypeFoldable_Generic, attributes(type_foldable)] => type_foldable_derive
);
decl_derive!(
[Lift_Generic] => lift_derive
[Lift_Generic, attributes(lift)] => lift_derive
);
#[cfg(not(feature = "nightly"))]
decl_derive!(
[GenericTypeVisitable] => customizable_type_visitable_derive
);

struct LiftedTy {
ty: syn::Type,
generic_parameter_bounds: Vec<syn::Ident>,
}

fn has_ignore_attr(attrs: &[Attribute], name: &'static str, meta: &'static str) -> bool {
let mut ignored = false;
attrs.iter().for_each(|attr| {
Expand Down Expand Up @@ -158,15 +163,46 @@ fn lift_derive(mut s: synstructure::Structure<'_>) -> proc_macro2::TokenStream {
s.add_impl_generic(parse_quote! { J });
s.add_where_predicate(parse_quote! { J: Interner });

let generic_parameters =
s.ast().generics.type_params().map(|ty| ty.ident.clone()).collect::<Vec<_>>();

let mut wc = vec![];
s.bind_with(|_| synstructure::BindStyle::Move);
let body_fold = s.each_variant(|vi| {
let bindings = vi.bindings();
vi.construct(|field, index| {
let ty = field.ty.clone();
let lifted_ty = lift(ty.clone());
wc.push(parse_quote! { #ty: ::rustc_type_ir::lift::Lift<J, Lifted = #lifted_ty> });
let bind = &bindings[index];
// Allow field to be ignored from lift
if has_ignore_attr(&field.attrs, "lift", "identity") {
return bind.to_token_stream();
}

let lifted = lift(ty.clone(), &generic_parameters);

for param in lifted.generic_parameter_bounds {
wc.push(parse_quote! { #param: ::rustc_type_ir::lift::Lift<J> });
}

// `lift(ty, ...)` rewrites any bare generic params inside the
// field type to their lifted associated type, e.g;
// `T` becomes `<T as Lift<J>>::Lifted`.
//
// That, however, does not imply that the field type itself
// implements `Lift<J>`. The generated body calls `lift_to_interner`
// on each field value, so Rust needs a `Lift<J>` impl for that
// value's complete declared type, not just for generic parameters
// that appear somewhere inside it. For non parameter fields, the
// implementation must also produce the exact mapped field type:
//
// Example:
// `I::DefId: Lift<J, Lifted = J::DefId>`
// `Binder<I, T>: Lift<J, Lifted = Binder<J, <T as Lift<J>>::Lifted>>`
if !is_type_param(&ty, &generic_parameters) {
let lifted_ty = lifted.ty;
wc.push(parse_quote! { #ty: ::rustc_type_ir::lift::Lift<J, Lifted = #lifted_ty> });
}

@lcnr lcnr May 26, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

so this adds field_ty: Lift<J, Lifted = mapped_field_ty>. Can you add a comment for why we need to add both T: Lift<J> (as in, for each param), and also support lifting for all fields?

It would be really cool if we can only add Lift bounds for the generic parameters, but likely that doesn't quite work?

View changes since the review

@Jamesbarford Jamesbarford May 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can't see a way to do that with the current generated code.

For example:

struct Foo<I: Interner, T> {
    value: T,
    id: I::DefId,
}

Adding bounds only for generic parameters gives us T: Lift<J>, which is enough for value.

However the generated body also calls lift_to_interner(...) on id, and the lifted struct expects that field to become J::DefId. So, unless I'm missing something, we still need:

I::DefId: Lift<J, Lifted = J::DefId>

Addressed feedback in; 2837932

@lcnr lcnr May 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

one way would be to add

trait LiftInto<J>: Interner
where
    Self::DefId: Lift<J, Lifted, J::DefId>,
    ...

I would expect this should work? 🤔 I never quite know how implied bounds work :>

and this would have a blanket impl that holds exactly if the implied bounds of the trait hold

@lcnr lcnr May 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah, need to write it a bit differently, can't use where-clauses, need to use associated type bounds

trait Super {
    type Assoc;
}

trait Trait: Super<Assoc: Copy> {}

fn yay<T: Trait>(x: &T::Assoc) -> T::Assoc {
    *x
}

@Jamesbarford Jamesbarford May 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think I follow. I tried with the following;

// I used a macro to deduplicate the code but what it generated was;

pub trait InternerCanLiftTo<J>:
    Interner<
        DefId: crate::lift::Lift<J, Lifted = J::DefId>,
        TraitId: crate::lift::Lift<J, Lifted = J::TraitId>,
        // etc..
    >
where
    J: Interner,
{}

impl<I, J> InternerCanLiftTo<J> for I
where
    J: Interner,
    I: Interner<
            DefId: crate::lift::Lift<J, Lifted = J::DefId>,
            TraitId: crate::lift::Lift<J, Lifted = J::TraitId>,
            // etc...
        >,
{}

The list, as you can imagine, was long. I did get rustc_type_ir to compile with this idea.

However this would fail in rustc_middle. I believe the problem is that InternerCanLiftTo<J> is a global bound on the whole Interner, while the bounds we need are local to each derived type. If the trait includes ParamEnv, Symbol etc., then every #[derive(Lift_Generic)] requires those associated types to implement Lift, even for types that do not contain those fields.

It seems a bit off in so much as it either does not include enough associated types, or it includes enough and subsequently over-constrains unrelated derived impls. Though I have perhaps missed your point and tried something a bit off piste 😅.

I have prototyped something that allows us to remove most manual implementations and I think with a bit of tweaking get rid of all manual implementations. So I shall push that up once I've got something working.

@Jamesbarford Jamesbarford May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

91a328f leaves 3 manual implementations of which there are two code patterns;

  1. An implementation of Lift for Option which seems distinct enough from the other implementations that leaving it alone seemed reasonable to me.
impl<'tcx, T: Lift<TyCtxt<'tcx>>> Lift<TyCtxt<'tcx>> for Option<T> {
    type Lifted = Option<T::Lifted>;
    fn lift_to_interner(self, tcx: TyCtxt<'tcx>) -> Self::Lifted {
        self.map(|x| tcx.lift(x))
    }
}
  1. An implementation of Lift for a struct where there is a match on self.kind() for example;
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct Term<'tcx> {
    ptr: NonNull<()>,
    marker: PhantomData<(Ty<'tcx>, Const<'tcx>)>,
}

impl<'a, 'tcx> Lift<TyCtxt<'tcx>> for Term<'a> {
    type Lifted = ty::Term<'tcx>;
    fn lift_to_interner(self, tcx: TyCtxt<'tcx>) -> Self::Lifted {
        match self.kind() {
            TermKind::Ty(ty) => tcx.lift(ty).into(),
            TermKind::Const(c) => tcx.lift(c).into(),
        }
    }
}

A way I saw to be able to handle that would be to add a #[derive(<new_name>)] to an enum and then possibly create another #[derive(Lift_KindGeneric)] to then generate the implementation of fn lift_to_interner(). I experimented with that and had something broadly working locally in a toy example but I'm not sure if that's too much magic for what we are trying to achieve. Moreover this pattern is only implemented in two places and the Lift_Generic lives in rustc_type_ir_macros;

  • compiler/rustc_middle/src/ty/generic_args.rs
  • compiler/rustc_middle/src/ty/structural_impls.rs

@lcnr lcnr Jun 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems a bit off in so much as it either does not include enough associated types, or it includes enough and subsequently over-constrains unrelated derived impls. Though I have perhaps missed your point and tried something a bit off piste 😅.

I would include all types we'd ever want to lift in there. While this is not necessary for all impls of a given type, it is required for lift to make sense in general

@lcnr lcnr Jun 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the option Lift impl is just the derive. whether u call map or match on Option manually doesn't really matter :3

@lcnr lcnr Jun 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lift for these packed pointers is more interesting and would prolly keep this as a manual impl, seems innocent enough

@Jamesbarford Jamesbarford Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would include all types we'd ever want to lift in there

Done; a55c892.

This did mean I had to implement Lift for a couple other types, as well as expand a macro. I got a pipeline failure So implemented Lift<I> for ()

   Compiling unicase v2.8.1
error[E0599]: no method named `lift_to_interner` found for unit type `()` in the current scope
  --> compiler/rustc_type_ir/src/const_kind.rs:72:77
   |
72 |   #[derive(TypeVisitable_Generic, GenericTypeVisitable, TypeFoldable_Generic, Lift_Generic)]
   |                                                                               -^^^^^^^^^^^
   |                                                                               |
   |                                                                               method not found in `()`
   |                                                                               in this derive macro expansion
   |


quote! {
#bind.lift_to_interner(interner)
}
Expand All @@ -179,7 +215,8 @@ fn lift_derive(mut s: synstructure::Structure<'_>) -> proc_macro2::TokenStream {
let (_, ty_generics, _) = s.ast().generics.split_for_impl();
let name = s.ast().ident.clone();
let self_ty: syn::Type = parse_quote! { #name #ty_generics };
let lifted_ty = lift(self_ty);
let lifted = lift(self_ty, &generic_parameters);
let lifted_ty = lifted.ty;

s.bound_impl(
quote!(::rustc_type_ir::lift::Lift<J>),
Expand All @@ -196,24 +233,54 @@ fn lift_derive(mut s: synstructure::Structure<'_>) -> proc_macro2::TokenStream {
)
}

fn lift(mut ty: syn::Type) -> syn::Type {
struct ItoJ;
impl VisitMut for ItoJ {
/// Returns true only for bare generic parameters like `T`, not paths such as
/// `I::Ty`, `Vec<T>`, or `Binder<I, T>`
fn is_type_param(ty: &syn::Type, generic_parameters: &[syn::Ident]) -> bool {
if let syn::Type::Path(ty) = ty
&& ty.path.segments.len() == 1
&& let Some(segment) = ty.path.segments.first()
{
matches!(segment.arguments, syn::PathArguments::None)
&& generic_parameters.iter().any(|param| segment.ident == *param)
} else {
false
}
}

fn lift(mut ty: syn::Type, generic_parameters: &[syn::Ident]) -> LiftedTy {
struct ItoJ<'a> {
generic_parameters: &'a [syn::Ident],
generic_parameter_bounds: Vec<syn::Ident>,
}

impl VisitMut for ItoJ<'_> {
fn visit_type_path_mut(&mut self, i: &mut syn::TypePath) {
if i.qself.is_none() {
if let Some(first) = i.path.segments.first_mut()
&& first.ident == "I"
{
*first = parse_quote! { J };
let segments_len = i.path.segments.len();
if let Some(first) = i.path.segments.first_mut() {
// Turn paths from `I` into `J`
if first.ident == "I" {
*first = parse_quote! { J };
} else if segments_len == 1
&& matches!(first.arguments, syn::PathArguments::None)
&& self.generic_parameters.iter().any(|param| first.ident == *param)
{
let ident = first.ident.clone();
if !self.generic_parameter_bounds.iter().any(|param| *param == ident) {
self.generic_parameter_bounds.push(ident.clone());
}

*i = parse_quote! { <#ident as ::rustc_type_ir::lift::Lift<J>>::Lifted };
return;
}
}
}
syn::visit_mut::visit_type_path_mut(self, i);
}
}

ItoJ.visit_type_mut(&mut ty);

ty
let mut visitor = ItoJ { generic_parameters, generic_parameter_bounds: Vec::new() };
visitor.visit_type_mut(&mut ty);
LiftedTy { ty, generic_parameter_bounds: visitor.generic_parameter_bounds }
}

#[cfg(not(feature = "nightly"))]
Expand Down
Loading