Automatically skip traversal of boring fields

A trait, `rustc_type_ir::SkipTraversalAutoImplOnly`, is auto-implemented
for types that do not contain anything that may be of interest to
traversers (folders and visitors).  "Auto-deref specialisation" is then
used in the derive macros to avoid traversing fields of such types
without requiring them to implement the traversable traits.

Author: eggyal

compiler/rustc_macros/src/lib.rs

@@ -120,17 +120,26 @@ 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, attributes(skip_traversal)] =>
     /// 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).
+    /// type if it has one. Fields of non-generic types that do not contain anything that may be of
+    /// interest to folders will automatically be left unchanged whether the type implements
+    /// `TypeFoldable` or not; the same behaviour can be achieved for fields of generic types by
+    /// applying `#[skip_traversal]` to the field definition (or even to a variant definition if it
+    /// should apply to all fields therein), but the derived implementation will only be applicable
+    /// to concrete types where such annotated fields do not contain anything that may be of
+    /// interest to folders (thus preventing fields from being left unchanged erroneously).
+    ///
+    /// In some rare situations, it may be necessary for `TypeFoldable` to be implemented for types
+    /// that do not contain anything of interest to folders. Whilst the macro expansion in such
+    /// cases would (as described above) result in folds that merely reconstruct `self`, this is
+    /// accomplished via method calls that might not get optimised away. One can therefore annotate
+    /// the type itself with `#[skip_traversal]` to immediately return `self` instead; as above,
+    /// such derived implementations are only applicable if the annotated type does not contain
+    /// anything that may be of interest to a folder.
     ///
     /// 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
