Auto merge of #116480 - GuillaumeGomez:rollup-s0lv33r, r=GuillaumeGomez

Rollup of 6 pull requests

Successful merges:

 - #111595 (`waitqueue` clarifications for SGX platform)
 - #115454 (Clarify example in docs of str::char_slice)
 - #115522 (Clarify ManuallyDrop bit validity)
 - #115588 (Fix a comment in std::iter::successors)
 - #116329 (update some comments around swap())
 - #116475 (rustdoc-search: fix bug with multi-item impl trait)

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

Author: bors

Diff for: compiler/rustc_middle/src/mir/syntax.rs

@@ -996,7 +996,7 @@ pub type AssertMessage<'tcx> = AssertKind<Operand<'tcx>>;
 ///
 /// [UCG#319]: https://github.com/rust-lang/unsafe-code-guidelines/issues/319
 ///
-/// Rust currently requires that every place obey those two rules. This is checked by MIRI and taken
+/// Rust currently requires that every place obey those two rules. This is checked by Miri and taken
 /// advantage of by codegen (via `gep inbounds`). That is possibly subject to change.
 #[derive(Copy, Clone, PartialEq, Eq, Hash, TyEncodable, HashStable, TypeFoldable, TypeVisitable)]
 pub struct Place<'tcx> {

Diff for: library/core/src/iter/sources/successors.rs

@@ -17,7 +17,7 @@ where
     F: FnMut(&T) -> Option<T>,
 {
     // If this function returned `impl Iterator<Item=T>`
-    // it could be based on `unfold` and not need a dedicated type.
+    // it could be based on `from_fn` and not need a dedicated type.
     // However having a named `Successors<T, F>` type allows it to be `Clone` when `T` and `F` are.
     Successors { next: first, succ }
 }

Diff for: library/core/src/mem/manually_drop.rs

@@ -4,12 +4,12 @@ use crate::ptr;
 /// A wrapper to inhibit compiler from automatically calling `T`’s destructor.
 /// This wrapper is 0-cost.
 ///
-/// `ManuallyDrop<T>` is guaranteed to have the same layout as `T`, and is subject
-/// to the same layout optimizations as `T`. As a consequence, it has *no effect*
-/// on the assumptions that the compiler makes about its contents. For example,
-/// initializing a `ManuallyDrop<&mut T>` with [`mem::zeroed`] is undefined
-/// behavior. If you need to handle uninitialized data, use [`MaybeUninit<T>`]
-/// instead.
+/// `ManuallyDrop<T>` is guaranteed to have the same layout and bit validity as
+/// `T`, and is subject to the same layout optimizations as `T`. As a consequence,
+/// it has *no effect* on the assumptions that the compiler makes about its
+/// contents. For example, initializing a `ManuallyDrop<&mut T>` with [`mem::zeroed`]
+/// is undefined behavior. If you need to handle uninitialized data, use
+/// [`MaybeUninit<T>`] instead.
 ///
 /// Note that accessing the value inside a `ManuallyDrop<T>` is safe.
 /// This means that a `ManuallyDrop<T>` whose content has been dropped must not

Diff for: library/core/src/mem/mod.rs

