From 069892294fe697e67b98dd4b4161d38853aafd2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Fri, 20 Nov 2020 21:45:51 +0100 Subject: [PATCH 01/28] Deprecate compare_and_swap on all atomic types --- library/core/src/sync/atomic.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/library/core/src/sync/atomic.rs b/library/core/src/sync/atomic.rs index 9d2045990570b..1fff4bfe53b03 100644 --- a/library/core/src/sync/atomic.rs +++ b/library/core/src/sync/atomic.rs @@ -479,6 +479,10 @@ impl AtomicBool { /// ``` #[inline] #[stable(feature = "rust1", since = "1.0.0")] + #[rustc_deprecated( + since = "1.50.0", + reason = "Use `compare_exchange` or `compare_exchange_weak` instead" + )] #[cfg(target_has_atomic = "8")] pub fn compare_and_swap(&self, current: bool, new: bool, order: Ordering) -> bool { match self.compare_exchange(current, new, order, strongest_failure_ordering(order)) { @@ -1058,6 +1062,10 @@ impl AtomicPtr { /// ``` #[inline] #[stable(feature = "rust1", since = "1.0.0")] + #[rustc_deprecated( + since = "1.50.0", + reason = "Use `compare_exchange` or `compare_exchange_weak` instead" + )] #[cfg(target_has_atomic = "ptr")] pub fn compare_and_swap(&self, current: *mut T, new: *mut T, order: Ordering) -> *mut T { match self.compare_exchange(current, new, order, strongest_failure_ordering(order)) { @@ -1582,6 +1590,10 @@ assert_eq!(some_var.load(Ordering::Relaxed), 10); ```"), #[inline] #[$stable] + #[rustc_deprecated( + since = "1.50.0", + reason = "Use `compare_exchange` or `compare_exchange_weak` instead") + ] #[$cfg_cas] pub fn compare_and_swap(&self, current: $int_type, From 7bd770ca3fc31bbe6cc8a608270b5635664cdf83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Fri, 20 Nov 2020 22:16:15 +0100 Subject: [PATCH 02/28] Add documentation on migrating away from compare_and_swap --- library/core/src/sync/atomic.rs | 51 +++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/library/core/src/sync/atomic.rs b/library/core/src/sync/atomic.rs index 1fff4bfe53b03..d27c4bc076339 100644 --- a/library/core/src/sync/atomic.rs +++ b/library/core/src/sync/atomic.rs @@ -464,6 +464,23 @@ impl AtomicBool { /// **Note:** This method is only available on platforms that support atomic /// operations on `u8`. /// + /// # Migrating to `compare_exchange` and `compare_exchange_weak` + /// + /// `compare_and_swap` is equivalent to `compare_exchange` with the following mapping for + /// memory orderings: + /// + /// Original | Success | Failure + /// -------- | ------- | ------- + /// Relaxed | Relaxed | Relaxed + /// Acquire | Acquire | Acquire + /// Release | Release | Relaxed + /// AcqRel | AcqRel | Acquire + /// SeqCst | SeqCst | SeqCst + /// + /// `compare_exchange_weak` is allowed to fail spuriously even when the comparison succeeds, + /// which allows the compiler to generate better assembly code when the compare and swap + /// is used in a loop. + /// /// # Examples /// /// ``` @@ -1048,6 +1065,23 @@ impl AtomicPtr { /// **Note:** This method is only available on platforms that support atomic /// operations on pointers. /// + /// # Migrating to `compare_exchange` and `compare_exchange_weak` + /// + /// `compare_and_swap` is equivalent to `compare_exchange` with the following mapping for + /// memory orderings: + /// + /// Original | Success | Failure + /// -------- | ------- | ------- + /// Relaxed | Relaxed | Relaxed + /// Acquire | Acquire | Acquire + /// Release | Release | Relaxed + /// AcqRel | AcqRel | Acquire + /// SeqCst | SeqCst | SeqCst + /// + /// `compare_exchange_weak` is allowed to fail spuriously even when the comparison succeeds, + /// which allows the compiler to generate better assembly code when the compare and swap + /// is used in a loop. + /// /// # Examples /// /// ``` @@ -1575,6 +1609,23 @@ happens, and using [`Release`] makes the load part [`Relaxed`]. **Note**: This method is only available on platforms that support atomic operations on [`", $s_int_type, "`](", $int_ref, "). +# Migrating to `compare_exchange` and `compare_exchange_weak` + +`compare_and_swap` is equivalent to `compare_exchange` with the following mapping for +memory orderings: + +Original | Success | Failure +-------- | ------- | ------- +Relaxed | Relaxed | Relaxed +Acquire | Acquire | Acquire +Release | Release | Relaxed +AcqRel | AcqRel | Acquire +SeqCst | SeqCst | SeqCst + +`compare_exchange_weak` is allowed to fail spuriously even when the comparison succeeds, +which allows the compiler to generate better assembly code when the compare and swap +is used in a loop. + # Examples ``` From df2ef1d0a842e69a4a883b5fe67de5ab69e84a11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Fri, 20 Nov 2020 22:27:50 +0100 Subject: [PATCH 03/28] Migrate standard library away from compare_and_swap --- library/core/tests/atomic.rs | 6 +++--- library/std/src/sync/mpsc/blocking.rs | 6 +++++- library/std/src/sync/mpsc/oneshot.rs | 14 +++++++++++--- library/std/src/sync/mpsc/shared.rs | 11 +++++++++-- library/std/src/sync/mpsc/stream.rs | 9 ++++++--- library/std/src/sync/once.rs | 16 +++++++++++----- library/std/src/sys/sgx/abi/mod.rs | 8 ++++---- library/std/src/sys/sgx/waitqueue/spin_mutex.rs | 2 +- library/std/src/sys/windows/mutex.rs | 6 +++--- library/std/src/sys_common/condvar/check.rs | 6 +++--- library/std/src/sys_common/thread_local_key.rs | 8 ++++---- .../std/src/sys_common/thread_parker/futex.rs | 2 +- .../ui/array-slice-vec/box-of-array-of-drop-1.rs | 7 ++++++- .../ui/array-slice-vec/box-of-array-of-drop-2.rs | 7 ++++++- src/test/ui/array-slice-vec/nested-vec-3.rs | 7 ++++++- 15 files changed, 79 insertions(+), 36 deletions(-) diff --git a/library/core/tests/atomic.rs b/library/core/tests/atomic.rs index acbd913982c1f..59a7067dd1113 100644 --- a/library/core/tests/atomic.rs +++ b/library/core/tests/atomic.rs @@ -4,11 +4,11 @@ use core::sync::atomic::*; #[test] fn bool_() { let a = AtomicBool::new(false); - assert_eq!(a.compare_and_swap(false, true, SeqCst), false); - assert_eq!(a.compare_and_swap(false, true, SeqCst), true); + assert_eq!(a.compare_exchange(false, true, SeqCst, SeqCst), Ok(false)); + assert_eq!(a.compare_exchange(false, true, SeqCst, SeqCst), Err(true)); a.store(false, SeqCst); - assert_eq!(a.compare_and_swap(false, true, SeqCst), false); + assert_eq!(a.compare_exchange(false, true, SeqCst, SeqCst), Ok(false)); } #[test] diff --git a/library/std/src/sync/mpsc/blocking.rs b/library/std/src/sync/mpsc/blocking.rs index d34de6a4fac3e..4c852b8ee812f 100644 --- a/library/std/src/sync/mpsc/blocking.rs +++ b/library/std/src/sync/mpsc/blocking.rs @@ -36,7 +36,11 @@ pub fn tokens() -> (WaitToken, SignalToken) { impl SignalToken { pub fn signal(&self) -> bool { - let wake = !self.inner.woken.compare_and_swap(false, true, Ordering::SeqCst); + let wake = self + .inner + .woken + .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) + .is_ok(); if wake { self.inner.thread.unpark(); } diff --git a/library/std/src/sync/mpsc/oneshot.rs b/library/std/src/sync/mpsc/oneshot.rs index 75f5621fa127e..3dcf03f579a0f 100644 --- a/library/std/src/sync/mpsc/oneshot.rs +++ b/library/std/src/sync/mpsc/oneshot.rs @@ -129,7 +129,7 @@ impl Packet { let ptr = unsafe { signal_token.cast_to_usize() }; // race with senders to enter the blocking state - if self.state.compare_and_swap(EMPTY, ptr, Ordering::SeqCst) == EMPTY { + if self.state.compare_exchange(EMPTY, ptr, Ordering::SeqCst, Ordering::SeqCst).is_ok() { if let Some(deadline) = deadline { let timed_out = !wait_token.wait_max_until(deadline); // Try to reset the state @@ -161,7 +161,12 @@ impl Packet { // the state changes under our feet we'd rather just see that state // change. DATA => { - self.state.compare_and_swap(DATA, EMPTY, Ordering::SeqCst); + let _ = self.state.compare_exchange( + DATA, + EMPTY, + Ordering::SeqCst, + Ordering::SeqCst, + ); match (&mut *self.data.get()).take() { Some(data) => Ok(data), None => unreachable!(), @@ -264,7 +269,10 @@ impl Packet { // If we've got a blocked thread, then use an atomic to gain ownership // of it (may fail) - ptr => self.state.compare_and_swap(ptr, EMPTY, Ordering::SeqCst), + ptr => self + .state + .compare_exchange(ptr, EMPTY, Ordering::SeqCst, Ordering::SeqCst) + .unwrap_or_else(|x| x), }; // Now that we've got ownership of our state, figure out what to do diff --git a/library/std/src/sync/mpsc/shared.rs b/library/std/src/sync/mpsc/shared.rs index 898654f21f2ea..0c32e636a5633 100644 --- a/library/std/src/sync/mpsc/shared.rs +++ b/library/std/src/sync/mpsc/shared.rs @@ -385,8 +385,15 @@ impl Packet { self.port_dropped.store(true, Ordering::SeqCst); let mut steals = unsafe { *self.steals.get() }; while { - let cnt = self.cnt.compare_and_swap(steals, DISCONNECTED, Ordering::SeqCst); - cnt != DISCONNECTED && cnt != steals + match self.cnt.compare_exchange( + steals, + DISCONNECTED, + Ordering::SeqCst, + Ordering::SeqCst, + ) { + Ok(_) => false, + Err(old) => old != DISCONNECTED, + } } { // See the discussion in 'try_recv' for why we yield // control of this thread. diff --git a/library/std/src/sync/mpsc/stream.rs b/library/std/src/sync/mpsc/stream.rs index 9f7c1af895199..a652f24c58a19 100644 --- a/library/std/src/sync/mpsc/stream.rs +++ b/library/std/src/sync/mpsc/stream.rs @@ -322,12 +322,15 @@ impl Packet { // (because there is a bounded number of senders). let mut steals = unsafe { *self.queue.consumer_addition().steals.get() }; while { - let cnt = self.queue.producer_addition().cnt.compare_and_swap( + match self.queue.producer_addition().cnt.compare_exchange( steals, DISCONNECTED, Ordering::SeqCst, - ); - cnt != DISCONNECTED && cnt != steals + Ordering::SeqCst, + ) { + Ok(_) => false, + Err(old) => old != DISCONNECTED, + } } { while self.queue.pop().is_some() { steals += 1; diff --git a/library/std/src/sync/once.rs b/library/std/src/sync/once.rs index de5ddf1daf27b..9a17d121db19e 100644 --- a/library/std/src/sync/once.rs +++ b/library/std/src/sync/once.rs @@ -65,7 +65,7 @@ // must do so with Release ordering to make the result available. // - `wait` inserts `Waiter` nodes as a pointer in `state_and_queue`, and // needs to make the nodes available with Release ordering. The load in -// its `compare_and_swap` can be Relaxed because it only has to compare +// its `compare_exchange` can be Relaxed because it only has to compare // the atomic, not to read other data. // - `WaiterQueue::Drop` must see the `Waiter` nodes, so it must load // `state_and_queue` with Acquire ordering. @@ -395,12 +395,13 @@ impl Once { } POISONED | INCOMPLETE => { // Try to register this thread as the one RUNNING. - let old = self.state_and_queue.compare_and_swap( + let exchange_result = self.state_and_queue.compare_exchange( state_and_queue, RUNNING, Ordering::Acquire, + Ordering::Acquire, ); - if old != state_and_queue { + if let Err(old) = exchange_result { state_and_queue = old; continue; } @@ -452,8 +453,13 @@ fn wait(state_and_queue: &AtomicUsize, mut current_state: usize) { // Try to slide in the node at the head of the linked list, making sure // that another thread didn't just replace the head of the linked list. - let old = state_and_queue.compare_and_swap(current_state, me | RUNNING, Ordering::Release); - if old != current_state { + let exchange_result = state_and_queue.compare_exchange( + current_state, + me | RUNNING, + Ordering::Release, + Ordering::Relaxed, + ); + if let Err(old) = exchange_result { current_state = old; continue; } diff --git a/library/std/src/sys/sgx/abi/mod.rs b/library/std/src/sys/sgx/abi/mod.rs index a0eb12c3d154a..a5e453034762c 100644 --- a/library/std/src/sys/sgx/abi/mod.rs +++ b/library/std/src/sys/sgx/abi/mod.rs @@ -36,20 +36,20 @@ unsafe extern "C" fn tcs_init(secondary: bool) { } // Try to atomically swap UNINIT with BUSY. The returned state can be: - match RELOC_STATE.compare_and_swap(UNINIT, BUSY, Ordering::Acquire) { + match RELOC_STATE.compare_exchange(UNINIT, BUSY, Ordering::Acquire, Ordering::Acquire) { // This thread just obtained the lock and other threads will observe BUSY - UNINIT => { + Ok(_) => { reloc::relocate_elf_rela(); RELOC_STATE.store(DONE, Ordering::Release); } // We need to wait until the initialization is done. - BUSY => { + Err(BUSY) => { while RELOC_STATE.load(Ordering::Acquire) == BUSY { core::hint::spin_loop(); } } // Initialization is done. - DONE => {} + Err(DONE) => {} _ => unreachable!(), } } diff --git a/library/std/src/sys/sgx/waitqueue/spin_mutex.rs b/library/std/src/sys/sgx/waitqueue/spin_mutex.rs index d99ce895da594..9140041c58414 100644 --- a/library/std/src/sys/sgx/waitqueue/spin_mutex.rs +++ b/library/std/src/sys/sgx/waitqueue/spin_mutex.rs @@ -42,7 +42,7 @@ impl SpinMutex { #[inline(always)] pub fn try_lock(&self) -> Option> { - if !self.lock.compare_and_swap(false, true, Ordering::Acquire) { + if self.lock.compare_exchange(false, true, Ordering::Acquire, Ordering::Acquire).is_ok() { Some(SpinMutexGuard { mutex: self }) } else { None diff --git a/library/std/src/sys/windows/mutex.rs b/library/std/src/sys/windows/mutex.rs index fa51b006c346f..d4cc56d4cb3ef 100644 --- a/library/std/src/sys/windows/mutex.rs +++ b/library/std/src/sys/windows/mutex.rs @@ -123,9 +123,9 @@ impl Mutex { let inner = box Inner { remutex: ReentrantMutex::uninitialized(), held: Cell::new(false) }; inner.remutex.init(); let inner = Box::into_raw(inner); - match self.lock.compare_and_swap(0, inner as usize, Ordering::SeqCst) { - 0 => inner, - n => { + match self.lock.compare_exchange(0, inner as usize, Ordering::SeqCst, Ordering::SeqCst) { + Ok(_) => inner, + Err(n) => { Box::from_raw(inner).remutex.destroy(); n as *const _ } diff --git a/library/std/src/sys_common/condvar/check.rs b/library/std/src/sys_common/condvar/check.rs index fecb732b910ce..1578a2de60cef 100644 --- a/library/std/src/sys_common/condvar/check.rs +++ b/library/std/src/sys_common/condvar/check.rs @@ -23,9 +23,9 @@ impl SameMutexCheck { } pub fn verify(&self, mutex: &MovableMutex) { let addr = mutex.raw() as *const mutex_imp::Mutex as usize; - match self.addr.compare_and_swap(0, addr, Ordering::SeqCst) { - 0 => {} // Stored the address - n if n == addr => {} // Lost a race to store the same address + match self.addr.compare_exchange(0, addr, Ordering::SeqCst, Ordering::SeqCst) { + Ok(_) => {} // Stored the address + Err(n) if n == addr => {} // Lost a race to store the same address _ => panic!("attempted to use a condition variable with two mutexes"), } } diff --git a/library/std/src/sys_common/thread_local_key.rs b/library/std/src/sys_common/thread_local_key.rs index dbcb7b36265f5..32cd56416655f 100644 --- a/library/std/src/sys_common/thread_local_key.rs +++ b/library/std/src/sys_common/thread_local_key.rs @@ -168,7 +168,7 @@ impl StaticKey { return key; } - // POSIX allows the key created here to be 0, but the compare_and_swap + // POSIX allows the key created here to be 0, but the compare_exchange // below relies on using 0 as a sentinel value to check who won the // race to set the shared TLS key. As far as I know, there is no // guaranteed value that cannot be returned as a posix_key_create key, @@ -186,11 +186,11 @@ impl StaticKey { key2 }; rtassert!(key != 0); - match self.key.compare_and_swap(0, key as usize, Ordering::SeqCst) { + match self.key.compare_exchange(0, key as usize, Ordering::SeqCst, Ordering::SeqCst) { // The CAS succeeded, so we've created the actual key - 0 => key as usize, + Ok(_) => key as usize, // If someone beat us to the punch, use their key instead - n => { + Err(n) => { imp::destroy(key); n } diff --git a/library/std/src/sys_common/thread_parker/futex.rs b/library/std/src/sys_common/thread_parker/futex.rs index a5d4927dcc5ca..c2ad6d55dc1f2 100644 --- a/library/std/src/sys_common/thread_parker/futex.rs +++ b/library/std/src/sys_common/thread_parker/futex.rs @@ -49,7 +49,7 @@ impl Parker { // Wait for something to happen, assuming it's still set to PARKED. futex_wait(&self.state, PARKED, None); // Change NOTIFIED=>EMPTY and return in that case. - if self.state.compare_and_swap(NOTIFIED, EMPTY, Acquire) == NOTIFIED { + if self.state.compare_exchange_weak(NOTIFIED, EMPTY, Acquire, Acquire).is_ok() { return; } else { // Spurious wake up. We loop to try again. diff --git a/src/test/ui/array-slice-vec/box-of-array-of-drop-1.rs b/src/test/ui/array-slice-vec/box-of-array-of-drop-1.rs index d485893281562..c8559d2472824 100644 --- a/src/test/ui/array-slice-vec/box-of-array-of-drop-1.rs +++ b/src/test/ui/array-slice-vec/box-of-array-of-drop-1.rs @@ -17,7 +17,12 @@ impl Drop for D { fn drop(&mut self) { println!("Dropping {}", self.0); let old = LOG.load(Ordering::SeqCst); - LOG.compare_and_swap(old, old << 4 | self.0 as usize, Ordering::SeqCst); + let _ = LOG.compare_exchange( + old, + old << 4 | self.0 as usize, + Ordering::SeqCst, + Ordering::SeqCst + ); } } diff --git a/src/test/ui/array-slice-vec/box-of-array-of-drop-2.rs b/src/test/ui/array-slice-vec/box-of-array-of-drop-2.rs index e8a5b00a55b9c..e75051caabcc3 100644 --- a/src/test/ui/array-slice-vec/box-of-array-of-drop-2.rs +++ b/src/test/ui/array-slice-vec/box-of-array-of-drop-2.rs @@ -17,7 +17,12 @@ impl Drop for D { fn drop(&mut self) { println!("Dropping {}", self.0); let old = LOG.load(Ordering::SeqCst); - LOG.compare_and_swap(old, old << 4 | self.0 as usize, Ordering::SeqCst); + let _ = LOG.compare_exchange( + old, + old << 4 | self.0 as usize, + Ordering::SeqCst, + Ordering::SeqCst + ); } } diff --git a/src/test/ui/array-slice-vec/nested-vec-3.rs b/src/test/ui/array-slice-vec/nested-vec-3.rs index 52b892dbcdfaf..96497a53d308e 100644 --- a/src/test/ui/array-slice-vec/nested-vec-3.rs +++ b/src/test/ui/array-slice-vec/nested-vec-3.rs @@ -18,7 +18,12 @@ impl Drop for D { fn drop(&mut self) { println!("Dropping {}", self.0); let old = LOG.load(Ordering::SeqCst); - LOG.compare_and_swap(old, old << 4 | self.0 as usize, Ordering::SeqCst); + let _ = LOG.compare_exchange( + old, + old << 4 | self.0 as usize, + Ordering::SeqCst, + Ordering::SeqCst, + ); } } From 90e6eebfd9aec44f06973e6b02e7ded20846763c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Sun, 22 Nov 2020 18:56:47 +0100 Subject: [PATCH 04/28] Add doc aliases to compare_exchange[_weak] --- library/core/src/sync/atomic.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/core/src/sync/atomic.rs b/library/core/src/sync/atomic.rs index d27c4bc076339..6fcc2d148c1db 100644 --- a/library/core/src/sync/atomic.rs +++ b/library/core/src/sync/atomic.rs @@ -546,6 +546,7 @@ impl AtomicBool { /// ``` #[inline] #[stable(feature = "extended_compare_and_swap", since = "1.10.0")] + #[doc(alias = "compare_and_swap")] #[cfg(target_has_atomic = "8")] pub fn compare_exchange( &self, @@ -599,6 +600,7 @@ impl AtomicBool { /// ``` #[inline] #[stable(feature = "extended_compare_and_swap", since = "1.10.0")] + #[doc(alias = "compare_and_swap")] #[cfg(target_has_atomic = "8")] pub fn compare_exchange_weak( &self, From ed7b244b4c6a146769f8bc9482b0e5027d40225e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Sun, 22 Nov 2020 20:26:36 +0100 Subject: [PATCH 05/28] Fix documentation typo --- library/std/src/sync/once.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sync/once.rs b/library/std/src/sync/once.rs index 9a17d121db19e..6a330834489df 100644 --- a/library/std/src/sync/once.rs +++ b/library/std/src/sync/once.rs @@ -110,7 +110,7 @@ use crate::thread::{self, Thread}; /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub struct Once { - // `state_and_queue` is actually an a pointer to a `Waiter` with extra state + // `state_and_queue` is actually a pointer to a `Waiter` with extra state // bits, so we add the `PhantomData` appropriately. state_and_queue: AtomicUsize, _marker: marker::PhantomData<*const Waiter>, From 7a40b6d8dc28af1bb59bee75cf1d819ceea1b5de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Sun, 22 Nov 2020 20:36:29 +0100 Subject: [PATCH 06/28] Improve documentation on `success` and `failure` arguments --- library/core/src/sync/atomic.rs | 42 +++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/library/core/src/sync/atomic.rs b/library/core/src/sync/atomic.rs index 6fcc2d148c1db..8edcb0197a707 100644 --- a/library/core/src/sync/atomic.rs +++ b/library/core/src/sync/atomic.rs @@ -514,9 +514,10 @@ impl AtomicBool { /// the previous value. On success this value is guaranteed to be equal to `current`. /// /// `compare_exchange` takes two [`Ordering`] arguments to describe the memory - /// ordering of this operation. The first describes the required ordering if the - /// operation succeeds while the second describes the required ordering when the - /// operation fails. Using [`Acquire`] as success ordering makes the store part + /// ordering of this operation. `success` describes the required ordering for the + /// read-modify-write operation that takes place if the comparison with `current` succeeds. + /// `failure` describes the required ordering for the load operation that takes place when + /// the comparison fails. Using [`Acquire`] as success ordering makes the store part /// of this operation [`Relaxed`], and using [`Release`] makes the successful load /// [`Relaxed`]. The failure ordering can only be [`SeqCst`], [`Acquire`] or [`Relaxed`] /// and must be equivalent to or weaker than the success ordering. @@ -572,9 +573,10 @@ impl AtomicBool { /// previous value. /// /// `compare_exchange_weak` takes two [`Ordering`] arguments to describe the memory - /// ordering of this operation. The first describes the required ordering if the - /// operation succeeds while the second describes the required ordering when the - /// operation fails. Using [`Acquire`] as success ordering makes the store part + /// ordering of this operation. `success` describes the required ordering for the + /// read-modify-write operation that takes place if the comparison with `current` succeeds. + /// `failure` describes the required ordering for the load operation that takes place when + /// the comparison fails. Using [`Acquire`] as success ordering makes the store part /// of this operation [`Relaxed`], and using [`Release`] makes the successful load /// [`Relaxed`]. The failure ordering can only be [`SeqCst`], [`Acquire`] or [`Relaxed`] /// and must be equivalent to or weaker than the success ordering. @@ -1116,9 +1118,10 @@ impl AtomicPtr { /// the previous value. On success this value is guaranteed to be equal to `current`. /// /// `compare_exchange` takes two [`Ordering`] arguments to describe the memory - /// ordering of this operation. The first describes the required ordering if the - /// operation succeeds while the second describes the required ordering when the - /// operation fails. Using [`Acquire`] as success ordering makes the store part + /// ordering of this operation. `success` describes the required ordering for the + /// read-modify-write operation that takes place if the comparison with `current` succeeds. + /// `failure` describes the required ordering for the load operation that takes place when + /// the comparison fails. Using [`Acquire`] as success ordering makes the store part /// of this operation [`Relaxed`], and using [`Release`] makes the successful load /// [`Relaxed`]. The failure ordering can only be [`SeqCst`], [`Acquire`] or [`Relaxed`] /// and must be equivalent to or weaker than the success ordering. @@ -1173,9 +1176,10 @@ impl AtomicPtr { /// previous value. /// /// `compare_exchange_weak` takes two [`Ordering`] arguments to describe the memory - /// ordering of this operation. The first describes the required ordering if the - /// operation succeeds while the second describes the required ordering when the - /// operation fails. Using [`Acquire`] as success ordering makes the store part + /// ordering of this operation. `success` describes the required ordering for the + /// read-modify-write operation that takes place if the comparison with `current` succeeds. + /// `failure` describes the required ordering for the load operation that takes place when + /// the comparison fails. Using [`Acquire`] as success ordering makes the store part /// of this operation [`Relaxed`], and using [`Release`] makes the successful load /// [`Relaxed`]. The failure ordering can only be [`SeqCst`], [`Acquire`] or [`Relaxed`] /// and must be equivalent to or weaker than the success ordering. @@ -1671,9 +1675,10 @@ containing the previous value. On success this value is guaranteed to be equal t `current`. `compare_exchange` takes two [`Ordering`] arguments to describe the memory -ordering of this operation. The first describes the required ordering if the -operation succeeds while the second describes the required ordering when the -operation fails. Using [`Acquire`] as success ordering makes the store part +ordering of this operation. `success` describes the required ordering for the +read-modify-write operation that takes place if the comparison with `current` succeeds. +`failure` describes the required ordering for the load operation that takes place when +the comparison fails. Using [`Acquire`] as success ordering makes the store part of this operation [`Relaxed`], and using [`Release`] makes the successful load [`Relaxed`]. The failure ordering can only be [`SeqCst`], [`Acquire`] or [`Relaxed`] and must be equivalent to or weaker than the success ordering. @@ -1723,9 +1728,10 @@ platforms. The return value is a result indicating whether the new value was written and containing the previous value. `compare_exchange_weak` takes two [`Ordering`] arguments to describe the memory -ordering of this operation. The first describes the required ordering if the -operation succeeds while the second describes the required ordering when the -operation fails. Using [`Acquire`] as success ordering makes the store part +ordering of this operation. `success` describes the required ordering for the +read-modify-write operation that takes place if the comparison with `current` succeeds. +`failure` describes the required ordering for the load operation that takes place when +the comparison fails. Using [`Acquire`] as success ordering makes the store part of this operation [`Relaxed`], and using [`Release`] makes the successful load [`Relaxed`]. The failure ordering can only be [`SeqCst`], [`Acquire`] or [`Relaxed`] and must be equivalent to or weaker than the success ordering. From 50b2ade8adecdfaa9de710d10578fa34dc969fe7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Mon, 23 Nov 2020 00:58:09 +0100 Subject: [PATCH 07/28] Revert the usage of compare_exchange_weak --- library/std/src/sys_common/thread_parker/futex.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys_common/thread_parker/futex.rs b/library/std/src/sys_common/thread_parker/futex.rs index c2ad6d55dc1f2..0132743b24404 100644 --- a/library/std/src/sys_common/thread_parker/futex.rs +++ b/library/std/src/sys_common/thread_parker/futex.rs @@ -49,7 +49,7 @@ impl Parker { // Wait for something to happen, assuming it's still set to PARKED. futex_wait(&self.state, PARKED, None); // Change NOTIFIED=>EMPTY and return in that case. - if self.state.compare_exchange_weak(NOTIFIED, EMPTY, Acquire, Acquire).is_ok() { + if self.state.compare_exchange(NOTIFIED, EMPTY, Acquire, Acquire).is_ok() { return; } else { // Spurious wake up. We loop to try again. From 3b8617b9b6718f04d412ffae52337053e79c3c6b Mon Sep 17 00:00:00 2001 From: Albin Hedman Date: Thu, 26 Nov 2020 23:22:36 +0100 Subject: [PATCH 08/28] Added [T; N]::zip() --- library/core/src/array/mod.rs | 28 ++++++++++++++++++++++++++++ library/core/tests/array.rs | 8 ++++++++ library/core/tests/lib.rs | 1 + 3 files changed, 37 insertions(+) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index a7cb1023229bb..02b771863e92b 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -463,6 +463,34 @@ impl [T; N] { unsafe { crate::mem::transmute_copy::<_, [U; N]>(&dst) } } + /// 'Zips up' two arrays into a single array of pairs. + /// `zip()` returns a new array where every element is a tuple where the first element comes from the first array, and the second element comes from the second array. + /// In other words, it zips two arrays together, into a single one. + /// + /// # Examples + /// + /// ``` + /// #![feature(array_zip)] + /// let x = [1, 2, 3]; + /// let y = [4, 5, 6]; + /// let z = x.zip(y); + /// assert_eq!(z, [(1, 4), (2, 5), (3, 6)]); + /// ``` + #[unstable(feature = "array_zip", issue = "none")] + pub fn zip(self, rhs: [U; N]) -> [(T, U); N] { + use crate::mem::MaybeUninit; + + let mut dst = MaybeUninit::uninit_array::(); + for ((lhs, rhs), dst) in IntoIter::new(self).zip(IntoIter::new(rhs)).zip(&mut dst) { + dst.write((lhs, rhs)); + } + // FIXME: Convert to crate::mem::transmute once it works with generics. + // unsafe { crate::mem::transmute::<[MaybeUninit; N], [U; N]>(dst) } + // SAFETY: At this point we've properly initialized the whole array + // and we just need to cast it to the correct type. + unsafe { crate::mem::transmute_copy::<_, [(T, U); N]>(&dst) } + } + /// Returns a slice containing the entire array. Equivalent to `&s[..]`. #[unstable(feature = "array_methods", issue = "76118")] pub fn as_slice(&self) -> &[T] { diff --git a/library/core/tests/array.rs b/library/core/tests/array.rs index 89c2a969c28bb..9005e3d8f43e3 100644 --- a/library/core/tests/array.rs +++ b/library/core/tests/array.rs @@ -317,6 +317,14 @@ fn array_map() { assert_eq!(b, [1, 2, 3]); } +#[test] +fn array_zip() { + let a = [1, 2, 3]; + let b = [4, 5, 6]; + let c = a.zip(b); + assert_eq!(c, [(1, 4), (2, 5), (3, 6)]); +} + // See note on above test for why `should_panic` is used. #[test] #[should_panic(expected = "test succeeded")] diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index 1efb3b7411898..2daf2650b5822 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -3,6 +3,7 @@ #![feature(array_from_ref)] #![feature(array_methods)] #![feature(array_map)] +#![feature(array_zip)] #![feature(array_windows)] #![feature(bool_to_option)] #![feature(bound_cloned)] From 2f35fb1e116d5da9e851c1a298390078ea7089ed Mon Sep 17 00:00:00 2001 From: Albin Hedman Date: Fri, 27 Nov 2020 11:46:49 +0100 Subject: [PATCH 09/28] Remove redundant tests --- library/core/tests/array.rs | 8 -------- library/core/tests/lib.rs | 1 - 2 files changed, 9 deletions(-) diff --git a/library/core/tests/array.rs b/library/core/tests/array.rs index 9005e3d8f43e3..89c2a969c28bb 100644 --- a/library/core/tests/array.rs +++ b/library/core/tests/array.rs @@ -317,14 +317,6 @@ fn array_map() { assert_eq!(b, [1, 2, 3]); } -#[test] -fn array_zip() { - let a = [1, 2, 3]; - let b = [4, 5, 6]; - let c = a.zip(b); - assert_eq!(c, [(1, 4), (2, 5), (3, 6)]); -} - // See note on above test for why `should_panic` is used. #[test] #[should_panic(expected = "test succeeded")] diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index 2daf2650b5822..1efb3b7411898 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -3,7 +3,6 @@ #![feature(array_from_ref)] #![feature(array_methods)] #![feature(array_map)] -#![feature(array_zip)] #![feature(array_windows)] #![feature(bool_to_option)] #![feature(bound_cloned)] From 06ca6bba8dd21fe3330b2212a34c6bf244300486 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 22 Nov 2020 21:58:41 +0000 Subject: [PATCH 10/28] Add tests --- .../overlapping_range_endpoints.rs | 6 ++++++ .../overlapping_range_endpoints.stderr | 16 +++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs index af720a0569322..2463d5cf4dc94 100644 --- a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs +++ b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs @@ -33,6 +33,12 @@ fn main() { m!(0u8, 25, 20..=30); m!(0u8, 30, 20..=30); //~ ERROR multiple patterns covering the same range + match 0u8 { + 0..=10 => {} + 20..=30 => {} + 10..=20 => {} //~ ERROR multiple patterns covering the same range + _ => {} + } match (0u8, true) { (0..=10, true) => {} (10..20, true) => {} // not detected diff --git a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr index 7bb747cdf6fc1..5351a7e61c22e 100644 --- a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr +++ b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr @@ -61,7 +61,17 @@ LL | m!(0u8, 30, 20..=30); | this range overlaps on `30_u8` error: multiple patterns covering the same range - --> $DIR/overlapping_range_endpoints.rs:44:16 + --> $DIR/overlapping_range_endpoints.rs:39:9 + | +LL | 0..=10 => {} + | ------ this range overlaps on `10_u8` +LL | 20..=30 => {} + | ------- this range overlaps on `20_u8` +LL | 10..=20 => {} + | ^^^^^^^ overlapping patterns + +error: multiple patterns covering the same range + --> $DIR/overlapping_range_endpoints.rs:50:16 | LL | (true, 0..=10) => {} | ------ this range overlaps on `10_u8` @@ -69,12 +79,12 @@ LL | (true, 10..20) => {} | ^^^^^^ overlapping patterns error: multiple patterns covering the same range - --> $DIR/overlapping_range_endpoints.rs:50:14 + --> $DIR/overlapping_range_endpoints.rs:56:14 | LL | Some(0..=10) => {} | ------ this range overlaps on `10_u8` LL | Some(10..20) => {} | ^^^^^^ overlapping patterns -error: aborting due to 9 previous errors +error: aborting due to 10 previous errors From d1a50ffb7c332677f3c613540d507114c83e0f86 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 22 Oct 2020 18:34:00 +0100 Subject: [PATCH 11/28] Rename the `overlapping_patterns` lint to `overlapping_range_endpoints` --- compiler/rustc_lint/src/lib.rs | 3 +- compiler/rustc_lint_defs/src/builtin.rs | 17 ++++--- .../src/thir/pattern/deconstruct_pat.rs | 13 +++--- src/test/ui/issues/issue-21475.rs | 2 +- src/test/ui/issues/issue-26251.rs | 2 +- .../overlapping_range_endpoints.rs | 22 +++++----- .../overlapping_range_endpoints.stderr | 44 +++++++++---------- 7 files changed, 52 insertions(+), 51 deletions(-) diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 81549be4b0915..d5e77843b92df 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -283,7 +283,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) { UNUSED_MUT, UNREACHABLE_CODE, UNREACHABLE_PATTERNS, - OVERLAPPING_PATTERNS, + OVERLAPPING_RANGE_ENDPOINTS, UNUSED_MUST_USE, UNUSED_UNSAFE, PATH_STATEMENTS, @@ -335,6 +335,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) { store.register_renamed("exceeding_bitshifts", "arithmetic_overflow"); store.register_renamed("redundant_semicolon", "redundant_semicolons"); store.register_renamed("intra_doc_link_resolution_failure", "broken_intra_doc_links"); + store.register_renamed("overlapping_patterns", "overlapping_range_endpoints"); store.register_removed("unknown_features", "replaced by an error"); store.register_removed("unsigned_negation", "replaced by negate_unsigned feature gate"); store.register_removed("negate_unsigned", "cast a signed value instead"); diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index fa82dce0ae2ed..20ea7377e3ffc 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -588,8 +588,8 @@ declare_lint! { } declare_lint! { - /// The `overlapping_patterns` lint detects `match` arms that have - /// [range patterns] that overlap. + /// The `overlapping_range_endpoints` lint detects `match` arms that have [range patterns] that + /// overlap on their endpoints. /// /// [range patterns]: https://doc.rust-lang.org/nightly/reference/patterns.html#range-patterns /// @@ -607,13 +607,12 @@ declare_lint! { /// /// ### Explanation /// - /// It is likely a mistake to have range patterns in a match expression - /// that overlap. Check that the beginning and end values are what you - /// expect, and keep in mind that with `..=` the left and right bounds are - /// inclusive. - pub OVERLAPPING_PATTERNS, + /// It is likely a mistake to have range patterns in a match expression that overlap in this + /// way. Check that the beginning and end values are what you expect, and keep in mind that + /// with `..=` the left and right bounds are inclusive. + pub OVERLAPPING_RANGE_ENDPOINTS, Warn, - "detects overlapping patterns" + "detects range patterns with overlapping endpoints" } declare_lint! { @@ -2765,7 +2764,7 @@ declare_lint_pass! { DEAD_CODE, UNREACHABLE_CODE, UNREACHABLE_PATTERNS, - OVERLAPPING_PATTERNS, + OVERLAPPING_RANGE_ENDPOINTS, BINDINGS_WITH_VARIANT_NAME, UNUSED_MACROS, WARNINGS, diff --git a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs index 3b2eef5a905dd..34cb9ff1cc5fc 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs @@ -273,7 +273,7 @@ impl IntRange { let mut borders: Vec<_> = row_borders.chain(self_borders).collect(); borders.sort_unstable(); - self.lint_overlapping_patterns(pcx, hir_id, overlaps); + self.lint_overlapping_range_endpoints(pcx, hir_id, overlaps); // We're going to iterate through every adjacent pair of borders, making sure that // each represents an interval of nonnegative length, and convert each such @@ -296,7 +296,7 @@ impl IntRange { .collect() } - fn lint_overlapping_patterns( + fn lint_overlapping_range_endpoints( &self, pcx: PatCtxt<'_, '_, '_>, hir_id: Option, @@ -304,22 +304,23 @@ impl IntRange { ) { if let (true, Some(hir_id)) = (!overlaps.is_empty(), hir_id) { pcx.cx.tcx.struct_span_lint_hir( - lint::builtin::OVERLAPPING_PATTERNS, + lint::builtin::OVERLAPPING_RANGE_ENDPOINTS, hir_id, pcx.span, |lint| { - let mut err = lint.build("multiple patterns covering the same range"); - err.span_label(pcx.span, "overlapping patterns"); + let mut err = lint.build("multiple patterns overlap on their endpoints"); + err.span_label(pcx.span, "overlapping range endpoints"); for (int_range, span) in overlaps { // Use the real type for user display of the ranges: err.span_label( span, &format!( "this range overlaps on `{}`", - int_range.to_pat(pcx.cx.tcx, pcx.ty), + int_range.to_pat(pcx.cx.tcx, pcx.ty) ), ); } + // FIXME: add note err.emit(); }, ); diff --git a/src/test/ui/issues/issue-21475.rs b/src/test/ui/issues/issue-21475.rs index ab0a18869632a..b028fcae0775b 100644 --- a/src/test/ui/issues/issue-21475.rs +++ b/src/test/ui/issues/issue-21475.rs @@ -1,5 +1,5 @@ // run-pass -#![allow(unused_imports, overlapping_patterns)] +#![allow(unused_imports, overlapping_range_endpoints)] // pretty-expanded FIXME #23616 use m::{START, END}; diff --git a/src/test/ui/issues/issue-26251.rs b/src/test/ui/issues/issue-26251.rs index edb06fea8ad53..a3e26a41232c7 100644 --- a/src/test/ui/issues/issue-26251.rs +++ b/src/test/ui/issues/issue-26251.rs @@ -1,5 +1,5 @@ // run-pass -#![allow(overlapping_patterns)] +#![allow(overlapping_range_endpoints)] fn main() { let x = 'a'; diff --git a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs index 2463d5cf4dc94..6ad87d873ee6e 100644 --- a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs +++ b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs @@ -1,5 +1,5 @@ #![feature(exclusive_range_pattern)] -#![deny(overlapping_patterns)] +#![deny(overlapping_range_endpoints)] macro_rules! m { ($s:expr, $t1:pat, $t2:pat) => { @@ -12,31 +12,31 @@ macro_rules! m { } fn main() { - m!(0u8, 20..=30, 30..=40); //~ ERROR multiple patterns covering the same range - m!(0u8, 30..=40, 20..=30); //~ ERROR multiple patterns covering the same range + m!(0u8, 20..=30, 30..=40); //~ ERROR multiple patterns overlap on their endpoints + m!(0u8, 30..=40, 20..=30); //~ ERROR multiple patterns overlap on their endpoints m!(0u8, 20..=30, 31..=40); m!(0u8, 20..=30, 29..=40); - m!(0u8, 20.. 30, 29..=40); //~ ERROR multiple patterns covering the same range + m!(0u8, 20.. 30, 29..=40); //~ ERROR multiple patterns overlap on their endpoints m!(0u8, 20.. 30, 28..=40); m!(0u8, 20.. 30, 30..=40); m!(0u8, 20..=30, 30..=30); - m!(0u8, 20..=30, 30..=31); //~ ERROR multiple patterns covering the same range + m!(0u8, 20..=30, 30..=31); //~ ERROR multiple patterns overlap on their endpoints m!(0u8, 20..=30, 29..=30); m!(0u8, 20..=30, 20..=20); m!(0u8, 20..=30, 20..=21); - m!(0u8, 20..=30, 19..=20); //~ ERROR multiple patterns covering the same range + m!(0u8, 20..=30, 19..=20); //~ ERROR multiple patterns overlap on their endpoints m!(0u8, 20..=30, 20); m!(0u8, 20..=30, 25); m!(0u8, 20..=30, 30); m!(0u8, 20.. 30, 29); - m!(0u8, 20, 20..=30); //~ ERROR multiple patterns covering the same range + m!(0u8, 20, 20..=30); //~ ERROR multiple patterns overlap on their endpoints m!(0u8, 25, 20..=30); - m!(0u8, 30, 20..=30); //~ ERROR multiple patterns covering the same range + m!(0u8, 30, 20..=30); //~ ERROR multiple patterns overlap on their endpoints match 0u8 { 0..=10 => {} 20..=30 => {} - 10..=20 => {} //~ ERROR multiple patterns covering the same range + 10..=20 => {} //~ ERROR multiple patterns overlap on their endpoints _ => {} } match (0u8, true) { @@ -47,13 +47,13 @@ fn main() { } match (true, 0u8) { (true, 0..=10) => {} - (true, 10..20) => {} //~ ERROR multiple patterns covering the same range + (true, 10..20) => {} //~ ERROR multiple patterns overlap on their endpoints (false, 10..20) => {} _ => {} } match Some(0u8) { Some(0..=10) => {} - Some(10..20) => {} //~ ERROR multiple patterns covering the same range + Some(10..20) => {} //~ ERROR multiple patterns overlap on their endpoints _ => {} } } diff --git a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr index 5351a7e61c22e..56995421f2ba3 100644 --- a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr +++ b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr @@ -1,66 +1,66 @@ -error: multiple patterns covering the same range +error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:15:22 | LL | m!(0u8, 20..=30, 30..=40); - | ------- ^^^^^^^ overlapping patterns + | ------- ^^^^^^^ overlapping range endpoints | | | this range overlaps on `30_u8` | note: the lint level is defined here --> $DIR/overlapping_range_endpoints.rs:2:9 | -LL | #![deny(overlapping_patterns)] - | ^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(overlapping_range_endpoints)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: multiple patterns covering the same range +error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:16:22 | LL | m!(0u8, 30..=40, 20..=30); - | ------- ^^^^^^^ overlapping patterns + | ------- ^^^^^^^ overlapping range endpoints | | | this range overlaps on `30_u8` -error: multiple patterns covering the same range +error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:19:22 | LL | m!(0u8, 20.. 30, 29..=40); - | ------- ^^^^^^^ overlapping patterns + | ------- ^^^^^^^ overlapping range endpoints | | | this range overlaps on `29_u8` -error: multiple patterns covering the same range +error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:23:22 | LL | m!(0u8, 20..=30, 30..=31); - | ------- ^^^^^^^ overlapping patterns + | ------- ^^^^^^^ overlapping range endpoints | | | this range overlaps on `30_u8` -error: multiple patterns covering the same range +error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:27:22 | LL | m!(0u8, 20..=30, 19..=20); - | ------- ^^^^^^^ overlapping patterns + | ------- ^^^^^^^ overlapping range endpoints | | | this range overlaps on `20_u8` -error: multiple patterns covering the same range +error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:32:17 | LL | m!(0u8, 20, 20..=30); - | -- ^^^^^^^ overlapping patterns + | -- ^^^^^^^ overlapping range endpoints | | | this range overlaps on `20_u8` -error: multiple patterns covering the same range +error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:34:17 | LL | m!(0u8, 30, 20..=30); - | -- ^^^^^^^ overlapping patterns + | -- ^^^^^^^ overlapping range endpoints | | | this range overlaps on `30_u8` -error: multiple patterns covering the same range +error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:39:9 | LL | 0..=10 => {} @@ -68,23 +68,23 @@ LL | 0..=10 => {} LL | 20..=30 => {} | ------- this range overlaps on `20_u8` LL | 10..=20 => {} - | ^^^^^^^ overlapping patterns + | ^^^^^^^ overlapping range endpoints -error: multiple patterns covering the same range +error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:50:16 | LL | (true, 0..=10) => {} | ------ this range overlaps on `10_u8` LL | (true, 10..20) => {} - | ^^^^^^ overlapping patterns + | ^^^^^^ overlapping range endpoints -error: multiple patterns covering the same range +error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:56:14 | LL | Some(0..=10) => {} | ------ this range overlaps on `10_u8` LL | Some(10..20) => {} - | ^^^^^^ overlapping patterns + | ^^^^^^ overlapping range endpoints error: aborting due to 10 previous errors From c89d439bb5c18608054d767e5b472679fa2f01da Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 22 Oct 2020 19:25:55 +0100 Subject: [PATCH 12/28] Be consistent about linting singletons --- .../src/thir/pattern/deconstruct_pat.rs | 2 +- .../overlapping_range_endpoints.rs | 4 ++-- .../overlapping_range_endpoints.stderr | 18 +----------------- 3 files changed, 4 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs index 34cb9ff1cc5fc..7c466ff591cec 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs @@ -161,7 +161,7 @@ impl IntRange { // 2 -------- // 2 ------- let (lo, hi) = self.boundaries(); let (other_lo, other_hi) = other.boundaries(); - lo == other_hi || hi == other_lo + (lo == other_hi || hi == other_lo) && !self.is_singleton() && !other.is_singleton() } fn to_pat<'tcx>(&self, tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Pat<'tcx> { diff --git a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs index 6ad87d873ee6e..5ea92b07081af 100644 --- a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs +++ b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.rs @@ -29,9 +29,9 @@ fn main() { m!(0u8, 20..=30, 25); m!(0u8, 20..=30, 30); m!(0u8, 20.. 30, 29); - m!(0u8, 20, 20..=30); //~ ERROR multiple patterns overlap on their endpoints + m!(0u8, 20, 20..=30); m!(0u8, 25, 20..=30); - m!(0u8, 30, 20..=30); //~ ERROR multiple patterns overlap on their endpoints + m!(0u8, 30, 20..=30); match 0u8 { 0..=10 => {} diff --git a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr index 56995421f2ba3..f694e4c9aab83 100644 --- a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr +++ b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr @@ -44,22 +44,6 @@ LL | m!(0u8, 20..=30, 19..=20); | | | this range overlaps on `20_u8` -error: multiple patterns overlap on their endpoints - --> $DIR/overlapping_range_endpoints.rs:32:17 - | -LL | m!(0u8, 20, 20..=30); - | -- ^^^^^^^ overlapping range endpoints - | | - | this range overlaps on `20_u8` - -error: multiple patterns overlap on their endpoints - --> $DIR/overlapping_range_endpoints.rs:34:17 - | -LL | m!(0u8, 30, 20..=30); - | -- ^^^^^^^ overlapping range endpoints - | | - | this range overlaps on `30_u8` - error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:39:9 | @@ -86,5 +70,5 @@ LL | Some(0..=10) => {} LL | Some(10..20) => {} | ^^^^^^ overlapping range endpoints -error: aborting due to 10 previous errors +error: aborting due to 8 previous errors From 94ad5e167281d212ce3b32868717fde92501d04d Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 22 Oct 2020 19:32:46 +0100 Subject: [PATCH 13/28] Improve error message --- .../src/thir/pattern/deconstruct_pat.rs | 6 +-- .../overlapping_range_endpoints.stderr | 49 ++++++++++++------- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs index 7c466ff591cec..5b1dbabe9a174 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs @@ -309,18 +309,18 @@ impl IntRange { pcx.span, |lint| { let mut err = lint.build("multiple patterns overlap on their endpoints"); - err.span_label(pcx.span, "overlapping range endpoints"); + err.span_label(pcx.span, "... with this range"); for (int_range, span) in overlaps { // Use the real type for user display of the ranges: err.span_label( span, &format!( - "this range overlaps on `{}`", + "this range overlaps on `{}`...", int_range.to_pat(pcx.cx.tcx, pcx.ty) ), ); } - // FIXME: add note + err.note("this is likely to be a mistake"); err.emit(); }, ); diff --git a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr index f694e4c9aab83..045c487873a35 100644 --- a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr +++ b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr @@ -2,73 +2,88 @@ error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:15:22 | LL | m!(0u8, 20..=30, 30..=40); - | ------- ^^^^^^^ overlapping range endpoints + | ------- ^^^^^^^ ... with this range | | - | this range overlaps on `30_u8` + | this range overlaps on `30_u8`... | note: the lint level is defined here --> $DIR/overlapping_range_endpoints.rs:2:9 | LL | #![deny(overlapping_range_endpoints)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this is likely to be a mistake error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:16:22 | LL | m!(0u8, 30..=40, 20..=30); - | ------- ^^^^^^^ overlapping range endpoints + | ------- ^^^^^^^ ... with this range | | - | this range overlaps on `30_u8` + | this range overlaps on `30_u8`... + | + = note: this is likely to be a mistake error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:19:22 | LL | m!(0u8, 20.. 30, 29..=40); - | ------- ^^^^^^^ overlapping range endpoints + | ------- ^^^^^^^ ... with this range | | - | this range overlaps on `29_u8` + | this range overlaps on `29_u8`... + | + = note: this is likely to be a mistake error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:23:22 | LL | m!(0u8, 20..=30, 30..=31); - | ------- ^^^^^^^ overlapping range endpoints + | ------- ^^^^^^^ ... with this range | | - | this range overlaps on `30_u8` + | this range overlaps on `30_u8`... + | + = note: this is likely to be a mistake error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:27:22 | LL | m!(0u8, 20..=30, 19..=20); - | ------- ^^^^^^^ overlapping range endpoints + | ------- ^^^^^^^ ... with this range | | - | this range overlaps on `20_u8` + | this range overlaps on `20_u8`... + | + = note: this is likely to be a mistake error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:39:9 | LL | 0..=10 => {} - | ------ this range overlaps on `10_u8` + | ------ this range overlaps on `10_u8`... LL | 20..=30 => {} - | ------- this range overlaps on `20_u8` + | ------- this range overlaps on `20_u8`... LL | 10..=20 => {} - | ^^^^^^^ overlapping range endpoints + | ^^^^^^^ ... with this range + | + = note: this is likely to be a mistake error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:50:16 | LL | (true, 0..=10) => {} - | ------ this range overlaps on `10_u8` + | ------ this range overlaps on `10_u8`... LL | (true, 10..20) => {} - | ^^^^^^ overlapping range endpoints + | ^^^^^^ ... with this range + | + = note: this is likely to be a mistake error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:56:14 | LL | Some(0..=10) => {} - | ------ this range overlaps on `10_u8` + | ------ this range overlaps on `10_u8`... LL | Some(10..20) => {} - | ^^^^^^ overlapping range endpoints + | ^^^^^^ ... with this range + | + = note: this is likely to be a mistake error: aborting due to 8 previous errors From 5687c162792db8d267f80de7bd56c319e12efead Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 22 Nov 2020 22:21:09 +0000 Subject: [PATCH 14/28] `overlapping_range_endpoints` does not belong in the `unused` lint group --- compiler/rustc_lint/src/lib.rs | 1 - .../integer-ranges/exhaustiveness.rs | 1 + .../integer-ranges/exhaustiveness.stderr | 24 ++++----- .../usefulness/integer-ranges/reachability.rs | 1 + .../integer-ranges/reachability.stderr | 50 +++++++++---------- 5 files changed, 39 insertions(+), 38 deletions(-) diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index d5e77843b92df..f87796a95e934 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -283,7 +283,6 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) { UNUSED_MUT, UNREACHABLE_CODE, UNREACHABLE_PATTERNS, - OVERLAPPING_RANGE_ENDPOINTS, UNUSED_MUST_USE, UNUSED_UNSAFE, PATH_STATEMENTS, diff --git a/src/test/ui/pattern/usefulness/integer-ranges/exhaustiveness.rs b/src/test/ui/pattern/usefulness/integer-ranges/exhaustiveness.rs index 5a44dfc28bb45..ef573db821046 100644 --- a/src/test/ui/pattern/usefulness/integer-ranges/exhaustiveness.rs +++ b/src/test/ui/pattern/usefulness/integer-ranges/exhaustiveness.rs @@ -1,5 +1,6 @@ #![feature(exclusive_range_pattern)] #![feature(assoc_char_consts)] +#![allow(overlapping_range_endpoints)] #![deny(unreachable_patterns)] macro_rules! m { diff --git a/src/test/ui/pattern/usefulness/integer-ranges/exhaustiveness.stderr b/src/test/ui/pattern/usefulness/integer-ranges/exhaustiveness.stderr index 2e0023348e4d8..b1440375494b1 100644 --- a/src/test/ui/pattern/usefulness/integer-ranges/exhaustiveness.stderr +++ b/src/test/ui/pattern/usefulness/integer-ranges/exhaustiveness.stderr @@ -1,5 +1,5 @@ error[E0004]: non-exhaustive patterns: `u8::MAX` not covered - --> $DIR/exhaustiveness.rs:47:8 + --> $DIR/exhaustiveness.rs:48:8 | LL | m!(0u8, 0..255); | ^^^ pattern `u8::MAX` not covered @@ -8,7 +8,7 @@ LL | m!(0u8, 0..255); = note: the matched value is of type `u8` error[E0004]: non-exhaustive patterns: `u8::MAX` not covered - --> $DIR/exhaustiveness.rs:48:8 + --> $DIR/exhaustiveness.rs:49:8 | LL | m!(0u8, 0..=254); | ^^^ pattern `u8::MAX` not covered @@ -17,7 +17,7 @@ LL | m!(0u8, 0..=254); = note: the matched value is of type `u8` error[E0004]: non-exhaustive patterns: `0_u8` not covered - --> $DIR/exhaustiveness.rs:49:8 + --> $DIR/exhaustiveness.rs:50:8 | LL | m!(0u8, 1..=255); | ^^^ pattern `0_u8` not covered @@ -26,7 +26,7 @@ LL | m!(0u8, 1..=255); = note: the matched value is of type `u8` error[E0004]: non-exhaustive patterns: `42_u8` not covered - --> $DIR/exhaustiveness.rs:50:8 + --> $DIR/exhaustiveness.rs:51:8 | LL | m!(0u8, 0..42 | 43..=255); | ^^^ pattern `42_u8` not covered @@ -35,7 +35,7 @@ LL | m!(0u8, 0..42 | 43..=255); = note: the matched value is of type `u8` error[E0004]: non-exhaustive patterns: `i8::MAX` not covered - --> $DIR/exhaustiveness.rs:51:8 + --> $DIR/exhaustiveness.rs:52:8 | LL | m!(0i8, -128..127); | ^^^ pattern `i8::MAX` not covered @@ -44,7 +44,7 @@ LL | m!(0i8, -128..127); = note: the matched value is of type `i8` error[E0004]: non-exhaustive patterns: `i8::MAX` not covered - --> $DIR/exhaustiveness.rs:52:8 + --> $DIR/exhaustiveness.rs:53:8 | LL | m!(0i8, -128..=126); | ^^^ pattern `i8::MAX` not covered @@ -53,7 +53,7 @@ LL | m!(0i8, -128..=126); = note: the matched value is of type `i8` error[E0004]: non-exhaustive patterns: `i8::MIN` not covered - --> $DIR/exhaustiveness.rs:53:8 + --> $DIR/exhaustiveness.rs:54:8 | LL | m!(0i8, -127..=127); | ^^^ pattern `i8::MIN` not covered @@ -62,7 +62,7 @@ LL | m!(0i8, -127..=127); = note: the matched value is of type `i8` error[E0004]: non-exhaustive patterns: `0_i8` not covered - --> $DIR/exhaustiveness.rs:54:11 + --> $DIR/exhaustiveness.rs:55:11 | LL | match 0i8 { | ^^^ pattern `0_i8` not covered @@ -71,7 +71,7 @@ LL | match 0i8 { = note: the matched value is of type `i8` error[E0004]: non-exhaustive patterns: `u128::MAX` not covered - --> $DIR/exhaustiveness.rs:59:8 + --> $DIR/exhaustiveness.rs:60:8 | LL | m!(0u128, 0..=ALMOST_MAX); | ^^^^^ pattern `u128::MAX` not covered @@ -80,7 +80,7 @@ LL | m!(0u128, 0..=ALMOST_MAX); = note: the matched value is of type `u128` error[E0004]: non-exhaustive patterns: `5_u128..=u128::MAX` not covered - --> $DIR/exhaustiveness.rs:60:8 + --> $DIR/exhaustiveness.rs:61:8 | LL | m!(0u128, 0..=4); | ^^^^^ pattern `5_u128..=u128::MAX` not covered @@ -89,7 +89,7 @@ LL | m!(0u128, 0..=4); = note: the matched value is of type `u128` error[E0004]: non-exhaustive patterns: `0_u128` not covered - --> $DIR/exhaustiveness.rs:61:8 + --> $DIR/exhaustiveness.rs:62:8 | LL | m!(0u128, 1..=u128::MAX); | ^^^^^ pattern `0_u128` not covered @@ -98,7 +98,7 @@ LL | m!(0u128, 1..=u128::MAX); = note: the matched value is of type `u128` error[E0004]: non-exhaustive patterns: `(126_u8..=127_u8, false)` not covered - --> $DIR/exhaustiveness.rs:69:11 + --> $DIR/exhaustiveness.rs:70:11 | LL | match (0u8, true) { | ^^^^^^^^^^^ pattern `(126_u8..=127_u8, false)` not covered diff --git a/src/test/ui/pattern/usefulness/integer-ranges/reachability.rs b/src/test/ui/pattern/usefulness/integer-ranges/reachability.rs index 6516925e93918..fb4d59b05780e 100644 --- a/src/test/ui/pattern/usefulness/integer-ranges/reachability.rs +++ b/src/test/ui/pattern/usefulness/integer-ranges/reachability.rs @@ -1,4 +1,5 @@ #![feature(exclusive_range_pattern)] +#![allow(overlapping_range_endpoints)] #![deny(unreachable_patterns)] macro_rules! m { diff --git a/src/test/ui/pattern/usefulness/integer-ranges/reachability.stderr b/src/test/ui/pattern/usefulness/integer-ranges/reachability.stderr index e6878d950d625..9a02fac6a75dd 100644 --- a/src/test/ui/pattern/usefulness/integer-ranges/reachability.stderr +++ b/src/test/ui/pattern/usefulness/integer-ranges/reachability.stderr @@ -1,149 +1,149 @@ error: unreachable pattern - --> $DIR/reachability.rs:16:17 + --> $DIR/reachability.rs:17:17 | LL | m!(0u8, 42, 42); | ^^ | note: the lint level is defined here - --> $DIR/reachability.rs:2:9 + --> $DIR/reachability.rs:3:9 | LL | #![deny(unreachable_patterns)] | ^^^^^^^^^^^^^^^^^^^^ error: unreachable pattern - --> $DIR/reachability.rs:20:22 + --> $DIR/reachability.rs:21:22 | LL | m!(0u8, 20..=30, 20); | ^^ error: unreachable pattern - --> $DIR/reachability.rs:21:22 + --> $DIR/reachability.rs:22:22 | LL | m!(0u8, 20..=30, 21); | ^^ error: unreachable pattern - --> $DIR/reachability.rs:22:22 + --> $DIR/reachability.rs:23:22 | LL | m!(0u8, 20..=30, 25); | ^^ error: unreachable pattern - --> $DIR/reachability.rs:23:22 + --> $DIR/reachability.rs:24:22 | LL | m!(0u8, 20..=30, 29); | ^^ error: unreachable pattern - --> $DIR/reachability.rs:24:22 + --> $DIR/reachability.rs:25:22 | LL | m!(0u8, 20..=30, 30); | ^^ error: unreachable pattern - --> $DIR/reachability.rs:27:21 + --> $DIR/reachability.rs:28:21 | LL | m!(0u8, 20..30, 20); | ^^ error: unreachable pattern - --> $DIR/reachability.rs:28:21 + --> $DIR/reachability.rs:29:21 | LL | m!(0u8, 20..30, 21); | ^^ error: unreachable pattern - --> $DIR/reachability.rs:29:21 + --> $DIR/reachability.rs:30:21 | LL | m!(0u8, 20..30, 25); | ^^ error: unreachable pattern - --> $DIR/reachability.rs:30:21 + --> $DIR/reachability.rs:31:21 | LL | m!(0u8, 20..30, 29); | ^^ error: unreachable pattern - --> $DIR/reachability.rs:34:22 + --> $DIR/reachability.rs:35:22 | LL | m!(0u8, 20..=30, 20..=30); | ^^^^^^^ error: unreachable pattern - --> $DIR/reachability.rs:35:22 + --> $DIR/reachability.rs:36:22 | LL | m!(0u8, 20.. 30, 20.. 30); | ^^^^^^^ error: unreachable pattern - --> $DIR/reachability.rs:36:22 + --> $DIR/reachability.rs:37:22 | LL | m!(0u8, 20..=30, 20.. 30); | ^^^^^^^ error: unreachable pattern - --> $DIR/reachability.rs:38:22 + --> $DIR/reachability.rs:39:22 | LL | m!(0u8, 20..=30, 21..=30); | ^^^^^^^ error: unreachable pattern - --> $DIR/reachability.rs:39:22 + --> $DIR/reachability.rs:40:22 | LL | m!(0u8, 20..=30, 20..=29); | ^^^^^^^ error: unreachable pattern - --> $DIR/reachability.rs:41:24 + --> $DIR/reachability.rs:42:24 | LL | m!('a', 'A'..='z', 'a'..='z'); | ^^^^^^^^^ error: unreachable pattern - --> $DIR/reachability.rs:48:9 + --> $DIR/reachability.rs:49:9 | LL | 5..=8 => {}, | ^^^^^ error: unreachable pattern - --> $DIR/reachability.rs:54:9 + --> $DIR/reachability.rs:55:9 | LL | 5..15 => {}, | ^^^^^ error: unreachable pattern - --> $DIR/reachability.rs:61:9 + --> $DIR/reachability.rs:62:9 | LL | 5..25 => {}, | ^^^^^ error: unreachable pattern - --> $DIR/reachability.rs:69:9 + --> $DIR/reachability.rs:70:9 | LL | 5..25 => {}, | ^^^^^ error: unreachable pattern - --> $DIR/reachability.rs:75:9 + --> $DIR/reachability.rs:76:9 | LL | 5..15 => {}, | ^^^^^ error: unreachable pattern - --> $DIR/reachability.rs:82:9 + --> $DIR/reachability.rs:83:9 | LL | '\u{D7FF}'..='\u{E000}' => {}, | ^^^^^^^^^^^^^^^^^^^^^^^ error: unreachable pattern - --> $DIR/reachability.rs:103:9 + --> $DIR/reachability.rs:104:9 | LL | &FOO => {} | ^^^^ error: unreachable pattern - --> $DIR/reachability.rs:104:9 + --> $DIR/reachability.rs:105:9 | LL | BAR => {} | ^^^ From 0e0ae47af9843cddb5f67cc060cc163871da6943 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Tue, 15 Dec 2020 19:22:23 +0100 Subject: [PATCH 15/28] Add a new lint to rustdoc for invalid codeblocks This will make rustdoc behave properly when -Dwarnings is given --- compiler/rustc_lint/src/lib.rs | 3 +- compiler/rustc_lint_defs/src/builtin.rs | 13 ++++ src/doc/rustdoc/src/lints.md | 35 +++++++++ src/librustdoc/core.rs | 2 + .../passes/check_code_block_syntax.rs | 71 +++++++++++++------ src/test/rustdoc-ui/invalid-syntax.stderr | 1 + 6 files changed, 102 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 80ef855c3859e..d1a9d518fae87 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -70,7 +70,7 @@ use rustc_middle::ty::TyCtxt; use rustc_session::lint::builtin::{ BARE_TRAIT_OBJECTS, BROKEN_INTRA_DOC_LINKS, ELIDED_LIFETIMES_IN_PATHS, EXPLICIT_OUTLIVES_REQUIREMENTS, INVALID_CODEBLOCK_ATTRIBUTES, INVALID_HTML_TAGS, - MISSING_DOC_CODE_EXAMPLES, NON_AUTOLINKS, PRIVATE_DOC_TESTS, + INVALID_RUST_CODEBLOCK, MISSING_DOC_CODE_EXAMPLES, NON_AUTOLINKS, PRIVATE_DOC_TESTS, }; use rustc_span::symbol::{Ident, Symbol}; use rustc_span::Span; @@ -320,6 +320,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) { BROKEN_INTRA_DOC_LINKS, PRIVATE_INTRA_DOC_LINKS, INVALID_CODEBLOCK_ATTRIBUTES, + INVALID_RUST_CODEBLOCK, MISSING_DOC_CODE_EXAMPLES, PRIVATE_DOC_TESTS, INVALID_HTML_TAGS diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index fa82dce0ae2ed..90c5348c6ea6e 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -1847,6 +1847,18 @@ declare_lint! { "codeblock attribute looks a lot like a known one" } +declare_lint! { + /// The `invalid_rust_codeblock` lint detects Rust code blocks in + /// documentation examples that are invalid (e.g. empty, not parsable as + /// Rust code). This is a `rustdoc` only lint, see the documentation in the + /// [rustdoc book]. + /// + /// [rustdoc book]: ../../../rustdoc/lints.html#invalid_rust_codeblock + pub INVALID_RUST_CODEBLOCK, + Warn, + "codeblock could not be parsed as valid Rust or is empty" +} + declare_lint! { /// The `missing_crate_level_docs` lint detects if documentation is /// missing at the crate root. This is a `rustdoc` only lint, see the @@ -2803,6 +2815,7 @@ declare_lint_pass! { BROKEN_INTRA_DOC_LINKS, PRIVATE_INTRA_DOC_LINKS, INVALID_CODEBLOCK_ATTRIBUTES, + INVALID_RUST_CODEBLOCK, MISSING_CRATE_LEVEL_DOCS, MISSING_DOC_CODE_EXAMPLES, INVALID_HTML_TAGS, diff --git a/src/doc/rustdoc/src/lints.md b/src/doc/rustdoc/src/lints.md index 41292b3d83841..94e0cbe223a95 100644 --- a/src/doc/rustdoc/src/lints.md +++ b/src/doc/rustdoc/src/lints.md @@ -285,6 +285,41 @@ warning: unclosed HTML tag `h1` warning: 2 warnings emitted ``` + +## invalid_rust_codeblock + +This lint **warns by default**. It detects Rust code blocks in documentation +examples that are invalid (e.g. empty, not parsable as Rust). For example: + +```rust +/// Empty code block, with and without Rust: +/// +/// ```rust +/// ``` +/// +/// Unclosed code block: +/// +/// ```rust +fn main() {} +``` + +Which will give: + +```text +warning: Rust code block is empty + --> src/lib.rs:3:5 + | +3 | /// ```rust + | _____^ +4 | | /// ``` + | |_______^ + +warning: Rust code block is empty + --> src/lib.rs:8:5 + | +8 | /// ```rust + | ^^^^^^^ +``` ## non_autolinks diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 9c693c290e7f4..438ac63f0da41 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -322,6 +322,7 @@ crate fn run_core( let private_doc_tests = rustc_lint::builtin::PRIVATE_DOC_TESTS.name; let no_crate_level_docs = rustc_lint::builtin::MISSING_CRATE_LEVEL_DOCS.name; let invalid_codeblock_attributes_name = rustc_lint::builtin::INVALID_CODEBLOCK_ATTRIBUTES.name; + let invalid_rust_codeblock = rustc_lint::builtin::INVALID_RUST_CODEBLOCK.name; let invalid_html_tags = rustc_lint::builtin::INVALID_HTML_TAGS.name; let renamed_and_removed_lints = rustc_lint::builtin::RENAMED_AND_REMOVED_LINTS.name; let non_autolinks = rustc_lint::builtin::NON_AUTOLINKS.name; @@ -337,6 +338,7 @@ crate fn run_core( private_doc_tests.to_owned(), no_crate_level_docs.to_owned(), invalid_codeblock_attributes_name.to_owned(), + invalid_rust_codeblock.to_owned(), invalid_html_tags.to_owned(), renamed_and_removed_lints.to_owned(), unknown_lints.to_owned(), diff --git a/src/librustdoc/passes/check_code_block_syntax.rs b/src/librustdoc/passes/check_code_block_syntax.rs index 0c76dc571beee..323b140847148 100644 --- a/src/librustdoc/passes/check_code_block_syntax.rs +++ b/src/librustdoc/passes/check_code_block_syntax.rs @@ -1,6 +1,8 @@ use rustc_data_structures::sync::{Lock, Lrc}; use rustc_errors::{emitter::Emitter, Applicability, Diagnostic, Handler}; +use rustc_middle::lint::LintDiagnosticBuilder; use rustc_parse::parse_stream_from_source_str; +use rustc_session::lint; use rustc_session::parse::ParseSess; use rustc_span::source_map::{FilePathMapping, SourceMap}; use rustc_span::{FileName, InnerSpan}; @@ -47,51 +49,76 @@ impl<'a, 'tcx> SyntaxChecker<'a, 'tcx> { .unwrap_or(false); let buffer = buffer.borrow(); - if buffer.has_errors || is_empty { - let mut diag = if let Some(sp) = - super::source_span_for_markdown_range(self.cx, &dox, &code_block.range, &item.attrs) - { - let warning_message = if buffer.has_errors { + if !(buffer.has_errors || is_empty) { + // No errors in a non-empty program. + return; + } + + let local_id = match item.def_id.as_local() { + Some(id) => id, + // We don't need to check the syntax for other crates so returning + // without doing anything should not be a problem. + None => return, + }; + + let hir_id = self.cx.tcx.hir().local_def_id_to_hir_id(local_id); + let mark_with_text = code_block.syntax.is_none() && code_block.is_fenced; + + // The span and whether it is precise or not. + let (sp, precise_span) = match super::source_span_for_markdown_range( + self.cx, + &dox, + &code_block.range, + &item.attrs, + ) { + Some(sp) => (sp, true), + None => (super::span_of_attrs(&item.attrs).unwrap_or(item.source.span()), false), + }; + + // lambda that will use the lint to start a new diagnostic and add + // a suggestion to it when needed. + let diag_builder = |lint: LintDiagnosticBuilder<'_>| { + let mut diag = if precise_span { + let msg = if buffer.has_errors { "could not parse code block as Rust code" } else { "Rust code block is empty" }; - let mut diag = self.cx.sess().struct_span_warn(sp, warning_message); - - if code_block.syntax.is_none() && code_block.is_fenced { - let sp = sp.from_inner(InnerSpan::new(0, 3)); + let mut diag = lint.build(msg); + if mark_with_text { diag.span_suggestion( - sp, + sp.from_inner(InnerSpan::new(0, 3)), "mark blocks that do not contain Rust code as text", String::from("```text"), Applicability::MachineApplicable, ); } - diag } else { - // We couldn't calculate the span of the markdown block that had the error, so our - // diagnostics are going to be a bit lacking. - let mut diag = self.cx.sess().struct_span_warn( - super::span_of_attrs(&item.attrs).unwrap_or(item.source.span()), - "doc comment contains an invalid Rust code block", - ); - - if code_block.syntax.is_none() && code_block.is_fenced { + let mut diag = lint.build("doc comment contains an invalid Rust code block"); + if mark_with_text { diag.help("mark blocks that do not contain Rust code as text: ```text"); } diag }; - // FIXME(#67563): Provide more context for these errors by displaying the spans inline. for message in buffer.messages.iter() { diag.note(&message); } - diag.emit(); - } + }; + + // Finally build and emit the completed diagnostic. + // All points of divergence have been handled earlier so this can be + // done the same way whether the span is precise or not. + self.cx.tcx.struct_span_lint_hir( + lint::builtin::INVALID_RUST_CODEBLOCK, + hir_id, + sp, + diag_builder, + ); } } diff --git a/src/test/rustdoc-ui/invalid-syntax.stderr b/src/test/rustdoc-ui/invalid-syntax.stderr index 9a7a4d2101362..a55a0b1d90e24 100644 --- a/src/test/rustdoc-ui/invalid-syntax.stderr +++ b/src/test/rustdoc-ui/invalid-syntax.stderr @@ -7,6 +7,7 @@ LL | | /// \__________pkt->size___________/ \_result->size_/ \__pkt->si LL | | /// ``` | |_______^ | + = note: `#[warn(invalid_rust_codeblock)]` on by default = note: error from rustc: unknown start of token: \ = note: error from rustc: unknown start of token: \ = note: error from rustc: unknown start of token: \ From be2c8f2d436d777cbdac73f6dd238ea135048ff5 Mon Sep 17 00:00:00 2001 From: Albin Hedman Date: Wed, 16 Dec 2020 18:32:29 +0100 Subject: [PATCH 16/28] Update zip for better codegen, see discussion --- library/core/src/array/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index 02b771863e92b..9de87922f54f2 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -481,8 +481,8 @@ impl [T; N] { use crate::mem::MaybeUninit; let mut dst = MaybeUninit::uninit_array::(); - for ((lhs, rhs), dst) in IntoIter::new(self).zip(IntoIter::new(rhs)).zip(&mut dst) { - dst.write((lhs, rhs)); + for (i, (lhs, rhs)) in IntoIter::new(self).zip(IntoIter::new(rhs)).enumerate() { + dst[i].write((lhs, rhs)); } // FIXME: Convert to crate::mem::transmute once it works with generics. // unsafe { crate::mem::transmute::<[MaybeUninit; N], [U; N]>(dst) } From baa5e47106cd3544f4eb06317eea1a174d6e2341 Mon Sep 17 00:00:00 2001 From: Albin Hedman Date: Wed, 16 Dec 2020 21:12:10 +0100 Subject: [PATCH 17/28] Update doc comment Co-authored-by: Mara Bos --- library/core/src/array/mod.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index 9de87922f54f2..6791f0e744427 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -464,8 +464,11 @@ impl [T; N] { } /// 'Zips up' two arrays into a single array of pairs. - /// `zip()` returns a new array where every element is a tuple where the first element comes from the first array, and the second element comes from the second array. - /// In other words, it zips two arrays together, into a single one. + /// + /// `zip()` returns a new array where every element is a tuple where the + /// first element comes from the first array, and the second element comes + /// from the second array. In other words, it zips two arrays together, + /// into a single one. /// /// # Examples /// From 8b3725973a696f8dd77e17127863ec56512322cb Mon Sep 17 00:00:00 2001 From: Albin Hedman Date: Thu, 17 Dec 2020 00:27:21 +0100 Subject: [PATCH 18/28] Added reference to tracking issue --- library/core/src/array/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index 6791f0e744427..71548bec7aaee 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -479,7 +479,7 @@ impl [T; N] { /// let z = x.zip(y); /// assert_eq!(z, [(1, 4), (2, 5), (3, 6)]); /// ``` - #[unstable(feature = "array_zip", issue = "none")] + #[unstable(feature = "array_zip", issue = "80094")] pub fn zip(self, rhs: [U; N]) -> [(T, U); N] { use crate::mem::MaybeUninit; From 830ceaa41908bd428e36b1a804dd93c9a257aea8 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Sun, 6 Dec 2020 13:57:37 +0100 Subject: [PATCH 19/28] Remap instrument-coverage line numbers in doctests This uses the `SourceMap::doctest_offset_line` method to re-map line numbers from doctests. Remapping columns is not yet done. Part of issue #79417. --- .../rustc_mir/src/transform/coverage/mod.rs | 6 +- compiler/rustc_span/src/lib.rs | 2 +- src/librustdoc/doctest.rs | 75 +++++--- src/librustdoc/doctest/tests.rs | 69 +++++-- src/librustdoc/html/markdown.rs | 2 +- .../coverage-reports/Makefile | 22 ++- .../expected_show_coverage.doctest.txt | 79 ++++++++ .../expected_show_coverage.uses_crate.txt | 4 +- .../coverage-reports/normalize_paths.py | 10 + ...est.main.-------.InstrumentCoverage.0.html | 127 +++++++++++++ ...doctests.-------.InstrumentCoverage.0.html | 173 ++++++++++++++++++ .../coverage/compiletest-ignore-dir | 4 +- .../coverage/coverage_tools.mk | 4 +- .../run-make-fulldeps/coverage/doctest.rs | 66 +++++++ .../coverage/lib/doctest_crate.rs | 9 + 15 files changed, 596 insertions(+), 56 deletions(-) create mode 100644 src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt create mode 100644 src/test/run-make-fulldeps/coverage-reports/normalize_paths.py create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.doctest/doctest.main.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.doctest_crate/doctest_crate.fn_run_in_doctests.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage/doctest.rs create mode 100644 src/test/run-make-fulldeps/coverage/lib/doctest_crate.rs diff --git a/compiler/rustc_mir/src/transform/coverage/mod.rs b/compiler/rustc_mir/src/transform/coverage/mod.rs index 4590d37c182e4..93133e9b7f063 100644 --- a/compiler/rustc_mir/src/transform/coverage/mod.rs +++ b/compiler/rustc_mir/src/transform/coverage/mod.rs @@ -30,6 +30,7 @@ use rustc_middle::mir::{ }; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::DefId; +use rustc_span::source_map::SourceMap; use rustc_span::{CharPos, Pos, SourceFile, Span, Symbol}; /// A simple error message wrapper for `coverage::Error`s. @@ -311,7 +312,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { self.mir_body, counter_kind, self.bcb_leader_bb(bcb), - Some(make_code_region(file_name, &self.source_file, span, body_span)), + Some(make_code_region(source_map, file_name, &self.source_file, span, body_span)), ); } } @@ -489,6 +490,7 @@ fn inject_intermediate_expression(mir_body: &mut mir::Body<'tcx>, expression: Co /// Convert the Span into its file name, start line and column, and end line and column fn make_code_region( + source_map: &SourceMap, file_name: Symbol, source_file: &Lrc, span: Span, @@ -508,6 +510,8 @@ fn make_code_region( } else { source_file.lookup_file_pos(span.hi()) }; + let start_line = source_map.doctest_offset_line(&source_file.name, start_line); + let end_line = source_map.doctest_offset_line(&source_file.name, end_line); CodeRegion { file_name, start_line: start_line as u32, diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index f63a73acbf4ba..fbef4d06709ec 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -182,7 +182,7 @@ impl std::fmt::Display for FileName { use FileName::*; match *self { Real(RealFileName::Named(ref path)) => write!(fmt, "{}", path.display()), - // FIXME: might be nice to display both compoments of Devirtualized. + // FIXME: might be nice to display both components of Devirtualized. // But for now (to backport fix for issue #70924), best to not // perturb diagnostics so its obvious test suite still works. Real(RealFileName::Devirtualized { ref local_path, virtual_name: _ }) => { diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 37fe13c32ce74..a08ded926402f 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -247,9 +247,10 @@ fn run_test( edition: Edition, outdir: DirState, path: PathBuf, + test_id: &str, ) -> Result<(), TestFailure> { let (test, line_offset, supports_color) = - make_test(test, Some(cratename), as_test_harness, opts, edition); + make_test(test, Some(cratename), as_test_harness, opts, edition, Some(test_id)); let output_file = outdir.path().join("rust_out"); @@ -387,6 +388,7 @@ crate fn make_test( dont_insert_main: bool, opts: &TestOptions, edition: Edition, + test_id: Option<&str>, ) -> (String, usize, bool) { let (crate_attrs, everything_else, crates) = partition_source(s); let everything_else = everything_else.trim(); @@ -542,16 +544,40 @@ crate fn make_test( prog.push_str(everything_else); } else { let returns_result = everything_else.trim_end().ends_with("(())"); + // Give each doctest main function a unique name. + // This is for example needed for the tooling around `-Z instrument-coverage`. + let inner_fn_name = if let Some(test_id) = test_id { + format!("_doctest_main_{}", test_id) + } else { + "_inner".into() + }; let (main_pre, main_post) = if returns_result { ( - "fn main() { fn _inner() -> Result<(), impl core::fmt::Debug> {", - "}\n_inner().unwrap() }", + format!( + "fn main() {{ fn {}() -> Result<(), impl core::fmt::Debug> {{\n", + inner_fn_name + ), + format!("\n}}; {}().unwrap() }}", inner_fn_name), + ) + } else if test_id.is_some() { + ( + format!("fn main() {{ fn {}() {{\n", inner_fn_name), + format!("\n}}; {}() }}", inner_fn_name), ) } else { - ("fn main() {\n", "\n}") + ("fn main() {\n".into(), "\n}".into()) }; - prog.extend([main_pre, everything_else, main_post].iter().cloned()); + // Note on newlines: We insert a line/newline *before*, and *after* + // the doctest and adjust the `line_offset` accordingly. + // In the case of `-Z instrument-coverage`, this means that the generated + // inner `main` function spans from the doctest opening codeblock to the + // closing one. For example + // /// ``` <- start of the inner main + // /// <- code under doctest + // /// ``` <- end of the inner main line_offset += 1; + + prog.extend([&main_pre, everything_else, &main_post].iter().cloned()); } debug!("final doctest:\n{}", prog); @@ -749,28 +775,24 @@ impl Tester for Collector { _ => PathBuf::from(r"doctest.rs"), }; + // For example `module/file.rs` would become `module_file_rs` + let file = filename + .to_string() + .chars() + .map(|c| if c.is_ascii_alphanumeric() { c } else { '_' }) + .collect::(); + let test_id = format!( + "{file}_{line}_{number}", + file = file, + line = line, + number = { + // Increases the current test number, if this file already + // exists or it creates a new entry with a test number of 0. + self.visited_tests.entry((file.clone(), line)).and_modify(|v| *v += 1).or_insert(0) + }, + ); let outdir = if let Some(mut path) = options.persist_doctests.clone() { - // For example `module/file.rs` would become `module_file_rs` - let folder_name = filename - .to_string() - .chars() - .map(|c| if c == '\\' || c == '/' || c == '.' { '_' } else { c }) - .collect::(); - - path.push(format!( - "{krate}_{file}_{line}_{number}", - krate = cratename, - file = folder_name, - line = line, - number = { - // Increases the current test number, if this file already - // exists or it creates a new entry with a test number of 0. - self.visited_tests - .entry((folder_name.clone(), line)) - .and_modify(|v| *v += 1) - .or_insert(0) - }, - )); + path.push(&test_id); std::fs::create_dir_all(&path) .expect("Couldn't create directory for doctest executables"); @@ -817,6 +839,7 @@ impl Tester for Collector { edition, outdir, path, + &test_id, ); if let Err(err) = res { diff --git a/src/librustdoc/doctest/tests.rs b/src/librustdoc/doctest/tests.rs index a024e9c72a43e..7c0df673c1b9e 100644 --- a/src/librustdoc/doctest/tests.rs +++ b/src/librustdoc/doctest/tests.rs @@ -11,7 +11,7 @@ fn main() { assert_eq!(2+2, 4); }" .to_string(); - let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 2)); } @@ -26,7 +26,7 @@ fn main() { assert_eq!(2+2, 4); }" .to_string(); - let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 2)); } @@ -44,7 +44,7 @@ use asdf::qwop; assert_eq!(2+2, 4); }" .to_string(); - let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 3)); } @@ -61,7 +61,7 @@ use asdf::qwop; assert_eq!(2+2, 4); }" .to_string(); - let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 2)); } @@ -79,7 +79,7 @@ use std::*; assert_eq!(2+2, 4); }" .to_string(); - let (output, len, _) = make_test(input, Some("std"), false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, Some("std"), false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 2)); } @@ -98,7 +98,7 @@ use asdf::qwop; assert_eq!(2+2, 4); }" .to_string(); - let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 2)); } @@ -115,7 +115,7 @@ use asdf::qwop; assert_eq!(2+2, 4); }" .to_string(); - let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 2)); } @@ -134,7 +134,7 @@ use asdf::qwop; assert_eq!(2+2, 4); }" .to_string(); - let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 3)); // Adding more will also bump the returned line offset. @@ -147,7 +147,7 @@ use asdf::qwop; assert_eq!(2+2, 4); }" .to_string(); - let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 4)); } @@ -164,7 +164,7 @@ fn main() { assert_eq!(2+2, 4); }" .to_string(); - let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 2)); } @@ -180,7 +180,7 @@ fn main() { assert_eq!(2+2, 4); }" .to_string(); - let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 1)); } @@ -196,7 +196,7 @@ fn main() { assert_eq!(2+2, 4); }" .to_string(); - let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 2)); } @@ -210,7 +210,7 @@ assert_eq!(2+2, 4);"; //Ceci n'est pas une `fn main` assert_eq!(2+2, 4);" .to_string(); - let (output, len, _) = make_test(input, None, true, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, None, true, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 1)); } @@ -224,7 +224,7 @@ fn make_test_display_warnings() { assert_eq!(2+2, 4); }" .to_string(); - let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 1)); } @@ -242,7 +242,7 @@ assert_eq!(2+2, 4); }" .to_string(); - let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 2)); let input = "extern crate hella_qwop; @@ -256,7 +256,7 @@ assert_eq!(asdf::foo, 4); }" .to_string(); - let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 3)); } @@ -274,6 +274,41 @@ test_wrapper! { }" .to_string(); - let (output, len, _) = make_test(input, Some("my_crate"), false, &opts, DEFAULT_EDITION); + let (output, len, _) = make_test(input, Some("my_crate"), false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 1)); } + +#[test] +fn make_test_returns_result() { + // creates an inner function and unwraps it + let opts = TestOptions::default(); + let input = "use std::io; +let mut input = String::new(); +io::stdin().read_line(&mut input)?; +Ok::<(), io:Error>(())"; + let expected = "#![allow(unused)] +fn main() { fn _inner() -> Result<(), impl core::fmt::Debug> { +use std::io; +let mut input = String::new(); +io::stdin().read_line(&mut input)?; +Ok::<(), io:Error>(()) +}; _inner().unwrap() }" + .to_string(); + let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None); + assert_eq!((output, len), (expected, 2)); +} + +#[test] +fn make_test_named_wrapper() { + // creates an inner function with a specific name + let opts = TestOptions::default(); + let input = "assert_eq!(2+2, 4);"; + let expected = "#![allow(unused)] +fn main() { fn _doctest_main_some_unique_name() { +assert_eq!(2+2, 4); +}; _doctest_main_some_unique_name() }" + .to_string(); + let (output, len, _) = + make_test(input, None, false, &opts, DEFAULT_EDITION, Some("some_unique_name")); + assert_eq!((output, len), (expected, 2)); +} diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 22096203d4ce6..f911a2ce3fc78 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -248,7 +248,7 @@ impl<'a, I: Iterator>> Iterator for CodeBlocks<'_, 'a, I> { .join("\n"); let krate = krate.as_ref().map(|s| &**s); let (test, _, _) = - doctest::make_test(&test, krate, false, &Default::default(), edition); + doctest::make_test(&test, krate, false, &Default::default(), edition, None); let channel = if test.contains("#![feature(") { "&version=nightly" } else { "" }; let edition_string = format!("&edition={}", edition); diff --git a/src/test/run-make-fulldeps/coverage-reports/Makefile b/src/test/run-make-fulldeps/coverage-reports/Makefile index 5c24f909130c0..c4700b317efa0 100644 --- a/src/test/run-make-fulldeps/coverage-reports/Makefile +++ b/src/test/run-make-fulldeps/coverage-reports/Makefile @@ -98,7 +98,7 @@ endif # Run it in order to generate some profiling data, # with `LLVM_PROFILE_FILE=` environment variable set to # output the coverage stats for this run. - LLVM_PROFILE_FILE="$(TMPDIR)"/$@.profraw \ + LLVM_PROFILE_FILE="$(TMPDIR)"/$@-%p.profraw \ $(call RUN,$@) || \ ( \ status=$$?; \ @@ -108,9 +108,16 @@ endif ) \ ) + # Run it through rustdoc as well to cover doctests + LLVM_PROFILE_FILE="$(TMPDIR)"/$@-%p.profraw \ + $(RUSTDOC) --crate-name workaround_for_79771 --test $(SOURCEDIR)/$@.rs \ + $$( grep -q '^\/\/ require-rust-edition-2018' $(SOURCEDIR)/$@.rs && echo "--edition=2018" ) \ + -L "$(TMPDIR)" -Zinstrument-coverage \ + -Z unstable-options --persist-doctests=$(TMPDIR)/rustdoc-$@ + # Postprocess the profiling data so it can be used by the llvm-cov tool "$(LLVM_BIN_DIR)"/llvm-profdata merge --sparse \ - "$(TMPDIR)"/$@.profraw \ + "$(TMPDIR)"/$@-*.profraw \ -o "$(TMPDIR)"/$@.profdata # Generate a coverage report using `llvm-cov show`. @@ -121,8 +128,15 @@ endif --show-line-counts-or-regions \ --instr-profile="$(TMPDIR)"/$@.profdata \ $(call BIN,"$(TMPDIR)"/$@) \ - > "$(TMPDIR)"/actual_show_coverage.$@.txt \ - 2> "$(TMPDIR)"/show_coverage_stderr.$@.txt || \ + $$( \ + for file in $(TMPDIR)/rustdoc-$@/*/rust_out; \ + do \ + [[ -x $$file ]] && printf "%s %s " -object $$file; \ + done \ + ) \ + 2> "$(TMPDIR)"/show_coverage_stderr.$@.txt \ + | "$(PYTHON)" $(BASEDIR)/normalize_paths.py \ + > "$(TMPDIR)"/actual_show_coverage.$@.txt || \ ( status=$$? ; \ >&2 cat "$(TMPDIR)"/show_coverage_stderr.$@.txt ; \ exit $$status \ diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt new file mode 100644 index 0000000000000..e1731c7223c5d --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt @@ -0,0 +1,79 @@ +../coverage/doctest.rs: + 1| |//! This test ensures that code from doctests is properly re-mapped. + 2| |//! See for more info. + 3| |//! + 4| |//! Just some random code: + 5| 1|//! ``` + 6| 1|//! if true { + 7| |//! // this is executed! + 8| 1|//! assert_eq!(1, 1); + 9| |//! } else { + 10| |//! // this is not! + 11| |//! assert_eq!(1, 2); + 12| |//! } + 13| 1|//! ``` + 14| |//! + 15| |//! doctest testing external code: + 16| |//! ``` + 17| 1|//! extern crate doctest_crate; + 18| 1|//! doctest_crate::fn_run_in_doctests(1); + 19| 1|//! ``` + 20| |//! + 21| |//! doctest returning a result: + 22| 1|//! ``` + 23| 1|//! #[derive(Debug)] + 24| 1|//! struct SomeError; + 25| 1|//! let mut res = Err(SomeError); + 26| 1|//! if res.is_ok() { + 27| 0|//! res?; + 28| 1|//! } else { + 29| 1|//! res = Ok(0); + 30| 1|//! } + 31| |//! // need to be explicit because rustdoc cant infer the return type + 32| 1|//! Ok::<(), SomeError>(()) + 33| 1|//! ``` + 34| |//! + 35| |//! doctest with custom main: + 36| |//! ``` + 37| |//! #[derive(Debug)] + 38| |//! struct SomeError; + 39| |//! + 40| |//! extern crate doctest_crate; + 41| |//! + 42| 1|//! fn doctest_main() -> Result<(), SomeError> { + 43| 1|//! doctest_crate::fn_run_in_doctests(2); + 44| 1|//! Ok(()) + 45| 1|//! } + 46| |//! + 47| |//! // this `main` is not shown as covered, as it clashes with all the other + 48| |//! // `main` functions that were automatically generated for doctests + 49| |//! fn main() -> Result<(), SomeError> { + 50| |//! doctest_main() + 51| |//! } + 52| |//! ``` + 53| | + 54| |/// doctest attached to fn testing external code: + 55| |/// ``` + 56| 1|/// extern crate doctest_crate; + 57| 1|/// doctest_crate::fn_run_in_doctests(3); + 58| 1|/// ``` + 59| |/// + 60| 1|fn main() { + 61| 1| if true { + 62| 1| assert_eq!(1, 1); + 63| | } else { + 64| | assert_eq!(1, 2); + 65| | } + 66| 1|} + +../coverage/lib/doctest_crate.rs: + 1| |/// A function run only from within doctests + 2| 3|pub fn fn_run_in_doctests(conditional: usize) { + 3| 3| match conditional { + 4| 1| 1 => assert_eq!(1, 1), // this is run, + 5| 1| 2 => assert_eq!(1, 1), // this, + 6| 1| 3 => assert_eq!(1, 1), // and this too + 7| 0| _ => assert_eq!(1, 2), // however this is not + 8| | } + 9| 3|} + diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt index e14e733fff6d4..4c03e950af029 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt @@ -19,12 +19,12 @@ 18| 2| println!("used_only_from_bin_crate_generic_function with {:?}", arg); 19| 2|} ------------------ - | used_crate::used_only_from_bin_crate_generic_function::<&str>: + | used_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec>: | 17| 1|pub fn used_only_from_bin_crate_generic_function(arg: T) { | 18| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg); | 19| 1|} ------------------ - | used_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec>: + | used_crate::used_only_from_bin_crate_generic_function::<&str>: | 17| 1|pub fn used_only_from_bin_crate_generic_function(arg: T) { | 18| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg); | 19| 1|} diff --git a/src/test/run-make-fulldeps/coverage-reports/normalize_paths.py b/src/test/run-make-fulldeps/coverage-reports/normalize_paths.py new file mode 100644 index 0000000000000..05fb412cdb635 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-reports/normalize_paths.py @@ -0,0 +1,10 @@ +#!/usr/bin/env python + +import sys + +# Normalize file paths in output +for line in sys.stdin: + if line.startswith("..") and line.rstrip().endswith(".rs:"): + print(line.replace("\\", "/"), end='') + else: + print(line, end='') diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.doctest/doctest.main.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.doctest/doctest.main.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..8d074558aae20 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.doctest/doctest.main.-------.InstrumentCoverage.0.html @@ -0,0 +1,127 @@ + + + + +doctest.main - Coverage Spans + + + +
@0⦊fn main() ⦉@0{ + if @0⦊true⦉@0 { + @5⦊@4,6,7,8,9⦊assert_eq!(1, 1);⦉@4,6,7,8,9⦉@5 + } else { + @11⦊@10,12,13,14,15⦊assert_eq!(1, 2);⦉@10,12,13,14,15⦉@11 + } +}@16⦊‸⦉@16
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.doctest_crate/doctest_crate.fn_run_in_doctests.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.doctest_crate/doctest_crate.fn_run_in_doctests.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..ae119d9ca9f2e --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.doctest_crate/doctest_crate.fn_run_in_doctests.-------.InstrumentCoverage.0.html @@ -0,0 +1,173 @@ + + + + +doctest_crate.fn_run_in_doctests - Coverage Spans + + + +
@0⦊pub fn fn_run_in_doctests(conditional: usize) ⦉@0{ + match @0⦊conditional⦉@0 { + 1 => @7⦊@6,8,9,10,11⦊assert_eq!(1, 1)⦉@6,8,9,10,11⦉@7, // this is run, + 2 => @14⦊@13,15,16,17,18⦊assert_eq!(1, 1)⦉@13,15,16,17,18⦉@14, // this, + 3 => @21⦊@20,22,23,24,25⦊assert_eq!(1, 1)⦉@20,22,23,24,25⦉@21, // and this too + _ => @27⦊@26,28,29,30,31⦊assert_eq!(1, 2)⦉@26,28,29,30,31⦉@27, // however this is not + } +}@32⦊‸⦉@32
+ + diff --git a/src/test/run-make-fulldeps/coverage/compiletest-ignore-dir b/src/test/run-make-fulldeps/coverage/compiletest-ignore-dir index abf8df8fdc9e6..d1824d189e382 100644 --- a/src/test/run-make-fulldeps/coverage/compiletest-ignore-dir +++ b/src/test/run-make-fulldeps/coverage/compiletest-ignore-dir @@ -1,3 +1,3 @@ -# Directory "instrument-coverage" supports the tests at prefix ../instrument-coverage-* +# Directory "coverage" supports the tests at prefix ../coverage-* -# Use ./x.py [options] test src/test/run-make-fulldeps/instrument-coverage to run all related tests. +# Use ./x.py [options] test src/test/run-make-fulldeps/coverage to run all related tests. diff --git a/src/test/run-make-fulldeps/coverage/coverage_tools.mk b/src/test/run-make-fulldeps/coverage/coverage_tools.mk index 7dc485cd94d66..4d340d4b1dadd 100644 --- a/src/test/run-make-fulldeps/coverage/coverage_tools.mk +++ b/src/test/run-make-fulldeps/coverage/coverage_tools.mk @@ -1,7 +1,7 @@ -# Common Makefile include for Rust `run-make-fulldeps/instrument-coverage-* tests. Include this +# Common Makefile include for Rust `run-make-fulldeps/coverage-* tests. Include this # file with the line: # -# -include ../instrument-coverage/coverage_tools.mk +# -include ../coverage/coverage_tools.mk -include ../tools.mk diff --git a/src/test/run-make-fulldeps/coverage/doctest.rs b/src/test/run-make-fulldeps/coverage/doctest.rs new file mode 100644 index 0000000000000..e41d669bf0c76 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage/doctest.rs @@ -0,0 +1,66 @@ +//! This test ensures that code from doctests is properly re-mapped. +//! See for more info. +//! +//! Just some random code: +//! ``` +//! if true { +//! // this is executed! +//! assert_eq!(1, 1); +//! } else { +//! // this is not! +//! assert_eq!(1, 2); +//! } +//! ``` +//! +//! doctest testing external code: +//! ``` +//! extern crate doctest_crate; +//! doctest_crate::fn_run_in_doctests(1); +//! ``` +//! +//! doctest returning a result: +//! ``` +//! #[derive(Debug)] +//! struct SomeError; +//! let mut res = Err(SomeError); +//! if res.is_ok() { +//! res?; +//! } else { +//! res = Ok(0); +//! } +//! // need to be explicit because rustdoc cant infer the return type +//! Ok::<(), SomeError>(()) +//! ``` +//! +//! doctest with custom main: +//! ``` +//! #[derive(Debug)] +//! struct SomeError; +//! +//! extern crate doctest_crate; +//! +//! fn doctest_main() -> Result<(), SomeError> { +//! doctest_crate::fn_run_in_doctests(2); +//! Ok(()) +//! } +//! +//! // this `main` is not shown as covered, as it clashes with all the other +//! // `main` functions that were automatically generated for doctests +//! fn main() -> Result<(), SomeError> { +//! doctest_main() +//! } +//! ``` + +/// doctest attached to fn testing external code: +/// ``` +/// extern crate doctest_crate; +/// doctest_crate::fn_run_in_doctests(3); +/// ``` +/// +fn main() { + if true { + assert_eq!(1, 1); + } else { + assert_eq!(1, 2); + } +} diff --git a/src/test/run-make-fulldeps/coverage/lib/doctest_crate.rs b/src/test/run-make-fulldeps/coverage/lib/doctest_crate.rs new file mode 100644 index 0000000000000..c3210146d69b0 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage/lib/doctest_crate.rs @@ -0,0 +1,9 @@ +/// A function run only from within doctests +pub fn fn_run_in_doctests(conditional: usize) { + match conditional { + 1 => assert_eq!(1, 1), // this is run, + 2 => assert_eq!(1, 1), // this, + 3 => assert_eq!(1, 1), // and this too + _ => assert_eq!(1, 2), // however this is not + } +} From 5b6c175566e55e29c0d2c39cb0dab2119ac40c82 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 19 Dec 2020 17:48:31 +0000 Subject: [PATCH 20/28] Tweak diagnostics --- .../src/thir/pattern/deconstruct_pat.rs | 5 ++--- .../overlapping_range_endpoints.stderr | 16 ++++++++-------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs index 5b1dbabe9a174..94083289cabd7 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs @@ -309,9 +309,7 @@ impl IntRange { pcx.span, |lint| { let mut err = lint.build("multiple patterns overlap on their endpoints"); - err.span_label(pcx.span, "... with this range"); for (int_range, span) in overlaps { - // Use the real type for user display of the ranges: err.span_label( span, &format!( @@ -320,7 +318,8 @@ impl IntRange { ), ); } - err.note("this is likely to be a mistake"); + err.span_label(pcx.span, "... with this range"); + err.note("you likely meant to write mutually exclusive ranges"); err.emit(); }, ); diff --git a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr index 045c487873a35..24c0419e1dde3 100644 --- a/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr +++ b/src/test/ui/pattern/usefulness/integer-ranges/overlapping_range_endpoints.stderr @@ -11,7 +11,7 @@ note: the lint level is defined here | LL | #![deny(overlapping_range_endpoints)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this is likely to be a mistake + = note: you likely meant to write mutually exclusive ranges error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:16:22 @@ -21,7 +21,7 @@ LL | m!(0u8, 30..=40, 20..=30); | | | this range overlaps on `30_u8`... | - = note: this is likely to be a mistake + = note: you likely meant to write mutually exclusive ranges error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:19:22 @@ -31,7 +31,7 @@ LL | m!(0u8, 20.. 30, 29..=40); | | | this range overlaps on `29_u8`... | - = note: this is likely to be a mistake + = note: you likely meant to write mutually exclusive ranges error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:23:22 @@ -41,7 +41,7 @@ LL | m!(0u8, 20..=30, 30..=31); | | | this range overlaps on `30_u8`... | - = note: this is likely to be a mistake + = note: you likely meant to write mutually exclusive ranges error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:27:22 @@ -51,7 +51,7 @@ LL | m!(0u8, 20..=30, 19..=20); | | | this range overlaps on `20_u8`... | - = note: this is likely to be a mistake + = note: you likely meant to write mutually exclusive ranges error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:39:9 @@ -63,7 +63,7 @@ LL | 20..=30 => {} LL | 10..=20 => {} | ^^^^^^^ ... with this range | - = note: this is likely to be a mistake + = note: you likely meant to write mutually exclusive ranges error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:50:16 @@ -73,7 +73,7 @@ LL | (true, 0..=10) => {} LL | (true, 10..20) => {} | ^^^^^^ ... with this range | - = note: this is likely to be a mistake + = note: you likely meant to write mutually exclusive ranges error: multiple patterns overlap on their endpoints --> $DIR/overlapping_range_endpoints.rs:56:14 @@ -83,7 +83,7 @@ LL | Some(0..=10) => {} LL | Some(10..20) => {} | ^^^^^^ ... with this range | - = note: this is likely to be a mistake + = note: you likely meant to write mutually exclusive ranges error: aborting due to 8 previous errors From 52b717f8264baf1044a001035d20c77f545b6abf Mon Sep 17 00:00:00 2001 From: pierwill Date: Sat, 19 Dec 2020 14:08:41 -0800 Subject: [PATCH 21/28] Edit rustc_middle::lint::LintSource docs Edit punctuation in doc comment for rustc_middle::lint::LintSource::CommandLine. --- compiler/rustc_middle/src/lint.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index a61d37cc90eba..0724d50340785 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/compiler/rustc_middle/src/lint.rs @@ -22,8 +22,8 @@ pub enum LintSource { Node(Symbol, Span, Option /* RFC 2383 reason */), /// Lint level was set by a command-line flag. - /// The provided `Level` is the level specified on the command line - - /// the actual level may be lower due to `--cap-lints` + /// The provided `Level` is the level specified on the command line. + /// (The actual level may be lower due to `--cap-lints`.) CommandLine(Symbol, Level), } From 4fffa742d70c81f7414d02f55808d22eeeb77bb2 Mon Sep 17 00:00:00 2001 From: pierwill Date: Sat, 19 Dec 2020 14:25:24 -0800 Subject: [PATCH 22/28] docs: Edit rustc_middle::ty::query::on_disk_cache Expand abbreviations for "incremental compliation". Also added the word "to" to the description of CacheEncoder. --- compiler/rustc_middle/src/ty/query/on_disk_cache.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/ty/query/on_disk_cache.rs b/compiler/rustc_middle/src/ty/query/on_disk_cache.rs index e006dfeb66336..98fc32f34c888 100644 --- a/compiler/rustc_middle/src/ty/query/on_disk_cache.rs +++ b/compiler/rustc_middle/src/ty/query/on_disk_cache.rs @@ -648,7 +648,7 @@ impl<'sess> OnDiskCache<'sess> { //- DECODING ------------------------------------------------------------------- -/// A decoder that can read from the incr. comp. cache. It is similar to the one +/// A decoder that can read from the incremental compilation cache. It is similar to the one /// we use for crate metadata decoding in that it can rebase spans and eventually /// will also handle things that contain `Ty` instances. crate struct CacheDecoder<'a, 'tcx> { @@ -936,7 +936,7 @@ impl<'a, 'tcx> Decodable> for &'tcx [Span] { //- ENCODING ------------------------------------------------------------------- -/// An encoder that can write the incr. comp. cache. +/// An encoder that can write to the incremental compilation cache. struct CacheEncoder<'a, 'tcx, E: OpaqueEncoder> { tcx: TyCtxt<'tcx>, encoder: &'a mut E, From 41c43300820615aa377b9e6f5711ff0ba1c74fab Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sun, 20 Dec 2020 00:34:49 +0100 Subject: [PATCH 23/28] Make doc-test pass with the new rustdoc --- compiler/rustc_trait_selection/src/opaque_types.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/rustc_trait_selection/src/opaque_types.rs b/compiler/rustc_trait_selection/src/opaque_types.rs index f5bc90e6f9621..9cce863ac73ea 100644 --- a/compiler/rustc_trait_selection/src/opaque_types.rs +++ b/compiler/rustc_trait_selection/src/opaque_types.rs @@ -46,6 +46,7 @@ pub struct OpaqueTypeDecl<'tcx> { /// type Foo = impl Baz; /// fn bar() -> Foo { /// // ^^^ This is the span we are looking for! + /// # } /// ``` /// /// In cases where the fn returns `(impl Trait, impl Trait)` or From c127530be76bd8aebc7b61f3b4a54f1be577f74c Mon Sep 17 00:00:00 2001 From: Camelid Date: Sat, 19 Dec 2020 20:47:57 -0800 Subject: [PATCH 24/28] Fix labels for 'Library Tracking Issue' template Each label needs to be separated by a comma (see the ICE issue template for an example of correct usage). --- .github/ISSUE_TEMPLATE/library_tracking_issue.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/library_tracking_issue.md b/.github/ISSUE_TEMPLATE/library_tracking_issue.md index b8544e6a4e0fd..3e42594c8280d 100644 --- a/.github/ISSUE_TEMPLATE/library_tracking_issue.md +++ b/.github/ISSUE_TEMPLATE/library_tracking_issue.md @@ -2,7 +2,7 @@ name: Library Tracking Issue about: A tracking issue for an unstable library feature. title: Tracking Issue for XXX -labels: C-tracking-issue T-libs +labels: C-tracking-issue, T-libs --- $DIR/incorrect-move-async-order-issue-79694.rs:7:13 + | +LL | let _ = move async { }; + | ^^^^^^^^^^ + | +help: try switching the order + | +LL | let _ = async move { }; + | ^^^^^^^^^^ + +error: aborting due to previous error +