Skip to content

Commit

Permalink
Sort synthetic impls bounds before rendering
Browse files Browse the repository at this point in the history
This removes the implicit dependency on the iteration
order of FxHashMap
  • Loading branch information
Aaron1011 committed Feb 20, 2018
1 parent 8788179 commit 44d07df
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 5 deletions.
45 changes: 45 additions & 0 deletions src/librustdoc/clean/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1354,12 +1354,57 @@ impl<'a, 'tcx, 'rcx> AutoTraitFinder<'a, 'tcx, 'rcx> {
}
}

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, .. } => {
Expand Down
4 changes: 2 additions & 2 deletions src/test/rustdoc/synthetic_auto/complex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ mod foo {

// @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};

Expand Down
2 changes: 1 addition & 1 deletion src/test/rustdoc/synthetic_auto/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ where

// @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"
Expand Down
4 changes: 2 additions & 2 deletions src/test/rustdoc/synthetic_auto/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ where

// @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>,
}

0 comments on commit 44d07df

Please sign in to comment.