@@ -728,10 +728,6 @@ pub const fn swap<T>(x: &mut T, y: &mut T) {
     // reinterpretation of values as (chunkable) byte arrays, and the loop in the
     // block optimization in `swap_slice` is hard to rewrite back
     // into the (unoptimized) direct swapping implementation, so we disable it.
-    // FIXME(eddyb) the block optimization also prevents MIR optimizations from
-    // understanding `mem::replace`, `Option::take`, etc. - a better overall
-    // solution might be to make `ptr::swap_nonoverlapping` into an intrinsic, which
-    // a backend can choose to implement using the block optimization, or not.
     #[cfg(not(any(target_arch = "spirv")))]
     {
         // For types that are larger multiples of their alignment, the simple way
@@ -768,11 +764,14 @@ pub(crate) const fn swap_simple<T>(x: &mut T, y: &mut T) {
     // And LLVM actually optimizes it to 3×memcpy if called with
     // a type larger than it's willing to keep in a register.
     // Having typed reads and writes in MIR here is also good as
-    // it lets MIRI and CTFE understand them better, including things
+    // it lets Miri and CTFE understand them better, including things
     // like enforcing type validity for them.
     // Importantly, read+copy_nonoverlapping+write introduces confusing
     // asymmetry to the behaviour where one value went through read+write
     // whereas the other was copied over by the intrinsic (see #94371).
+    // Furthermore, using only read+write here benefits limited backends
+    // such as SPIR-V that work on an underlying *typed* view of memory,
+    // and thus have trouble with Rust's untyped memory operations.
 
     // SAFETY: exclusive references are always valid to read/write,
     // including being aligned, and nothing here panics so it's drop-safe.

Diff for: library/core/src/str/mod.rs

@@ -808,7 +808,7 @@ impl str {
     /// assert_eq!(Some((0, 'y')), char_indices.next()); // not (0, 'y̆')
     /// assert_eq!(Some((1, '\u{0306}')), char_indices.next());
     ///
-    /// // note the 3 here - the last character took up two bytes
+    /// // note the 3 here - the previous character took up two bytes
     /// assert_eq!(Some((3, 'e')), char_indices.next());
     /// assert_eq!(Some((4, 's')), char_indices.next());
     ///

Diff for: library/core/tests/array.rs

@@ -663,7 +663,7 @@ fn array_mixed_equality_nans() {
 
 #[test]
 fn array_into_iter_fold() {
-    // Strings to help MIRI catch if we double-free or something
+    // Strings to help Miri catch if we double-free or something
     let a = ["Aa".to_string(), "Bb".to_string(), "Cc".to_string()];
     let mut s = "s".to_string();
     a.into_iter().for_each(|b| s += &b);
@@ -679,7 +679,7 @@ fn array_into_iter_fold() {
 
 #[test]
 fn array_into_iter_rfold() {
-    // Strings to help MIRI catch if we double-free or something
+    // Strings to help Miri catch if we double-free or something
     let a = ["Aa".to_string(), "Bb".to_string(), "Cc".to_string()];
     let mut s = "s".to_string();
     a.into_iter().rev().for_each(|b| s += &b);

Diff for: library/std/src/sys/sgx/waitqueue/mod.rs

@@ -18,6 +18,7 @@ mod unsafe_list;
 
 use crate::num::NonZeroUsize;
 use crate::ops::{Deref, DerefMut};
+use crate::panic::{self, AssertUnwindSafe};
 use crate::time::Duration;
 
 use super::abi::thread;
@@ -147,7 +148,8 @@ impl WaitQueue {
     /// Adds the calling thread to the `WaitVariable`'s wait queue, then wait
     /// until a wakeup event.
     ///
-    /// This function does not return until this thread has been awoken.
+    /// This function does not return until this thread has been awoken. When `before_wait` panics,
+    /// this function will abort.
     pub fn wait<T, F: FnOnce()>(mut guard: SpinMutexGuard<'_, WaitVariable<T>>, before_wait: F) {
         // very unsafe: check requirements of UnsafeList::push
         unsafe {
@@ -157,8 +159,13 @@ impl WaitQueue {
             }));
             let entry = guard.queue.inner.push(&mut entry);
             drop(guard);
-            before_wait();
+            if let Err(_e) = panic::catch_unwind(AssertUnwindSafe(|| before_wait())) {
+                rtabort!("Panic before wait on wakeup event")
+            }
             while !entry.lock().wake {
+                // `entry.wake` is only set in `notify_one` and `notify_all` functions. Both ensure
+                // the entry is removed from the queue _before_ setting this bool. There are no
+                // other references to `entry`.
                 // don't panic, this would invalidate `entry` during unwinding
                 let eventset = rtunwrap!(Ok, usercalls::wait(EV_UNPARK, WAIT_INDEFINITE));
                 rtassert!(eventset & EV_UNPARK == EV_UNPARK);
@@ -169,6 +176,7 @@ impl WaitQueue {
     /// Adds the calling thread to the `WaitVariable`'s wait queue, then wait
     /// until a wakeup event or timeout. If event was observed, returns true.
     /// If not, it will remove the calling thread from the wait queue.
+    /// When `before_wait` panics, this function will abort.
     pub fn wait_timeout<T, F: FnOnce()>(
         lock: &SpinMutex<WaitVariable<T>>,
         timeout: Duration,
@@ -181,9 +189,13 @@ impl WaitQueue {
                 wake: false,
             }));
             let entry_lock = lock.lock().queue.inner.push(&mut entry);
-            before_wait();
+            if let Err(_e) = panic::catch_unwind(AssertUnwindSafe(|| before_wait())) {
+                rtabort!("Panic before wait on wakeup event or timeout")
+            }
             usercalls::wait_timeout(EV_UNPARK, timeout, || entry_lock.lock().wake);
-            // acquire the wait queue's lock first to avoid deadlock.
+            // acquire the wait queue's lock first to avoid deadlock
+            // and ensure no other function can simultaneously access the list
+            // (e.g., `notify_one` or `notify_all`)
             let mut guard = lock.lock();
             let success = entry_lock.lock().wake;
             if !success {
@@ -204,8 +216,8 @@ impl WaitQueue {
     ) -> Result<WaitGuard<'_, T>, SpinMutexGuard<'_, WaitVariable<T>>> {
         // SAFETY: lifetime of the pop() return value is limited to the map
         // closure (The closure return value is 'static). The underlying
-        // stack frame won't be freed until after the WaitGuard created below
-        // is dropped.
+        // stack frame won't be freed until after the lock on the queue is released
+        // (i.e., `guard` is dropped).
         unsafe {
             let tcs = guard.queue.inner.pop().map(|entry| -> Tcs {
                 let mut entry_guard = entry.lock();
@@ -231,7 +243,7 @@ impl WaitQueue {
     ) -> Result<WaitGuard<'_, T>, SpinMutexGuard<'_, WaitVariable<T>>> {
         // SAFETY: lifetime of the pop() return values are limited to the
         // while loop body. The underlying stack frames won't be freed until
-        // after the WaitGuard created below is dropped.
+        // after the lock on the queue is released (i.e., `guard` is dropped).
         unsafe {
             let mut count = 0;
             while let Some(entry) = guard.queue.inner.pop() {

Diff for: src/librustdoc/html/static/js/search.js

@@ -1555,7 +1555,7 @@ function initSearch(rawSearchIndex) {
                         return false;
                     }
                 }
-            } else if (fnType.id !== null) {
+            } else {
                 if (queryElem.id === typeNameIdOfArrayOrSlice &&
                     (fnType.id === typeNameIdOfSlice || fnType.id === typeNameIdOfArray)
                 ) {

Diff for: tests/rustdoc-js/impl-trait.js

@@ -1,3 +1,4 @@
+// exact-check
 // ignore-order
 
 const EXPECTED = [
@@ -20,9 +21,16 @@ const EXPECTED = [
     {
         'query': '-> Aaaaaaa',
         'others': [
-            { 'path': 'impl_trait::Ccccccc', 'name': 'fffffff' },
-            { 'path': 'impl_trait::Ccccccc', 'name': 'ddddddd' },
             { 'path': 'impl_trait', 'name': 'bbbbbbb' },
+            { 'path': 'impl_trait::Ccccccc', 'name': 'ddddddd' },
+            { 'path': 'impl_trait::Ccccccc', 'name': 'fffffff' },
+            { 'path': 'impl_trait::Ccccccc', 'name': 'ggggggg' },
+        ],
+    },
+    {
+        'query': '-> Bbbbbbb',
+        'others': [
+            { 'path': 'impl_trait::Ccccccc', 'name': 'ggggggg' },
         ],
     },
     {
@@ -31,13 +39,14 @@ const EXPECTED = [
             { 'path': 'impl_trait', 'name': 'Aaaaaaa' },
         ],
         'in_args': [
-            { 'path': 'impl_trait::Ccccccc', 'name': 'fffffff' },
             { 'path': 'impl_trait::Ccccccc', 'name': 'eeeeeee' },
+            { 'path': 'impl_trait::Ccccccc', 'name': 'fffffff' },
         ],
         'returned': [
-            { 'path': 'impl_trait::Ccccccc', 'name': 'fffffff' },
-            { 'path': 'impl_trait::Ccccccc', 'name': 'ddddddd' },
             { 'path': 'impl_trait', 'name': 'bbbbbbb' },
+            { 'path': 'impl_trait::Ccccccc', 'name': 'ddddddd' },
+            { 'path': 'impl_trait::Ccccccc', 'name': 'fffffff' },
+            { 'path': 'impl_trait::Ccccccc', 'name': 'ggggggg' },
         ],
     },
 ];

Diff for: tests/rustdoc-js/impl-trait.rs

@@ -1,6 +1,9 @@
 pub trait Aaaaaaa {}
 
+pub trait Bbbbbbb {}
+
 impl Aaaaaaa for () {}
+impl Bbbbbbb for () {}
 
 pub fn bbbbbbb() -> impl Aaaaaaa {
     ()
@@ -18,4 +21,7 @@ impl Ccccccc {
     pub fn fffffff(&self, x: impl Aaaaaaa) -> impl Aaaaaaa {
         x
     }
+    pub fn ggggggg(&self) -> impl Aaaaaaa + Bbbbbbb {
+        ()
+    }
 }

Diff for: tests/ui/consts/const-eval/nrvo.rs

@@ -1,7 +1,8 @@
 // run-pass
 
 // When the NRVO is applied, the return place (`_0`) gets treated like a normal local. For example,
-// its address may be taken and it may be written to indirectly. Ensure that MIRI can handle this.
+// its address may be taken and it may be written to indirectly. Ensure that the const-eval
+// interpreter can handle this.
 
 #![feature(const_mut_refs)]