@@ -146,17 +155,26 @@ decl_derive!(
     traversable::traversable_derive::<traversable::Foldable>
 );
 decl_derive!(
-    [TypeVisitable, attributes(type_visitable)] =>
+    [TypeVisitable, attributes(skip_traversal)] =>
     /// 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).
+    /// `TypeVisitable` implementation for its type if it has one. Fields of non-generic types that
+    /// do not contain anything that may be of interest to visitors will automatically be skipped
+    /// whether the type implements `TypeVisitable` or not; the same behaviour can be achieved for
+    /// fields of generic types by applying `#[skip_traversal]` to the field definition (or even to
+    /// a variant definition if it should apply to all fields therein), but the derived
+    /// implementation will only be applicable to concrete types where such annotated fields do not
+    /// contain anything that may be of interest to visitors (thus preventing fields from being so
+    /// skipped erroneously).
+    ///
+    /// In some rare situations, it may be necessary for `TypeVisitable` to be implemented for
+    /// types that do not contain anything of interest to visitors. Whilst the macro expansion in
+    /// such cases would (as described above) result in visits that skip all fields, this is
+    /// accomplished via method calls that might not get optimised away. One can therefore annotate
+    /// the type itself with `#[skip_traversal]` to immediately return `Continue(())` instead; as
+    /// above, such derived implementations are only applicable if the annotated type does not
+    /// contain anything that may 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,106 +1,141 @@
-use proc_macro2::TokenStream;
-use quote::{quote, ToTokens};
-use syn::{parse_quote, Attribute, Meta, NestedMeta};
+use proc_macro2::{Span, TokenStream};
+use quote::{quote, quote_spanned, ToTokens};
+use std::collections::HashMap;
+use syn::{parse_quote, spanned::Spanned, Attribute, Generics, Ident, Lifetime, LifetimeDef};
+
+/// 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())
+}
+
+/// 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 gen_interner(structure: &mut synstructure::Structure<'_>) -> TokenStream {
+    let lt = structure
+        .ast()
+        .generics
+        .lifetimes()
+        .find_map(|def| (def.lifetime.ident == "tcx").then_some(&def.lifetime))
+        .cloned()
+        .unwrap_or_else(|| {
+            let tcx: Lifetime = parse_quote! { 'tcx };
+            structure.add_impl_generic(LifetimeDef::new(tcx.clone()).into());
+            tcx
+        });
+
+    quote! { ::rustc_middle::ty::TyCtxt<#lt> }
+}
+
+/// Returns the `Span` of the first `#[skip_traversal]` attribute in `attrs`.
+fn find_skip_traversal_attribute(attrs: &[Attribute]) -> Option<Span> {
+    attrs.iter().find(|&attr| *attr == parse_quote! { #[skip_traversal] }).map(Spanned::span)
+}
 
 pub struct Foldable;
 pub struct Visitable;
 
 /// An abstraction over traversable traits.
 pub trait Traversable {
-    /// The trait that this `Traversable` represents.
-    fn traversable() -> TokenStream;
+    /// The trait that this `Traversable` represents, parameterised by `interner`.
+    fn traversable(interner: &impl ToTokens) -> TokenStream;
+
+    /// Any supertraits that this trait is required to implement.
+    fn supertraits(interner: &impl ToTokens) -> TokenStream;
 
-    /// The `match` arms for a traversal of this type.
-    fn arms(structure: &mut synstructure::Structure<'_>) -> TokenStream;
+    /// A (`noop`) traversal of this trait upon the `bind` expression.
+    fn traverse(bind: TokenStream, noop: bool) -> TokenStream;
 
-    /// The body of an implementation given the match `arms`.
-    fn impl_body(arms: impl ToTokens) -> 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: impl ToTokens,
+        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 traversable(interner: &impl ToTokens) -> TokenStream {
+        quote! { ::rustc_middle::ty::fold::TypeFoldable<#interner> }
     }
-    fn arms(structure: &mut synstructure::Structure<'_>) -> TokenStream {
-        structure.each_variant(|vi| {
-            let bindings = vi.bindings();
-            vi.construct(|_, index| {
-                let bind = &bindings[index];
-
-                // retain value of fields with #[type_foldable(identity)]
-                let fixed = bind
-                    .ast()
-                    .attrs
-                    .iter()
-                    .map(Attribute::parse_meta)
-                    .filter_map(Result::ok)
-                    .flat_map(|attr| match attr {
-                        Meta::List(list) if list.path.is_ident("type_foldable") => list.nested,
-                        _ => Default::default(),
-                    })
-                    .any(|nested| match nested {
-                        NestedMeta::Meta(Meta::Path(path)) => path.is_ident("identity"),
-                        _ => false,
-                    });
-
-                if fixed {
-                    bind.to_token_stream()
-                } else {
-                    quote! {
-                        ::rustc_middle::ty::fold::TypeFoldable::try_fold_with(#bind, __folder)?
-                    }
-                }
-            })
-        })
+    fn supertraits(interner: &impl ToTokens) -> TokenStream {
+        Visitable::traversable(interner)
+    }
+    fn traverse(bind: TokenStream, noop: bool) -> TokenStream {
+        if noop {
+            bind
+        } else {
+            quote! { ::rustc_middle::ty::prefer_noop_traversal_if_applicable!(#bind.try_fold_with(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(arms: impl ToTokens) -> TokenStream {
+    fn impl_body(
+        interner: impl ToTokens,
+        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: &impl ToTokens) -> TokenStream {
+        quote! { ::rustc_middle::ty::visit::TypeVisitable<#interner> }
     }
-    fn arms(structure: &mut synstructure::Structure<'_>) -> TokenStream {
-        // ignore fields with #[type_visitable(ignore)]
-        structure.filter(|bi| {
-            !bi.ast()
-                .attrs
-                .iter()
-                .map(Attribute::parse_meta)
-                .filter_map(Result::ok)
-                .flat_map(|attr| match attr {
-                    Meta::List(list) if list.path.is_ident("type_visitable") => list.nested,
-                    _ => Default::default(),
-                })
-                .any(|nested| match nested {
-                    NestedMeta::Meta(Meta::Path(path)) => path.is_ident("ignore"),
-                    _ => false,
-                })
-        });
-
-        structure.each(|bind| {
-            quote! {
-                ::rustc_middle::ty::visit::TypeVisitable::visit_with(#bind, __visitor)?;
-            }
-        })
+    fn supertraits(_: &impl ToTokens) -> TokenStream {
+        quote! { ::core::clone::Clone + ::core::fmt::Debug }
+    }
+    fn traverse(bind: TokenStream, noop: bool) -> TokenStream {
+        if noop {
+            quote! {}
+        } else {
+            quote! { ::rustc_middle::ty::prefer_noop_traversal_if_applicable!(#bind.visit_with(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: impl ToTokens,
+        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(())
             }
         }
     }
@@ -109,17 +144,78 @@ impl Traversable for Visitable {
 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 skip_traversal = quote! { ::rustc_middle::ty::SkipTraversalAutoImplOnly };
+
+    let interner = gen_interner(&mut structure);
+    let traverser = gen_param("T", &structure.ast().generics);
+    let traversable = T::traversable(&interner);
+    let ast = structure.ast();
 
-    structure.add_bounds(synstructure::AddBounds::Generics);
+    structure.add_bounds(synstructure::AddBounds::None);
     structure.bind_with(|_| synstructure::BindStyle::Move);
 
-    if !structure.ast().generics.lifetimes().any(|lt| lt.lifetime.ident == "tcx") {
-        structure.add_impl_generic(parse_quote! { 'tcx });
+    // 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.
+    let is_generic = !ast.generics.params.is_empty();
+    if is_generic {
+        let supertraits = T::supertraits(&interner);
+        structure.add_where_predicate(parse_quote! { Self: #supertraits });
     }
 
-    let arms = T::arms(&mut structure);
-    structure.bound_impl(T::traversable(), T::impl_body(arms))
+    let body = if let Some(_span) = find_skip_traversal_attribute(&ast.attrs) {
+        if !is_generic {
+            // FIXME: spanned error causes ICE: "suggestion must not have overlapping parts"
+            return quote!({
+                ::core::compile_error!("non-generic types are automatically skipped where possible");
+            });
+        }
+        structure.add_where_predicate(parse_quote! { Self: #skip_traversal });
+        T::traverse(quote! { self }, true)
+    } else {
+        // We add predicates to each generic field type, rather than to our generic type parameters.
+        // This results in a "perfect derive" that avoids having to propagate `#[skip_traversal]` annotations
+        // into wrapping types, 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 = HashMap::new();
+        let arms = structure.each_variant(|variant| {
+            let skipped_variant_span = find_skip_traversal_attribute(&variant.ast().attrs);
+            if variant.referenced_ty_params().is_empty() {
+                if let Some(span) = skipped_variant_span {
+                    return quote_spanned!(span => {
+                        ::core::compile_error!("non-generic variants are automatically skipped where possible");
+                    });
+                }
+            }
+            T::arm(variant, |bind| {
+                let ast = bind.ast();
+                let skipped_span = skipped_variant_span.or_else(|| find_skip_traversal_attribute(&ast.attrs));
+                if bind.referenced_ty_params().is_empty() {
+                    if skipped_variant_span.is_none() && let Some(span) = skipped_span {
+                        return quote_spanned!(span => {
+                            ::core::compile_error!("non-generic fields are automatically skipped where possible");
+                        });
+                    }
+                } else if let Some(prev) = predicates.insert(ast.ty.clone(), skipped_span) && let Some(span) = prev.xor(skipped_span) {
+                    // It makes no sense to allow this. Skipping the field requires that its type impls the `SkipTraversalAutoImplOnly`
+                    // auto-trait, which means that it does not contain anything interesting to traversers; not also skipping all other
+                    // fields of identical type is indicative of a likely erroneous assumption on the author's part--especially since
+                    // such other (unannotated) fields will be skipped anyway, as the noop traversal takes precedence.
+                    return quote_spanned!(span => {
+                        ::core::compile_error!("generic field skipped here, but another field of the same type is not skipped");
+                    });
+                }
+                T::traverse(bind.into_token_stream(), skipped_span.is_some())
+            })
+        });
+        // the order in which `where` predicates appear in rust source is irrelevant
+        #[allow(rustc::potential_query_instability)]
+        for (ty, skip) in predicates {
+            let bound = if skip.is_some() { &skip_traversal } else { &traversable };
+            structure.add_where_predicate(parse_quote! { #ty: #bound });
+        }
+        quote! { match self { #arms } }
+    };
+
+    structure.bound_impl(traversable, T::impl_body(interner, traverser, body))
 }

compiler/rustc_middle/src/mir/query.rs

@@ -161,8 +161,6 @@ pub struct GeneratorLayout<'tcx> {
     /// Which saved locals are storage-live at the same time. Locals that do not
     /// have conflicts with each other are allowed to overlap in the computed
     /// layout.
-    #[type_foldable(identity)]
-    #[type_visitable(ignore)]
     pub storage_conflicts: BitMatrix<GeneratorSavedLocal, GeneratorSavedLocal>,
 }