From f57cc8ca5cd344a6111650a8ba4744b4e4859da8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20BRANSTETT?= Date: Wed, 23 Feb 2022 15:44:57 +0100 Subject: [PATCH 1/9] Always evaluate all cfg predicate in all() and any() --- compiler/rustc_attr/src/builtin.rs | 12 ++++++++++-- src/test/ui/cfg/cfg-path-error.rs | 19 +++++++++++++++++++ src/test/ui/cfg/cfg-path-error.stderr | 26 ++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/cfg/cfg-path-error.rs create mode 100644 src/test/ui/cfg/cfg-path-error.stderr diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs index 49043e9f5f9d6..628da8b6bbb73 100644 --- a/compiler/rustc_attr/src/builtin.rs +++ b/compiler/rustc_attr/src/builtin.rs @@ -594,10 +594,18 @@ pub fn eval_condition( match cfg.name_or_empty() { sym::any => mis .iter() - .any(|mi| eval_condition(mi.meta_item().unwrap(), sess, features, eval)), + // We don't use any() here, because we want to evaluate all cfg condition + // as eval_condition can (and does) extra checks + .fold(false, |res, mi| { + res | eval_condition(mi.meta_item().unwrap(), sess, features, eval) + }), sym::all => mis .iter() - .all(|mi| eval_condition(mi.meta_item().unwrap(), sess, features, eval)), + // We don't use all() here, because we want to evaluate all cfg condition + // as eval_condition can (and does) extra checks + .fold(true, |res, mi| { + res & eval_condition(mi.meta_item().unwrap(), sess, features, eval) + }), sym::not => { if mis.len() != 1 { struct_span_err!( diff --git a/src/test/ui/cfg/cfg-path-error.rs b/src/test/ui/cfg/cfg-path-error.rs new file mode 100644 index 0000000000000..5bf80bd74b843 --- /dev/null +++ b/src/test/ui/cfg/cfg-path-error.rs @@ -0,0 +1,19 @@ +// check-fail + +#[cfg(any(foo, foo::bar))] +//~^ERROR `cfg` predicate key must be an identifier +fn foo1() {} + +#[cfg(any(foo::bar, foo))] +//~^ERROR `cfg` predicate key must be an identifier +fn foo2() {} + +#[cfg(all(foo, foo::bar))] +//~^ERROR `cfg` predicate key must be an identifier +fn foo3() {} + +#[cfg(all(foo::bar, foo))] +//~^ERROR `cfg` predicate key must be an identifier +fn foo4() {} + +fn main() {} diff --git a/src/test/ui/cfg/cfg-path-error.stderr b/src/test/ui/cfg/cfg-path-error.stderr new file mode 100644 index 0000000000000..84b44b2b0c24a --- /dev/null +++ b/src/test/ui/cfg/cfg-path-error.stderr @@ -0,0 +1,26 @@ +error: `cfg` predicate key must be an identifier + --> $DIR/cfg-path-error.rs:3:16 + | +LL | #[cfg(any(foo, foo::bar))] + | ^^^^^^^^ + +error: `cfg` predicate key must be an identifier + --> $DIR/cfg-path-error.rs:7:11 + | +LL | #[cfg(any(foo::bar, foo))] + | ^^^^^^^^ + +error: `cfg` predicate key must be an identifier + --> $DIR/cfg-path-error.rs:11:16 + | +LL | #[cfg(all(foo, foo::bar))] + | ^^^^^^^^ + +error: `cfg` predicate key must be an identifier + --> $DIR/cfg-path-error.rs:15:11 + | +LL | #[cfg(all(foo::bar, foo))] + | ^^^^^^^^ + +error: aborting due to 4 previous errors + From 7ef74bc8b97b9c129582a4d9c208fc615c5f6ba0 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Thu, 17 Feb 2022 23:36:16 -0800 Subject: [PATCH 2/9] Let `try_collect` take advantage of `try_fold` overrides Without this change it's going through `&mut impl Iterator`, which handles `?Sized` and thus currently can't forward generics. Here's the test added, to see that it fails before this PR (once a new enough nightly is out): https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=462f2896f2fed2c238ee63ca1a7e7c56 --- .../core/src/iter/adapters/by_ref_sized.rs | 42 +++++++++++++++++++ library/core/src/iter/adapters/mod.rs | 3 ++ library/core/src/iter/mod.rs | 2 +- library/core/src/iter/traits/iterator.rs | 3 +- library/core/tests/iter/traits/iterator.rs | 24 +++++++++++ 5 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 library/core/src/iter/adapters/by_ref_sized.rs diff --git a/library/core/src/iter/adapters/by_ref_sized.rs b/library/core/src/iter/adapters/by_ref_sized.rs new file mode 100644 index 0000000000000..0b5e2a89ef3de --- /dev/null +++ b/library/core/src/iter/adapters/by_ref_sized.rs @@ -0,0 +1,42 @@ +use crate::ops::Try; + +/// Like `Iterator::by_ref`, but requiring `Sized` so it can forward generics. +/// +/// Ideally this will no longer be required, eventually, but as can be seen in +/// the benchmarks (as of Feb 2022 at least) `by_ref` can have performance cost. +pub(crate) struct ByRefSized<'a, I>(pub &'a mut I); + +impl Iterator for ByRefSized<'_, I> { + type Item = I::Item; + + fn next(&mut self) -> Option { + self.0.next() + } + + fn size_hint(&self) -> (usize, Option) { + self.0.size_hint() + } + + fn advance_by(&mut self, n: usize) -> Result<(), usize> { + self.0.advance_by(n) + } + + fn nth(&mut self, n: usize) -> Option { + self.0.nth(n) + } + + fn fold(self, init: B, f: F) -> B + where + F: FnMut(B, Self::Item) -> B, + { + self.0.fold(init, f) + } + + fn try_fold(&mut self, init: B, f: F) -> R + where + F: FnMut(B, Self::Item) -> R, + R: Try, + { + self.0.try_fold(init, f) + } +} diff --git a/library/core/src/iter/adapters/mod.rs b/library/core/src/iter/adapters/mod.rs index 2ae92e89d63b5..d82fde4752020 100644 --- a/library/core/src/iter/adapters/mod.rs +++ b/library/core/src/iter/adapters/mod.rs @@ -1,6 +1,7 @@ use crate::iter::{InPlaceIterable, Iterator}; use crate::ops::{ChangeOutputType, ControlFlow, FromResidual, NeverShortCircuit, Residual, Try}; +mod by_ref_sized; mod chain; mod cloned; mod copied; @@ -31,6 +32,8 @@ pub use self::{ scan::Scan, skip::Skip, skip_while::SkipWhile, take::Take, take_while::TakeWhile, zip::Zip, }; +pub(crate) use self::by_ref_sized::ByRefSized; + #[stable(feature = "iter_cloned", since = "1.1.0")] pub use self::cloned::Cloned; diff --git a/library/core/src/iter/mod.rs b/library/core/src/iter/mod.rs index 65f56f64dbfa6..af71b3c070caf 100644 --- a/library/core/src/iter/mod.rs +++ b/library/core/src/iter/mod.rs @@ -417,7 +417,7 @@ pub use self::adapters::{ #[unstable(feature = "iter_intersperse", reason = "recently added", issue = "79524")] pub use self::adapters::{Intersperse, IntersperseWith}; -pub(crate) use self::adapters::try_process; +pub(crate) use self::adapters::{try_process, ByRefSized}; mod adapters; mod range; diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index b38df1c2d0228..49e1f7f351540 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -2,6 +2,7 @@ use crate::cmp::{self, Ordering}; use crate::ops::{ChangeOutputType, ControlFlow, FromResidual, Residual, Try}; use super::super::try_process; +use super::super::ByRefSized; use super::super::TrustedRandomAccessNoCoerce; use super::super::{Chain, Cloned, Copied, Cycle, Enumerate, Filter, FilterMap, Fuse}; use super::super::{FlatMap, Flatten}; @@ -1856,7 +1857,7 @@ pub trait Iterator { <::Item as Try>::Residual: Residual, B: FromIterator<::Output>, { - try_process(self, |i| i.collect()) + try_process(ByRefSized(self), |i| i.collect()) } /// Collects all the items from an iterator into a collection. diff --git a/library/core/tests/iter/traits/iterator.rs b/library/core/tests/iter/traits/iterator.rs index 32bd68e3d2554..731b1592d4193 100644 --- a/library/core/tests/iter/traits/iterator.rs +++ b/library/core/tests/iter/traits/iterator.rs @@ -551,6 +551,30 @@ fn test_collect_into() { assert!(a == b); } +#[test] +fn iter_try_collect_uses_try_fold_not_next() { + // This makes sure it picks up optimizations, and doesn't use the `&mut I` impl. + struct PanicOnNext(I); + impl Iterator for PanicOnNext { + type Item = I::Item; + fn next(&mut self) -> Option { + panic!("Iterator::next should not be called!") + } + fn try_fold(&mut self, init: B, f: F) -> R + where + Self: Sized, + F: FnMut(B, Self::Item) -> R, + R: std::ops::Try, + { + self.0.try_fold(init, f) + } + } + + let it = (0..10).map(Some); + let _ = PanicOnNext(it).try_collect::>(); + // validation is just that it didn't panic. +} + // just tests by whether or not this compiles fn _empty_impl_all_auto_traits() { use std::panic::{RefUnwindSafe, UnwindSafe}; From 4e3dbb3c19bb4059e30030e7c4be62e1d8919649 Mon Sep 17 00:00:00 2001 From: Grisha <33952698+GrishaVar@users.noreply.github.com> Date: Wed, 16 Mar 2022 06:37:41 +0100 Subject: [PATCH 3/9] Add test for >65535 hashes in lexing raw string --- compiler/rustc_lexer/src/tests.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/compiler/rustc_lexer/src/tests.rs b/compiler/rustc_lexer/src/tests.rs index 94017b7b286e2..548de67449abf 100644 --- a/compiler/rustc_lexer/src/tests.rs +++ b/compiler/rustc_lexer/src/tests.rs @@ -66,6 +66,23 @@ fn test_unterminated_no_pound() { ); } +#[test] +fn test_too_many_hashes() { + let max_count = u16::MAX; + let mut hashes: String = "#".repeat(max_count.into()); + + // Valid number of hashes (65535 = 2^16 - 1), but invalid string. + check_raw_str(&hashes, max_count, Some(RawStrError::InvalidStarter { bad_char: '\u{0}' })); + + // One more hash sign (65536 = 2^16) becomes too many. + hashes.push('#'); + check_raw_str( + &hashes, + 0, + Some(RawStrError::TooManyDelimiters { found: usize::from(max_count) + 1 }), + ); +} + #[test] fn test_valid_shebang() { // https://github.com/rust-lang/rust/issues/70528 From ba611d55f39909f4c4da43fe96a275c4746c38d1 Mon Sep 17 00:00:00 2001 From: Zachary S Date: Wed, 16 Mar 2022 11:36:31 -0500 Subject: [PATCH 4/9] Derive Eq for std::cmp::Ordering, instead of using manual impl. This allows consts of type Ordering to be used in patterns, and (with feature(adt_const_params)) allows using Orderings as const generic parameters. --- library/core/src/cmp.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/library/core/src/cmp.rs b/library/core/src/cmp.rs index ddaeb9eca975c..74328a3607d64 100644 --- a/library/core/src/cmp.rs +++ b/library/core/src/cmp.rs @@ -333,7 +333,7 @@ pub struct AssertParamIsEq { /// let result = 2.cmp(&1); /// assert_eq!(Ordering::Greater, result); /// ``` -#[derive(Clone, Copy, PartialEq, Debug, Hash)] +#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)] #[stable(feature = "rust1", since = "1.0.0")] #[repr(i8)] pub enum Ordering { @@ -861,9 +861,6 @@ pub macro Ord($item:item) { /* compiler built-in */ } -#[stable(feature = "rust1", since = "1.0.0")] -impl Eq for Ordering {} - #[stable(feature = "rust1", since = "1.0.0")] impl Ord for Ordering { #[inline] From b13b495b918026fc5f3bf059bbde19c832ffe3c0 Mon Sep 17 00:00:00 2001 From: Zachary S Date: Wed, 16 Mar 2022 14:01:48 -0500 Subject: [PATCH 5/9] Add test for StructuralEq for std::cmp::Ordering. Added test in library/core/tests/cmp.rs that ensures that `const`s of type `Ordering`s can be used in patterns. --- library/core/tests/cmp.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/library/core/tests/cmp.rs b/library/core/tests/cmp.rs index 2b234de6795eb..8d0e59d5a4972 100644 --- a/library/core/tests/cmp.rs +++ b/library/core/tests/cmp.rs @@ -133,6 +133,19 @@ fn ordering_const() { assert_eq!(THEN, Greater); } +#[test] +fn ordering_structural_eq() { + // test that consts of type `Ordering` are usable in patterns + + const ORDERING: Ordering = Greater; + + const REVERSE: Ordering = ORDERING.reverse(); + match Ordering::Less { + REVERSE => {} + _ => unreachable!(), + }; +} + #[test] fn cmp_default() { // Test default methods in PartialOrd and PartialEq From b1f31798045ccb5933749e01cc08ce6621889764 Mon Sep 17 00:00:00 2001 From: wcampbell Date: Thu, 17 Mar 2022 19:12:09 -0400 Subject: [PATCH 6/9] feat: Add use of bool::then in sys/unix/process Remove else { None } in favor of using bool::then() --- library/std/src/sys/unix/process/process_unix.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 9c477e5addc44..9d2803b40c445 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -648,11 +648,11 @@ impl ExitStatus { } pub fn code(&self) -> Option { - if self.exited() { Some(libc::WEXITSTATUS(self.0)) } else { None } + self.exited().then(|| libc::WEXITSTATUS(self.0)) } pub fn signal(&self) -> Option { - if libc::WIFSIGNALED(self.0) { Some(libc::WTERMSIG(self.0)) } else { None } + libc::WIFSIGNALED(self.0).then(|| libc::WTERMSIG(self.0)) } pub fn core_dumped(&self) -> bool { @@ -660,7 +660,7 @@ impl ExitStatus { } pub fn stopped_signal(&self) -> Option { - if libc::WIFSTOPPED(self.0) { Some(libc::WSTOPSIG(self.0)) } else { None } + libc::WIFSTOPPED(self.0).then(|| libc::WSTOPSIG(self.0)) } pub fn continued(&self) -> bool { From 1ddbae372fce6d0fde6cdda4082ec45dc2f113d3 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 11 Mar 2022 15:29:11 +0100 Subject: [PATCH 7/9] Compare installed browser-ui-test version to the one used in CI --- src/bootstrap/test.rs | 58 +++++++++++++------ .../host-x86_64/x86_64-gnu-tools/Dockerfile | 10 +++- .../x86_64-gnu-tools/browser-ui-test.version | 1 + 3 files changed, 50 insertions(+), 19 deletions(-) create mode 100644 src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 58b73ebed5000..c8b76809abad7 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -836,9 +836,9 @@ impl Step for RustdocJSNotStd { } } -fn check_if_browser_ui_test_is_installed_global(npm: &Path, global: bool) -> bool { +fn get_browser_ui_test_version_inner(npm: &Path, global: bool) -> Option { let mut command = Command::new(&npm); - command.arg("list").arg("--depth=0"); + command.arg("list").arg("--parseable").arg("--long").arg("--depth=0"); if global { command.arg("--global"); } @@ -846,12 +846,29 @@ fn check_if_browser_ui_test_is_installed_global(npm: &Path, global: bool) -> boo .output() .map(|output| String::from_utf8_lossy(&output.stdout).into_owned()) .unwrap_or(String::new()); - lines.contains(&" browser-ui-test@") + lines.lines().find_map(|l| l.split(":browser-ui-test@").skip(1).next()).map(|v| v.to_owned()) } -fn check_if_browser_ui_test_is_installed(npm: &Path) -> bool { - check_if_browser_ui_test_is_installed_global(npm, false) - || check_if_browser_ui_test_is_installed_global(npm, true) +fn get_browser_ui_test_version(npm: &Path) -> Option { + get_browser_ui_test_version_inner(npm, false) + .or_else(|| get_browser_ui_test_version_inner(npm, true)) +} + +fn compare_browser_ui_test_version(installed_version: &str, src: &Path) { + match fs::read_to_string( + src.join("src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version"), + ) { + Ok(v) => { + if v.trim() != installed_version { + eprintln!( + "⚠️ Installed version of browser-ui-test (`{}`) is different than the \ + one used in the CI (`{}`)", + installed_version, v + ); + } + } + Err(e) => eprintln!("Couldn't find the CI browser-ui-test version: {:?}", e), + } } #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] @@ -874,7 +891,7 @@ impl Step for RustdocGUI { .config .npm .as_ref() - .map(|p| check_if_browser_ui_test_is_installed(p)) + .map(|p| get_browser_ui_test_version(p).is_some()) .unwrap_or(false) })) } @@ -892,16 +909,23 @@ impl Step for RustdocGUI { // The goal here is to check if the necessary packages are installed, and if not, we // panic. - if !check_if_browser_ui_test_is_installed(&npm) { - eprintln!( - "error: rustdoc-gui test suite cannot be run because npm `browser-ui-test` \ - dependency is missing", - ); - eprintln!( - "If you want to install the `{0}` dependency, run `npm install {0}`", - "browser-ui-test", - ); - panic!("Cannot run rustdoc-gui tests"); + match get_browser_ui_test_version(&npm) { + Some(version) => { + // We also check the version currently used in CI and emit a warning if it's not the + // same one. + compare_browser_ui_test_version(&version, &builder.build.src); + } + None => { + eprintln!( + "error: rustdoc-gui test suite cannot be run because npm `browser-ui-test` \ + dependency is missing", + ); + eprintln!( + "If you want to install the `{0}` dependency, run `npm install {0}`", + "browser-ui-test", + ); + panic!("Cannot run rustdoc-gui tests"); + } } let out_dir = builder.test_out(self.target).join("rustdoc-gui"); diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile index d78fc6d208330..2358091a6dfbf 100644 --- a/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile +++ b/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile @@ -65,14 +65,20 @@ RUN /scripts/cmake.sh COPY host-x86_64/x86_64-gnu-tools/checktools.sh /tmp/ RUN curl -sL https://nodejs.org/dist/v14.4.0/node-v14.4.0-linux-x64.tar.xz | tar -xJ -ENV PATH="/node-v14.4.0-linux-x64/bin:${PATH}" +ENV NODE_FOLDER=/node-v14.4.0-linux-x64/bin +ENV PATH="$NODE_FOLDER:${PATH}" + +COPY host-x86_64/x86_64-gnu-tools/browser-ui-test.version /tmp/ # For now, we need to use `--unsafe-perm=true` to go around an issue when npm tries # to create a new folder. For reference: # https://github.com/puppeteer/puppeteer/issues/375 # # We also specify the version in case we need to update it to go around cache limitations. -RUN npm install -g browser-ui-test@0.8.3 --unsafe-perm=true +# +# The `browser-ui-test.version` file is also used by bootstrap to emit warnings in case +# the local version of the package is different than the one used by the CI. +RUN npm install -g browser-ui-test@$(head -n 1 /tmp/browser-ui-test.version) --unsafe-perm=true ENV RUST_CONFIGURE_ARGS \ --build=x86_64-unknown-linux-gnu \ diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version b/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version new file mode 100644 index 0000000000000..fab77af2a1a73 --- /dev/null +++ b/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version @@ -0,0 +1 @@ +0.8.3 \ No newline at end of file From c8158e9f40e400c485bb96c5293d30c0b9600627 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 18 Mar 2022 10:50:53 +0100 Subject: [PATCH 8/9] Run rustdoc GUI tests when browser-ui-test version is updated --- src/ci/scripts/should-skip-this.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ci/scripts/should-skip-this.sh b/src/ci/scripts/should-skip-this.sh index c046a6c8a26f3..c863f1b68c7fc 100755 --- a/src/ci/scripts/should-skip-this.sh +++ b/src/ci/scripts/should-skip-this.sh @@ -26,6 +26,7 @@ if [[ -n "${CI_ONLY_WHEN_SUBMODULES_CHANGED-}" ]]; then src/test/rustdoc-gui \ src/librustdoc \ src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile \ + src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version \ src/tools/rustdoc-gui); then # There was a change in either rustdoc or in its GUI tests. echo "Rustdoc was updated" From 156734dda02737d6462e72163d7a62acd05a6802 Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Fri, 18 Mar 2022 18:14:34 +0100 Subject: [PATCH 9/9] Document that `Option` discriminant elision applies for any ABI The current phrasing was not very clear on that aspect. --- library/core/src/option.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/core/src/option.rs b/library/core/src/option.rs index de1628f83ade8..bcba18a4a3e0a 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -80,11 +80,13 @@ //! * [`Box`] //! * `&U` //! * `&mut U` -//! * `fn`, `extern "C" fn` +//! * `fn`, `extern "C" fn`[^extern_fn] //! * [`num::NonZero*`] //! * [`ptr::NonNull`] //! * `#[repr(transparent)]` struct around one of the types in this list. //! +//! [^extern_fn]: this remains true for any other ABI: `extern "abi" fn` (_e.g._, `extern "system" fn`) +//! //! [`Box`]: ../../std/boxed/struct.Box.html //! [`num::NonZero*`]: crate::num //! [`ptr::NonNull`]: crate::ptr::NonNull