[feature] #7: predictable ordering of generic arguments

mversic · Jan 22, 2024 · 0fe5f52 · 0fe5f52
1 parent 4df7d3e
commit 0fe5f52
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 37 deletions.
diff --git a/src/disjoint.rs b/src/disjoint.rs
@@ -52,17 +52,17 @@ pub fn gen(impl_group_idx: usize, mut impls: Vec<ItemImpl>) -> Vec<ItemImpl> {
     impls
         .iter_mut()
         .zip(assoc_bounds)
-        .for_each(|(impl_, assoc_bound)| {
+        .for_each(|(impl_, assoc_bounds)| {
             let trait_ = &mut impl_.trait_.as_mut().unwrap().1;
             let path = trait_.segments.last_mut().unwrap();
 
             match &mut path.arguments {
                 syn::PathArguments::None => {
-                    let bracketed = syn::parse_quote! { <#( #assoc_bound ),*> };
+                    let bracketed = syn::parse_quote! { <#( #assoc_bounds ),*> };
                     path.arguments = syn::PathArguments::AngleBracketed(bracketed)
                 }
                 syn::PathArguments::AngleBracketed(bracketed) => {
-                    bracketed.args = assoc_bound
+                    bracketed.args = assoc_bounds
                         .into_iter()
                         .map::<syn::GenericArgument, _>(|param| syn::parse_quote!(#param))
                         .chain(core::mem::take(&mut bracketed.args))

diff --git a/src/lib.rs b/src/lib.rs
@@ -2,9 +2,10 @@ mod disjoint;
 mod main_trait;
 mod validate;
 
+use param::get_param_ident;
 use proc_macro::TokenStream;
 use proc_macro2::TokenStream as TokenStream2;
-use proc_macro_error::{abort, proc_macro_error, OptionExt};
+use proc_macro_error::{abort, proc_macro_error};
 use quote::{format_ident, quote};
 use rustc_hash::{FxHashMap, FxHashSet};
 use syn::visit::Visit;
@@ -218,20 +219,31 @@ struct AssocBounds<'ast> {
 
 impl<'ast> AssocBounds<'ast> {
     fn find(impl_group: &'ast [ItemImpl]) -> Self {
-        let mut type_param_bounds: FxHashSet<_> = if let Some(first_impl) = impl_group.first() {
-            let mut visitor = TraitBoundsVisitor::new();
-            visitor.visit_generics(&first_impl.generics);
-            visitor.type_param_bounds
-        } else {
-            FxHashSet::default()
-        };
+        let first_type_param_bounds = impl_group
+            .first()
+            .map(|first_impl| {
+                let mut visitor = TraitBoundsVisitor::new();
+                visitor.visit_generics(&first_impl.generics);
+                visitor.type_param_bounds
+            })
+            .unwrap_or_default();
 
-        let assoc_bounds: Vec<_> = impl_group
+        let mut type_param_bounds = first_type_param_bounds
+            .iter()
+            .cloned()
+            .collect::<FxHashSet<_>>();
+
+        let assoc_bounds = impl_group
             .iter()
             .map(|impl_| {
                 let mut visitor = TraitBoundsVisitor::new();
                 visitor.visit_generics(&impl_.generics);
-                type_param_bounds = &type_param_bounds & &visitor.type_param_bounds;
+
+                type_param_bounds = &type_param_bounds
+                    & &visitor
+                        .type_param_bounds
+                        .into_iter()
+                        .collect::<FxHashSet<_>>();
 
                 visitor.assoc_bounds
             })
@@ -249,22 +261,41 @@ impl<'ast> AssocBounds<'ast> {
 
                         None
                     })
-                    .collect::<FxHashMap<_, _>>()
+                    .collect::<Vec<_>>()
             })
-            .collect();
-
-        let assoc_bound_idents = assoc_bounds
-            .iter()
-            .fold(FxHashSet::default(), |mut acc, e| {
-                acc.extend(e.keys());
-                acc
-            })
-            .into_iter()
-            .collect();
+            .collect::<Vec<_>>();
+
+        let assoc_bound_idents = {
+            let mut seen_assoc_bound_idents = FxHashSet::default();
+            let mut seen_type_param_bounds = FxHashSet::default();
+
+            let assoc_bound_idents = assoc_bounds
+                .iter()
+                .flatten()
+                .map(|(id, _)| *id)
+                .filter(|&id| seen_assoc_bound_idents.insert(id))
+                .fold(FxHashMap::<_, Vec<_>>::default(), |mut acc, id| {
+                    acc.entry((id.0, id.1)).or_default().push(id);
+                    acc
+                });
+
+            first_type_param_bounds
+                .into_iter()
+                .filter(|&bound| {
+                    seen_type_param_bounds.insert(bound) && assoc_bound_idents.contains_key(&bound)
+                })
+                .flat_map(|type_param_bound| assoc_bound_idents.get(&type_param_bound).unwrap())
+                .cloned()
+                .collect()
+        };
 
         Self {
             assoc_bound_idents,
-            assoc_bounds,
+
+            assoc_bounds: assoc_bounds
+                .into_iter()
+                .map(|impl_assoc_bounds| impl_assoc_bounds.into_iter().collect())
+                .collect(),
         }
     }
 }
@@ -275,7 +306,7 @@ struct TraitBoundsVisitor<'ast> {
     /// Trait bound currently being visited
     curr_trait_bound: Option<TraitBound<'ast>>,
 
-    type_param_bounds: FxHashSet<(Bounded<'ast>, TraitBound<'ast>)>,
+    type_param_bounds: Vec<(Bounded<'ast>, TraitBound<'ast>)>,
     /// Collection of associated bounds for every implementation
     assoc_bounds: Vec<(AssocBoundIdent<'ast>, &'ast AssocBoundPayload)>,
 }
@@ -286,7 +317,7 @@ impl<'ast> TraitBoundsVisitor<'ast> {
             curr_type_param: None,
             curr_trait_bound: None,
 
-            type_param_bounds: FxHashSet::default(),
+            type_param_bounds: Vec::default(),
             assoc_bounds: Vec::default(),
         }
     }
@@ -308,10 +339,29 @@ impl<'ast> Visit<'ast> for TraitBoundsVisitor<'ast> {
         self.visit_generics(&node.generics);
     }
 
+    fn visit_generics(&mut self, node: &'ast syn::Generics) {
+        let mut params: Vec<_> = node
+            .params
+            .iter()
+            .map(|param| (get_param_ident(param), param))
+            .collect();
+
+        // NOTE: Iterate in predictable order
+        params.sort_by_key(|(ident, _)| *ident);
+
+        for (_, param) in params {
+            self.visit_generic_param(param);
+        }
+        if let Some(where_clause) = &node.where_clause {
+            self.visit_where_clause(where_clause);
+        }
+    }
+
     fn visit_type_param(&mut self, node: &'ast syn::TypeParam) {
         self.curr_type_param = Some((&node.ident).into());
         syn::visit::visit_type_param(self, node);
     }
+
     fn visit_predicate_type(&mut self, node: &'ast syn::PredicateType) {
         self.curr_type_param = Some((&node.bounded_ty).into());
         syn::visit::visit_predicate_type(self, node);
@@ -321,7 +371,7 @@ impl<'ast> Visit<'ast> for TraitBoundsVisitor<'ast> {
         self.curr_trait_bound = Some(TraitBound(&node.path));
         syn::visit::visit_trait_bound(self, node);
 
-        self.type_param_bounds.insert((
+        self.type_param_bounds.push((
             self.curr_type_param.unwrap(),
             self.curr_trait_bound.unwrap(),
         ));

diff --git a/src/validate.rs b/src/validate.rs
@@ -1,3 +1,5 @@
+use proc_macro_error::OptionExt;
+
 use super::*;
 
 // TODO: Check content of ItemImpl::items, not only idents for impls?

diff --git a/tests/multiple_associated_types.rs b/tests/multiple_associated_types.rs
@@ -31,15 +31,15 @@ disjoint_impls! {
         const NAME: &'static str;
     }
 
+    impl<T: Dispatch<Group1 = GroupB>> Kita for T {
+        const NAME: &'static str = "Blanket B*";
+    }
     impl<T: Dispatch<Group1 = GroupA, Group2 = GroupA>> Kita for T {
         const NAME: &'static str = "Blanket AA";
     }
     impl<T: Dispatch<Group1 = GroupA, Group2 = GroupB>> Kita for T {
         const NAME: &'static str = "Blanket AB";
     }
-    impl<T: Dispatch<Group1 = GroupB>> Kita for T {
-        const NAME: &'static str = "Blanket B*";
-    }
 }
 
 /*

diff --git a/tests/multiple_dispatch_traits.rs b/tests/multiple_dispatch_traits.rs
@@ -58,22 +58,22 @@ pub trait Kita {
 }
 
 const _: () = {
-    pub trait _Kita0<_0: ?Sized, _1: ?Sized> {
+    pub trait _Kita0<_ŠČ0: ?Sized, _ŠČ1: ?Sized> {
         const NAME: &'static str;
     }
 
-    impl<_0: Dispatch1<Group = GroupA> + Dispatch2<Group = GroupA>> _Kita0<GroupA, GroupA> for _0 {
+    impl<_ŠČ0: Dispatch1<Group = GroupA> + Dispatch2<Group = GroupA>> _Kita0<GroupA, GroupA> for _ŠČ0 {
         const NAME: &'static str = "Blanket AA";
     }
-    impl<_0: Dispatch1<Group = GroupA> + Dispatch2<Group = GroupB>> _Kita0<GroupB, GroupA> for _0 {
+    impl<_ŠČ0: Dispatch1<Group = GroupA> + Dispatch2<Group = GroupB>> _Kita0<GroupA, GroupB> for _ŠČ0 {
         const NAME: &'static str = "Blanket AB";
     }
-    impl<_0: Dispatch1<Group = GroupB>, _1: Dispatch2> _Kita0<<_1 as Dispatch>::Group, GroupB> for _0 {
+    impl<_ŠČ0: Dispatch1<Group = GroupB> + Dispatch2> _Kita0<GroupB, <_ŠČ0 as Dispatch2>::Group> for _ŠČ0 {
         const NAME: &'static str = "Blanket B*";
     }
 
-    impl<_0> Kita for _0 where _0: Dispatch2 + Dispatch1, Self: _Kita0<<_0 as Dispatch2>::Group, <_0 as Dispatch1>::Group> {
-        const NAME: &'static str = <Self as _Kita0<<_0 as Dispatch2>::Group, <_0 as Dispatch1>::Group>>::NAME;
+    impl<_ŠČ0> Kita for _ŠČ0 where _ŠČ0: Dispatch2 + Dispatch1, Self: _Kita0<<_ŠČ0 as Dispatch1>::Group, <_ŠČ0 as Dispatch2>::Group> {
+        const NAME: &'static str = <Self as _Kita0<<_ŠČ0 as Dispatch1>::Group, <_ŠČ0 as Dispatch2>::Group>>::NAME;
     }
 };
 */