From 57e3df3f879090207f5a128a497b66916aeabb6b Mon Sep 17 00:00:00 2001 From: Phlosioneer Date: Mon, 19 Mar 2018 23:35:23 -0400 Subject: [PATCH 1/4] Fix ordering of auto-generated trait bounds in rustdoc output While the order of the where clauses was deterministic, the ordering of bounds and lifetimes was not. This made the order flip- flop randomly when new traits and impls were added to libstd. This PR makes the ordering of bounds and lifetimes deterministic, and re-enables the test that was causing the issue. Fixes #49123 --- src/librustdoc/clean/auto_trait.rs | 102 ++++++++++++------ .../rustdoc/synthetic_auto/no-redundancy.rs | 2 - 2 files changed, 67 insertions(+), 37 deletions(-) diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs index 370fc9bbca243..918bc1df0b1d6 100644 --- a/src/librustdoc/clean/auto_trait.rs +++ b/src/librustdoc/clean/auto_trait.rs @@ -9,6 +9,7 @@ // except according to those terms. use rustc::ty::TypeFoldable; +use std::fmt::Debug; use super::*; @@ -1081,18 +1082,25 @@ impl<'a, 'tcx, 'rcx> AutoTraitFinder<'a, 'tcx, 'rcx> { return None; } + let mut bounds_vec = bounds.into_iter().collect(); + self.sort_where_bounds(&mut bounds_vec); + Some(WherePredicate::BoundPredicate { ty, - bounds: bounds.into_iter().collect(), + bounds: bounds_vec, }) }) .chain( lifetime_to_bounds .into_iter() .filter(|&(_, ref bounds)| !bounds.is_empty()) - .map(|(lifetime, bounds)| WherePredicate::RegionPredicate { - lifetime, - bounds: bounds.into_iter().collect(), + .map(|(lifetime, bounds)| { + let mut bounds_vec = bounds.into_iter().collect(); + self.sort_where_lifetimes(&mut bounds_vec); + WherePredicate::RegionPredicate { + lifetime, + bounds: bounds_vec, + } }), ) .collect() @@ -1372,40 +1380,64 @@ impl<'a, 'tcx, 'rcx> AutoTraitFinder<'a, 'tcx, 'rcx> { // 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) { + #[inline] + fn sort_where_predicates(&self, mut predicates: &mut Vec) { // 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. + self.unstable_debug_sort(&mut predicates); + } + + // Ensure that the bounds are in a consistent order. The precise + // ordering doesn't actually matter, but it's important that + // a given set of bounds always appears in the same order - + // both for visual consistency between 'rustdoc' runs, and to + // make writing tests much easier + #[inline] + fn sort_where_bounds(&self, mut bounds: &mut Vec) { + // We should never have identical bounds - and if we do, + // they're visually identical as well. Therefore, using + // an unstable sort is fine. + self.unstable_debug_sort(&mut bounds); + } + + #[inline] + fn sort_where_lifetimes(&self, mut bounds: &mut Vec) { + // We should never have identical bounds - and if we do, + // they're visually identical as well. Therefore, using + // an unstable sort is fine. + self.unstable_debug_sort(&mut bounds); + } + + // 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 and TyParamBounds + // 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 or bound + // ourselves, so any order can be considered equally valid. By sorting the + // predicates and bounds, 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). It also allows us to solve the + // problem for both WherePredicates and TyParamBounds at the same time. 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. + fn unstable_debug_sort(&self, vec: &mut Vec) { + vec.sort_unstable_by(|first, second| { format!("{:?}", first).cmp(&format!("{:?}", second)) }); } diff --git a/src/test/rustdoc/synthetic_auto/no-redundancy.rs b/src/test/rustdoc/synthetic_auto/no-redundancy.rs index daa91c3e12bb8..0b37f2ed31790 100644 --- a/src/test/rustdoc/synthetic_auto/no-redundancy.rs +++ b/src/test/rustdoc/synthetic_auto/no-redundancy.rs @@ -8,8 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// ignore-test - pub struct Inner { field: T, } From 7daf3f941af996d6b816aa778aab4b9348d26703 Mon Sep 17 00:00:00 2001 From: Phlosioneer Date: Tue, 20 Mar 2018 01:02:15 -0400 Subject: [PATCH 2/4] Fix tidy trailing whitespace --- src/librustdoc/clean/auto_trait.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs index 918bc1df0b1d6..9ff3d25a45ae4 100644 --- a/src/librustdoc/clean/auto_trait.rs +++ b/src/librustdoc/clean/auto_trait.rs @@ -1387,7 +1387,7 @@ impl<'a, 'tcx, 'rcx> AutoTraitFinder<'a, 'tcx, 'rcx> { // an unstable sort is fine. self.unstable_debug_sort(&mut predicates); } - + // Ensure that the bounds are in a consistent order. The precise // ordering doesn't actually matter, but it's important that // a given set of bounds always appears in the same order - @@ -1400,7 +1400,7 @@ impl<'a, 'tcx, 'rcx> AutoTraitFinder<'a, 'tcx, 'rcx> { // an unstable sort is fine. self.unstable_debug_sort(&mut bounds); } - + #[inline] fn sort_where_lifetimes(&self, mut bounds: &mut Vec) { // We should never have identical bounds - and if we do, From 8d9774713df3b5d0b3a1bdbb4ecd55658d401d75 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 19 Mar 2018 21:57:41 +0300 Subject: [PATCH 3/4] Update Cargo to fix regression This should fix regressions in Cargo after swithing to clap: * If an external subcommand name was close to built-in one, clap errored (fixed by updating clap version) * External subcomands didn't received their name as a first arg --- src/Cargo.lock | 14 +++++++------- src/tools/cargo | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Cargo.lock b/src/Cargo.lock index 26508dec4bba2..675457905eade 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -184,7 +184,7 @@ version = "0.27.0" dependencies = [ "atty 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", "bufstream 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", - "clap 2.31.1 (registry+https://github.com/rust-lang/crates.io-index)", + "clap 2.31.2 (registry+https://github.com/rust-lang/crates.io-index)", "core-foundation 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", "crates-io 0.16.0", "crossbeam 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -273,7 +273,7 @@ dependencies = [ [[package]] name = "clap" -version = "2.31.1" +version = "2.31.2" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "ansi_term 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -840,7 +840,7 @@ version = "0.1.0" name = "installer" version = "0.0.0" dependencies = [ - "clap 2.31.1 (registry+https://github.com/rust-lang/crates.io-index)", + "clap 2.31.2 (registry+https://github.com/rust-lang/crates.io-index)", "error-chain 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", "flate2 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1033,7 +1033,7 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "chrono 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", - "clap 2.31.1 (registry+https://github.com/rust-lang/crates.io-index)", + "clap 2.31.2 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.5.5 (registry+https://github.com/rust-lang/crates.io-index)", "error-chain 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", "handlebars 0.29.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1322,7 +1322,7 @@ name = "racer" version = "2.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "clap 2.31.1 (registry+https://github.com/rust-lang/crates.io-index)", + "clap 2.31.2 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1533,7 +1533,7 @@ dependencies = [ name = "rustbook" version = "0.1.0" dependencies = [ - "clap 2.31.1 (registry+https://github.com/rust-lang/crates.io-index)", + "clap 2.31.2 (registry+https://github.com/rust-lang/crates.io-index)", "mdbook 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -2699,7 +2699,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum cc 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)" = "fedf677519ac9e865c4ff43ef8f930773b37ed6e6ea61b6b83b400a7b5787f49" "checksum cfg-if 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "d4c819a1287eb618df47cc647173c5c4c66ba19d888a6e50d605672aed3140de" "checksum chrono 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "7c20ebe0b2b08b0aeddba49c609fe7957ba2e33449882cb186a180bc60682fa9" -"checksum clap 2.31.1 (registry+https://github.com/rust-lang/crates.io-index)" = "5dc18f6f4005132120d9711636b32c46a233fad94df6217fa1d81c5e97a9f200" +"checksum clap 2.31.2 (registry+https://github.com/rust-lang/crates.io-index)" = "f0f16b89cbb9ee36d87483dc939fe9f1e13c05898d56d7b230a0d4dff033a536" "checksum cmake 0.1.29 (registry+https://github.com/rust-lang/crates.io-index)" = "56d741ea7a69e577f6d06b36b7dff4738f680593dc27a701ffa8506b73ce28bb" "checksum commoncrypto 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d056a8586ba25a1e4d61cb090900e495952c7886786fc55f909ab2f819b69007" "checksum commoncrypto-sys 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1fed34f46747aa73dfaa578069fd8279d2818ade2b55f38f22a9401c7f4083e2" diff --git a/src/tools/cargo b/src/tools/cargo index d6c3983fe3bd8..d10ec661b0642 160000 --- a/src/tools/cargo +++ b/src/tools/cargo @@ -1 +1 @@ -Subproject commit d6c3983fe3bd8fa06b54712e53fb23645598188b +Subproject commit d10ec661b06420654bbc4ed0ccd32295698aa1dc From 20e65f11f3bb0538c5676425e74b593676bd0f12 Mon Sep 17 00:00:00 2001 From: kennytm Date: Wed, 21 Mar 2018 02:59:07 +0800 Subject: [PATCH 4/4] Apply a fix to travis-ci/dpl#788 manually until dpl 1.9.5 is released. --- .travis.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.travis.yml b/.travis.yml index 41ea0c9afa87c..b2aba305aedc4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -318,6 +318,8 @@ before_deploy: deploy: - provider: s3 + edge: + branch: s3-eager-autoload bucket: rust-lang-ci2 skip_cleanup: true local_dir: deploy @@ -334,6 +336,8 @@ deploy: # this is the same as the above deployment provider except that it uploads to # a slightly different directory and has a different trigger - provider: s3 + edge: + branch: s3-eager-autoload bucket: rust-lang-ci2 skip_cleanup: true local_dir: deploy @@ -351,6 +355,8 @@ deploy: # try branch. Travis does not appear to provide a way to use "or" in these # conditions. - provider: s3 + edge: + branch: s3-eager-autoload bucket: rust-lang-ci2 skip_cleanup: true local_dir: deploy @@ -365,6 +371,8 @@ deploy: condition: $DEPLOY = 1 - provider: s3 + edge: + branch: s3-eager-autoload bucket: rust-lang-ci2 skip_cleanup: true local_dir: deploy