Skip to content

Commit 44d07df

Browse files
committed
Sort synthetic impls bounds before rendering
This removes the implicit dependency on the iteration order of FxHashMap
1 parent 8788179 commit 44d07df

File tree

4 files changed

+50
-5
lines changed

4 files changed

+50
-5
lines changed

src/librustdoc/clean/auto_trait.rs

+45
Original file line numberDiff line numberDiff line change
@@ -1354,12 +1354,57 @@ impl<'a, 'tcx, 'rcx> AutoTraitFinder<'a, 'tcx, 'rcx> {
13541354
}
13551355
}
13561356

1357+
self.sort_where_predicates(&mut existing_predicates);
1358+
13571359
Generics {
13581360
params: generic_params,
13591361
where_predicates: existing_predicates,
13601362
}
13611363
}
13621364

1365+
// Ensure that the predicates are in a consistent order. The precise
1366+
// ordering doesn't actually matter, but it's important that
1367+
// a given set of predicates always appears in the same order -
1368+
// both for visual consistency between 'rustdoc' runs, and to
1369+
// make writing tests much easier
1370+
fn sort_where_predicates(&self, predicates: &mut Vec<WherePredicate>) {
1371+
// We should never have identical bounds - and if we do,
1372+
// they're visually identical as well. Therefore, using
1373+
// an unstable sort is fine.
1374+
predicates.sort_unstable_by(|first, second| {
1375+
// This might look horrendously hacky, but it's actually not that bad.
1376+
//
1377+
// For performance reasons, we use several different FxHashMaps
1378+
// in the process of computing the final set of where predicates.
1379+
// However, the iteration order of a HashMap is completely unspecified.
1380+
// In fact, the iteration of an FxHashMap can even vary between platforms,
1381+
// since FxHasher has different behavior for 32-bit and 64-bit platforms.
1382+
//
1383+
// Obviously, it's extremely undesireable for documentation rendering
1384+
// to be depndent on the platform it's run on. Apart from being confusing
1385+
// to end users, it makes writing tests much more difficult, as predicates
1386+
// can appear in any order in the final result.
1387+
//
1388+
// To solve this problem, we sort WherePredicates by their Debug
1389+
// string. The thing to keep in mind is that we don't really
1390+
// care what the final order is - we're synthesizing an impl
1391+
// ourselves, so any order can be considered equally valid.
1392+
// By sorting the predicates, however, we ensure that for
1393+
// a given codebase, all auto-trait impls always render
1394+
// in exactly the same way.
1395+
//
1396+
// Using the Debug impementation for sorting prevents
1397+
// us from needing to write quite a bit of almost
1398+
// entirely useless code (e.g. how should two
1399+
// Types be sorted relative to each other).
1400+
// This approach is probably somewhat slower, but
1401+
// the small number of items involved (impls
1402+
// rarely have more than a few bounds) means
1403+
// that it shouldn't matter in practice.
1404+
format!("{:?}", first).cmp(&format!("{:?}", second))
1405+
});
1406+
}
1407+
13631408
fn is_fn_ty(&self, tcx: &TyCtxt, ty: &Type) -> bool {
13641409
match &ty {
13651410
&&Type::ResolvedPath { ref did, .. } => {

src/test/rustdoc/synthetic_auto/complex.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ mod foo {
3131

3232
// @has complex/struct.NotOuter.html
3333
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]/*/code' "impl<'a, T, K: \
34-
// ?Sized> Send for NotOuter<'a, T, K> where 'a: 'static, K: for<'b> Fn((&'b bool, &'a u8)) \
35-
// -> &'b i8, <T as MyTrait<'a>>::MyItem: Copy, T: MyTrait<'a>"
34+
// ?Sized> Send for NotOuter<'a, T, K> where K: for<'b> Fn((&'b bool, &'a u8)) \
35+
// -> &'b i8, T: MyTrait<'a>, <T as MyTrait<'a>>::MyItem: Copy, 'a: 'static"
3636

3737
pub use foo::{Foo, Inner as NotInner, MyTrait as NotMyTrait, Outer as NotOuter};
3838

src/test/rustdoc/synthetic_auto/lifetimes.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ where
1919

2020
// @has lifetimes/struct.Foo.html
2121
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]/*/code' "impl<'c, K> Send \
22-
// for Foo<'c, K> where 'c: 'static, K: for<'b> Fn(&'b bool) -> &'c u8"
22+
// for Foo<'c, K> where K: for<'b> Fn(&'b bool) -> &'c u8, 'c: 'static"
2323
//
2424
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]/*/code' "impl<'c, K> Sync \
2525
// for Foo<'c, K> where K: Sync"

src/test/rustdoc/synthetic_auto/project.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@ where
3434

3535
// @has project/struct.Foo.html
3636
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]/*/code' "impl<'c, K> Send \
37-
// for Foo<'c, K> where 'c: 'static, K: MyTrait<MyItem = bool>"
37+
// for Foo<'c, K> where K: MyTrait<MyItem = bool>, 'c: 'static"
3838
//
3939
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]/*/code' "impl<'c, K> Sync \
40-
// for Foo<'c, K> where 'c: 'static, K: MyTrait, <K as MyTrait>::MyItem: OtherTrait"
40+
// for Foo<'c, K> where K: MyTrait, <K as MyTrait>::MyItem: OtherTrait, 'c: 'static,"
4141
pub struct Foo<'c, K: 'c> {
4242
inner_field: Inner<'c, K>,
4343
}

0 commit comments

Comments
 (0)