Skip trivial fields in derived traversables

Author: eggyal

compiler/rustc_macros/Cargo.toml

@@ -13,3 +13,6 @@ quote = "1"
 syn = { version = "2.0.9", features = ["full"] }
 synstructure = "0.13.0"
 # tidy-alphabetical-end
+
+[dev-dependencies]
+syn = { version = "*", features = ["visit-mut"] }

compiler/rustc_macros/src/lib.rs

@@ -74,17 +74,15 @@ decl_derive!([TyEncodable] => serialize::type_encodable_derive);
 decl_derive!([MetadataDecodable] => serialize::meta_decodable_derive);
 decl_derive!([MetadataEncodable] => serialize::meta_encodable_derive);
 decl_derive!(
-    [TypeFoldable, attributes(type_foldable)] =>
+    [TypeFoldable] =>
     /// Derives `TypeFoldable` for the annotated `struct` or `enum` (`union` is not supported).
     ///
-    /// Folds will produce a value of the same struct or enum variant as the input, with each field
-    /// respectively folded (in definition order) using the `TypeFoldable` implementation for its
-    /// type. However, if a field of a struct or of an enum variant is annotated with
-    /// `#[type_foldable(identity)]` then that field will retain its incumbent value (and its type
-    /// is not required to implement `TypeFoldable`). However use of this attribute is dangerous
-    /// and should be used with extreme caution: should the type of the annotated field contain
-    /// (now or in the future) a type that is of interest to a folder, it will not get folded (which
-    /// may result in unexpected, hard-to-track bugs that could result in unsoundness).
+    /// Folds will produce a value of the same struct or enum variant as the input, with trivial
+    /// fields unchanged and all non-trivial fields respectively folded (in definition order) using
+    /// the `TypeFoldable` implementation for its type. A field of type `T` is "trivial" if `T`
+    /// both does not reference any generic type parameters and either
+    /// - does not reference any `'tcx` lifetime parameter; or
+    /// - does not contain anything that could be of interest to folders.
     ///
     /// If the annotated type has a `'tcx` lifetime parameter, then that will be used as the
     /// lifetime for the type context/interner; otherwise the lifetime of the type context/interner
@@ -100,17 +98,15 @@ decl_derive!(
     traversable::traversable_derive::<traversable::Foldable>
 );
 decl_derive!(
-    [TypeVisitable, attributes(type_visitable)] =>
+    [TypeVisitable] =>
     /// Derives `TypeVisitable` for the annotated `struct` or `enum` (`union` is not supported).
     ///
-    /// Each field of the struct or enum variant will be visited (in definition order) using the
-    /// `TypeVisitable` implementation for its type. However, if a field of a struct or of an enum
-    /// variant is annotated with `#[type_visitable(ignore)]` then that field will not be visited
-    /// (and its type is not required to implement `TypeVisitable`). However use of this attribute
-    /// is dangerous and should be used with extreme caution: should the type of the annotated
-    /// field (now or in the future) a type that is of interest to a visitor, it will not get
-    /// visited (which may result in unexpected, hard-to-track bugs that could result in
-    /// unsoundness).
+    /// Each non-trivial field of the struct or enum variant will be visited (in definition order)
+    /// using the `TypeVisitable` implementation for its type; trivial fields will be ignored. A
+    /// field of type `T` is "trivial" if `T` both does not reference any generic type parameters
+    /// and either
+    /// - does not reference any `'tcx` lifetime parameter; or
+    /// - does not contain anything that could be of interest to visitors.
     ///
     /// If the annotated type has a `'tcx` lifetime parameter, then that will be used as the
     /// lifetime for the type context/interner; otherwise the lifetime of the type context/interner

compiler/rustc_macros/src/traversable.rs

@@ -1,126 +1,247 @@
-use proc_macro2::TokenStream;
+use proc_macro2::{Ident, Span, TokenStream};
 use quote::{quote, ToTokens};
-use syn::parse_quote;
+use std::collections::HashSet;
+use syn::{parse_quote, visit, Field, Generics, Lifetime};
+
+#[cfg(test)]
+mod tests;
+
+/// Generate a type parameter with the given `suffix` that does not conflict with
+/// any of the `existing` generics.
+fn gen_param(suffix: impl ToString, existing: &Generics) -> Ident {
+    let mut suffix = suffix.to_string();
+    while existing.type_params().any(|t| t.ident == suffix) {
+        suffix.insert(0, '_');
+    }
+    Ident::new(&suffix, Span::call_site())
+}
+
+#[derive(Clone, Copy, PartialEq)]
+enum Type {
+    /// Describes a type that is not parameterised by the interner, and therefore cannot
+    /// be of any interest to traversers.
+    Trivial,
+
+    /// Describes a type that is parameterised by the interner lifetime `'tcx` but that is
+    /// otherwise not generic.
+    NotGeneric,
+
+    /// Describes a type that is generic.
+    Generic,
+}
+use Type::*;
+
+pub struct Interner<'a>(Option<&'a Lifetime>);
+
+impl<'a> Interner<'a> {
+    /// Return the `TyCtxt` interner for the given `structure`.
+    ///
+    /// If the input represented by `structure` has a `'tcx` lifetime parameter, then that will be used
+    /// used as the lifetime of the `TyCtxt`. Otherwise a `'tcx` lifetime parameter that is unrelated
+    /// to the input will be used.
+    fn resolve(generics: &'a Generics) -> Self {
+        Self(
+            generics
+                .lifetimes()
+                .find_map(|def| (def.lifetime.ident == "tcx").then_some(&def.lifetime)),
+        )
+    }
+}
+
+impl ToTokens for Interner<'_> {
+    fn to_tokens(&self, tokens: &mut TokenStream) {
+        let default = parse_quote! { 'tcx };
+        let lt = self.0.unwrap_or(&default);
+        tokens.extend(quote! { ::rustc_middle::ty::TyCtxt<#lt> });
+    }
+}
 
 pub struct Foldable;
 pub struct Visitable;
 
 /// An abstraction over traversable traits.
 pub trait Traversable {
-    /// The trait that this `Traversable` represents.
-    fn traversable() -> TokenStream;
-
-    /// The `match` arms for a traversal of this type.
-    fn arms(structure: &mut synstructure::Structure<'_>) -> TokenStream;
-
-    /// The body of an implementation given the match `arms`.
-    fn impl_body(arms: impl ToTokens) -> TokenStream;
+    /// The trait that this `Traversable` represents, parameterised by `interner`.
+    fn traversable(interner: &Interner<'_>) -> TokenStream;
+
+    /// Any supertraits that this trait is required to implement.
+    fn supertraits(interner: &Interner<'_>) -> TokenStream;
+
+    /// A (`noop`) traversal of this trait upon the `bind` expression.
+    fn traverse(bind: TokenStream, noop: bool) -> TokenStream;
+
+    /// A `match` arm for `variant`, where `f` generates the tokens for each binding.
+    fn arm(
+        variant: &synstructure::VariantInfo<'_>,
+        f: impl FnMut(&synstructure::BindingInfo<'_>) -> TokenStream,
+    ) -> TokenStream;
+
+    /// The body of an implementation given the `interner`, `traverser` and match expression `body`.
+    fn impl_body(
+        interner: Interner<'_>,
+        traverser: impl ToTokens,
+        body: impl ToTokens,
+    ) -> TokenStream;
 }
 
 impl Traversable for Foldable {
-    fn traversable() -> TokenStream {
-        quote! { ::rustc_middle::ty::fold::TypeFoldable<::rustc_middle::ty::TyCtxt<'tcx>> }
-    }
-    fn arms(structure: &mut synstructure::Structure<'_>) -> TokenStream {
-        structure.each_variant(|vi| {
-            let bindings = vi.bindings();
-            vi.construct(|_, index| {
-                let bind = &bindings[index];
-
-                let mut fixed = false;
-
-                // retain value of fields with #[type_foldable(identity)]
-                bind.ast().attrs.iter().for_each(|x| {
-                    if !x.path().is_ident("type_foldable") {
-                        return;
-                    }
-                    let _ = x.parse_nested_meta(|nested| {
-                        if nested.path.is_ident("identity") {
-                            fixed = true;
-                        }
-                        Ok(())
-                    });
-                });
-
-                if fixed {
-                    bind.to_token_stream()
-                } else {
-                    quote! {
-                        ::rustc_middle::ty::fold::TypeFoldable::try_fold_with(#bind, __folder)?
-                    }
-                }
-            })
-        })
+    fn traversable(interner: &Interner<'_>) -> TokenStream {
+        quote! { ::rustc_middle::ty::fold::TypeFoldable<#interner> }
+    }
+    fn supertraits(interner: &Interner<'_>) -> TokenStream {
+        Visitable::traversable(interner)
     }
-    fn impl_body(arms: impl ToTokens) -> TokenStream {
+    fn traverse(bind: TokenStream, noop: bool) -> TokenStream {
+        if noop {
+            bind
+        } else {
+            quote! { ::rustc_middle::ty::fold::TypeFoldable::try_fold_with(#bind, folder)? }
+        }
+    }
+    fn arm(
+        variant: &synstructure::VariantInfo<'_>,
+        mut f: impl FnMut(&synstructure::BindingInfo<'_>) -> TokenStream,
+    ) -> TokenStream {
+        let bindings = variant.bindings();
+        variant.construct(|_, index| f(&bindings[index]))
+    }
+    fn impl_body(
+        interner: Interner<'_>,
+        traverser: impl ToTokens,
+        body: impl ToTokens,
+    ) -> TokenStream {
         quote! {
-            fn try_fold_with<__F: ::rustc_middle::ty::fold::FallibleTypeFolder<::rustc_middle::ty::TyCtxt<'tcx>>>(
+            fn try_fold_with<#traverser: ::rustc_middle::ty::fold::FallibleTypeFolder<#interner>>(
                 self,
-                __folder: &mut __F
-            ) -> ::core::result::Result<Self, __F::Error> {
-                ::core::result::Result::Ok(match self { #arms })
+                folder: &mut #traverser
+            ) -> ::core::result::Result<Self, #traverser::Error> {
+                ::core::result::Result::Ok(#body)
             }
         }
     }
 }
 
 impl Traversable for Visitable {
-    fn traversable() -> TokenStream {
-        quote! { ::rustc_middle::ty::visit::TypeVisitable<::rustc_middle::ty::TyCtxt<'tcx>> }
+    fn traversable(interner: &Interner<'_>) -> TokenStream {
+        quote! { ::rustc_middle::ty::visit::TypeVisitable<#interner> }
     }
-    fn arms(structure: &mut synstructure::Structure<'_>) -> TokenStream {
-        // ignore fields with #[type_visitable(ignore)]
-        structure.filter(|bi| {
-            let mut ignored = false;
-
-            bi.ast().attrs.iter().for_each(|attr| {
-                if !attr.path().is_ident("type_visitable") {
-                    return;
-                }
-                let _ = attr.parse_nested_meta(|nested| {
-                    if nested.path.is_ident("ignore") {
-                        ignored = true;
-                    }
-                    Ok(())
-                });
-            });
-
-            !ignored
-        });
-
-        structure.each(|bind| {
-            quote! {
-                ::rustc_middle::ty::visit::TypeVisitable::visit_with(#bind, __visitor)?;
-            }
-        })
+    fn supertraits(_: &Interner<'_>) -> TokenStream {
+        quote! { ::core::clone::Clone + ::core::fmt::Debug }
+    }
+    fn traverse(bind: TokenStream, noop: bool) -> TokenStream {
+        if noop {
+            quote! {}
+        } else {
+            quote! { ::rustc_middle::ty::visit::TypeVisitable::visit_with(#bind, visitor)?; }
+        }
     }
-    fn impl_body(arms: impl ToTokens) -> TokenStream {
+    fn arm(
+        variant: &synstructure::VariantInfo<'_>,
+        f: impl FnMut(&synstructure::BindingInfo<'_>) -> TokenStream,
+    ) -> TokenStream {
+        variant.bindings().iter().map(f).collect()
+    }
+    fn impl_body(
+        interner: Interner<'_>,
+        traverser: impl ToTokens,
+        body: impl ToTokens,
+    ) -> TokenStream {
         quote! {
-            fn visit_with<__V: ::rustc_middle::ty::visit::TypeVisitor<::rustc_middle::ty::TyCtxt<'tcx>>>(
+            fn visit_with<#traverser: ::rustc_middle::ty::visit::TypeVisitor<#interner>>(
                 &self,
-                __visitor: &mut __V
-            ) -> ::std::ops::ControlFlow<__V::BreakTy> {
-                match self { #arms }
-                ::std::ops::ControlFlow::Continue(())
+                visitor: &mut #traverser
+            ) -> ::core::ops::ControlFlow<#traverser::BreakTy> {
+                #body
+                ::core::ops::ControlFlow::Continue(())
             }
         }
     }
 }
 
+impl Interner<'_> {
+    /// We consider a type to be internable if it references either a generic type parameter or,
+    /// if the interner is `TyCtxt<'tcx>`, the `'tcx` lifetime.
+    fn type_of<'a>(
+        &self,
+        referenced_ty_params: &[&Ident],
+        fields: impl IntoIterator<Item = &'a Field>,
+    ) -> Type {
+        use visit::Visit;
+
+        struct Info<'a> {
+            interner: &'a Lifetime,
+            contains_interner: bool,
+        }
+
+        impl Visit<'_> for Info<'_> {
+            fn visit_lifetime(&mut self, i: &Lifetime) {
+                if i == self.interner {
+                    self.contains_interner = true;
+                } else {
+                    visit::visit_lifetime(self, i)
+                }
+            }
+        }
+
+        if !referenced_ty_params.is_empty() {
+            Generic
+        } else if let Some(interner) = &self.0 && fields.into_iter().any(|field| {
+            let mut info = Info { interner, contains_interner: false };
+            info.visit_type(&field.ty);
+            info.contains_interner
+        }) {
+            NotGeneric
+        } else {
+            Trivial
+        }
+    }
+}
+
 pub fn traversable_derive<T: Traversable>(
     mut structure: synstructure::Structure<'_>,
 ) -> TokenStream {
-    if let syn::Data::Union(_) = structure.ast().data {
-        panic!("cannot derive on union")
-    }
+    let ast = structure.ast();
+
+    let interner = Interner::resolve(&ast.generics);
+    let traverser = gen_param("T", &ast.generics);
+    let traversable = T::traversable(&interner);
 
-    structure.add_bounds(synstructure::AddBounds::Generics);
+    structure.underscore_const(true);
+    structure.add_bounds(synstructure::AddBounds::None);
     structure.bind_with(|_| synstructure::BindStyle::Move);
 
-    if !structure.ast().generics.lifetimes().any(|lt| lt.lifetime.ident == "tcx") {
+    if interner.0.is_none() {
         structure.add_impl_generic(parse_quote! { 'tcx });
     }
 
-    let arms = T::arms(&mut structure);
-    structure.bound_impl(T::traversable(), T::impl_body(arms))
+    // If our derived implementation will be generic over the traversable type, then we must
+    // constrain it to only those generic combinations that satisfy the traversable trait's
+    // supertraits.
+    if ast.generics.type_params().next().is_some() {
+        let supertraits = T::supertraits(&interner);
+        structure.add_where_predicate(parse_quote! { Self: #supertraits });
+    }
+
+    // We add predicates to each generic field type, rather than to our generic type parameters.
+    // This results in a "perfect derive", but it can result in trait solver cycles if any type
+    // parameters are involved in recursive type definitions; fortunately that is not the case (yet).
+    let mut predicates = HashSet::new();
+    let arms = structure.each_variant(|variant| {
+        T::arm(variant, |bind| {
+            let ast = bind.ast();
+            let field_ty = interner.type_of(&bind.referenced_ty_params(), [ast]);
+            if field_ty == Generic {
+                predicates.insert(ast.ty.clone());
+            }
+            T::traverse(bind.into_token_stream(), field_ty == Trivial)
+        })
+    });
+    // the order in which `where` predicates appear in rust source is irrelevant
+    #[allow(rustc::potential_query_instability)]
+    for ty in predicates {
+        structure.add_where_predicate(parse_quote! { #ty: #traversable });
+    }
+    let body = quote! { match self { #arms } };
+
+    structure.bound_impl(traversable, T::impl_body(interner, traverser, body))
 }