Sort synthetic impls bounds before rendering

This removes the implicit dependency on the iteration
order of FxHashMap

diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs
index 5951af6d70e802392ce2c64bd6c4c1b5d36c6507..f1bba0e836189ff4383b2f20223c69e619705f6e 100644 (file)
--- a/src/librustdoc/clean/auto_trait.rs
+++ b/src/librustdoc/clean/auto_trait.rs
@@ -1354,12 +1354,57 @@ fn param_env_to_generics<'b, 'c, 'cx>(
             }
         }
 
+        self.sort_where_predicates(&mut existing_predicates);
+
         Generics {
             params: generic_params,
             where_predicates: existing_predicates,
         }
     }
 
+    // Ensure that the predicates are in a consistent order. The precise
+    // ordering doesn't actually matter, but it's important that
+    // a given set of predicates always appears in the same order -
+    // both for visual consistency between 'rustdoc' runs, and to
+    // make writing tests much easier
+    fn sort_where_predicates(&self, predicates: &mut Vec<WherePredicate>) {
+        // We should never have identical bounds - and if we do,
+        // they're visually identical as well. Therefore, using
+        // an unstable sort is fine.
+        predicates.sort_unstable_by(|first, second| {
+            // This might look horrendously hacky, but it's actually not that bad.
+            //
+            // For performance reasons, we use several different FxHashMaps
+            // in the process of computing the final set of where predicates.
+            // However, the iteration order of a HashMap is completely unspecified.
+            // In fact, the iteration of an FxHashMap can even vary between platforms,
+            // since FxHasher has different behavior for 32-bit and 64-bit platforms.
+            //
+            // Obviously, it's extremely undesireable for documentation rendering
+            // to be depndent on the platform it's run on. Apart from being confusing
+            // to end users, it makes writing tests much more difficult, as predicates
+            // can appear in any order in the final result.
+            //
+            // To solve this problem, we sort WherePredicates by their Debug
+            // string. The thing to keep in mind is that we don't really
+            // care what the final order is - we're synthesizing an impl
+            // ourselves, so any order can be considered equally valid.
+            // By sorting the predicates, however, we ensure that for
+            // a given codebase, all auto-trait impls always render
+            // in exactly the same way.
+            //
+            // Using the Debug impementation for sorting prevents
+            // us from needing to write quite a bit of almost
+            // entirely useless code (e.g. how should two
+            // Types be sorted relative to each other).
+            // This approach is probably somewhat slower, but
+            // the small number of items involved (impls
+            // rarely have more than a few bounds) means
+            // that it shouldn't matter in practice.
+            format!("{:?}", first).cmp(&format!("{:?}", second))
+        });
+    }
+
     fn is_fn_ty(&self, tcx: &TyCtxt, ty: &Type) -> bool {
         match &ty {
             &&Type::ResolvedPath { ref did, .. } => {

diff --git a/src/test/rustdoc/synthetic_auto/complex.rs b/src/test/rustdoc/synthetic_auto/complex.rs
index 81259a50108bdb6ee9929c8993f7b182c62c5a5a..531798c30c656852ec5f8397c32210dcf6b103c0 100644 (file)
--- a/src/test/rustdoc/synthetic_auto/complex.rs
+++ b/src/test/rustdoc/synthetic_auto/complex.rs
@@ -31,8 +31,8 @@ pub struct Foo<T> {
 
 // @has complex/struct.NotOuter.html
 // @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]/*/code' "impl<'a, T, K: \
-// ?Sized> Send for NotOuter<'a, T, K> where 'a: 'static, K: for<'b> Fn((&'b bool, &'a u8)) \
-// -> &'b i8, <T as MyTrait<'a>>::MyItem: Copy, T: MyTrait<'a>"
+// ?Sized> Send for NotOuter<'a, T, K> where K: for<'b> Fn((&'b bool, &'a u8)) \
+// -> &'b i8, T: MyTrait<'a>, <T as MyTrait<'a>>::MyItem: Copy, 'a: 'static"
 
 pub use foo::{Foo, Inner as NotInner, MyTrait as NotMyTrait, Outer as NotOuter};
 

diff --git a/src/test/rustdoc/synthetic_auto/lifetimes.rs b/src/test/rustdoc/synthetic_auto/lifetimes.rs
index 2f92627f9546c3582cafefbda49fdb69d93fde01..272925e5db5424d7a414a2d0a1e94bbea67b888f 100644 (file)
--- a/src/test/rustdoc/synthetic_auto/lifetimes.rs
+++ b/src/test/rustdoc/synthetic_auto/lifetimes.rs
@@ -19,7 +19,7 @@ unsafe impl<'a, T> Send for Inner<'a, T>
 
 // @has lifetimes/struct.Foo.html
 // @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]/*/code' "impl<'c, K> Send \
-// for Foo<'c, K> where 'c: 'static, K: for<'b> Fn(&'b bool) -> &'c u8"
+// for Foo<'c, K> where K: for<'b> Fn(&'b bool) -> &'c u8, 'c: 'static"
 //
 // @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]/*/code' "impl<'c, K> Sync \
 // for Foo<'c, K> where K: Sync"

diff --git a/src/test/rustdoc/synthetic_auto/project.rs b/src/test/rustdoc/synthetic_auto/project.rs
index e1b8621ff6dccf2f80974c20bc8d7e1557a82cca..977607fb148264652908b8f4b26a981d1c60b1db 100644 (file)
--- a/src/test/rustdoc/synthetic_auto/project.rs
+++ b/src/test/rustdoc/synthetic_auto/project.rs
@@ -34,10 +34,10 @@ unsafe impl<'a, T> Sync for Inner<'a, T>
 
 // @has project/struct.Foo.html
 // @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]/*/code' "impl<'c, K> Send \
-// for Foo<'c, K> where 'c: 'static, K: MyTrait<MyItem = bool>"
+// for Foo<'c, K> where K: MyTrait<MyItem = bool>, 'c: 'static"
 //
 // @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]/*/code' "impl<'c, K> Sync \
-// for Foo<'c, K> where 'c: 'static, K: MyTrait, <K as MyTrait>::MyItem: OtherTrait"
+// for Foo<'c, K> where K: MyTrait, <K as MyTrait>::MyItem: OtherTrait, 'c: 'static,"
 pub struct Foo<'c, K: 'c> {
     inner_field: Inner<'c, K>,
 }