Auto merge of #95090 - matthiaskrgr:rollup-pho6x6s, r=matthiaskrgr

Rollup of 7 pull requests

Successful merges:

 - #94115 (Let `try_collect` take advantage of `try_fold` overrides)
 - #94295 (Always evaluate all cfg predicate in all() and any())
 - #94848 (Compare installed browser-ui-test version to the one used in CI)
 - #94993 (Add test for >65535 hashes in lexing raw string)
 - #95017 (Derive Eq for std::cmp::Ordering, instead of using manual impl.)
 - #95058 (Add use of bool::then in sys/unix/process)
 - #95083 (Document that `Option<extern "abi" fn>` discriminant elision applies for any ABI)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup

compiler/rustc_attr/src/builtin.rs

@@ -603,10 +603,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!(

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

library/core/src/cmp.rs

@@ -333,7 +333,7 @@ pub struct AssertParamIsEq<T: Eq + ?Sized> {
 /// 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]

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<I: Iterator> Iterator for ByRefSized<'_, I> {
+    type Item = I::Item;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.0.next()
+    }
+
+    fn size_hint(&self) -> (usize, Option<usize>) {
+        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::Item> {
+        self.0.nth(n)
+    }
+
+    fn fold<B, F>(self, init: B, f: F) -> B
+    where
+        F: FnMut(B, Self::Item) -> B,
+    {
+        self.0.fold(init, f)
+    }
+
+    fn try_fold<B, F, R>(&mut self, init: B, f: F) -> R
+    where
+        F: FnMut(B, Self::Item) -> R,
+        R: Try<Output = B>,
+    {
+        self.0.try_fold(init, f)
+    }
+}

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;
 

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;

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};
@@ -1861,7 +1862,7 @@ pub trait Iterator {
         <<Self as Iterator>::Item as Try>::Residual: Residual<B>,
         B: FromIterator<<Self::Item as Try>::Output>,
     {
-        try_process(self, |i| i.collect())
+        try_process(ByRefSized(self), |i| i.collect())
     }
 
     /// Collects all the items from an iterator into a collection.

library/core/src/option.rs

@@ -80,11 +80,13 @@
 //! * [`Box<U>`]
 //! * `&U`
 //! * `&mut U`
-//! * `fn`, `extern "C" fn`
+//! * `fn`, `extern "C" fn`[^extern_fn]
 //! * [`num::NonZero*`]
 //! * [`ptr::NonNull<U>`]
 //! * `#[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<U>`]: ../../std/boxed/struct.Box.html
 //! [`num::NonZero*`]: crate::num
 //! [`ptr::NonNull<U>`]: crate::ptr::NonNull

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

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>(I);
+    impl<I: Iterator> Iterator for PanicOnNext<I> {
+        type Item = I::Item;
+        fn next(&mut self) -> Option<Self::Item> {
+            panic!("Iterator::next should not be called!")
+        }
+        fn try_fold<B, F, R>(&mut self, init: B, f: F) -> R
+        where
+            Self: Sized,
+            F: FnMut(B, Self::Item) -> R,
+            R: std::ops::Try<Output = B>,
+        {
+            self.0.try_fold(init, f)
+        }
+    }
+
+    let it = (0..10).map(Some);
+    let _ = PanicOnNext(it).try_collect::<Vec<_>>();
+    // validation is just that it didn't panic.
+}
+
 // just tests by whether or not this compiles
 fn _empty_impl_all_auto_traits<T>() {
     use std::panic::{RefUnwindSafe, UnwindSafe};

library/std/src/sys/unix/process/process_unix.rs

@@ -648,19 +648,19 @@ impl ExitStatus {
     }
 
     pub fn code(&self) -> Option<i32> {
-        if self.exited() { Some(libc::WEXITSTATUS(self.0)) } else { None }
+        self.exited().then(|| libc::WEXITSTATUS(self.0))
     }
 
     pub fn signal(&self) -> Option<i32> {
-        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 {
         libc::WIFSIGNALED(self.0) && libc::WCOREDUMP(self.0)
     }
 
     pub fn stopped_signal(&self) -> Option<i32> {
-        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 {

src/bootstrap/test.rs

@@ -836,22 +836,39 @@ 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<String> {
     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");
     }
     let lines = command
         .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<String> {
+    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");

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 \

src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version

@@ -0,0 +1 @@
+0.8.3

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"

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() {}