about summary refs log tree commit diff
path: root/library/std
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-05-12 12:04:32 +0000
committerbors <bors@rust-lang.org>2023-05-12 12:04:32 +0000
commit077fc26f0acfa54e9c580534616c17ffc279a9d4 (patch)
tree5ac9db2c8e201070d88c537c9f9b37c37caff01a /library/std
parent0b795044c6f0854445f1f2bb6443e87848e150d1 (diff)
parentf5aede9c822875345eb7eb468ecd9bc4568ed112 (diff)
downloadrust-077fc26f0acfa54e9c580534616c17ffc279a9d4.tar.gz
rust-077fc26f0acfa54e9c580534616c17ffc279a9d4.zip
Auto merge of #109732 - Urgau:uplift_drop_forget_ref_lints, r=davidtwco
Uplift `clippy::{drop,forget}_{ref,copy}` lints

This PR aims at uplifting the `clippy::drop_ref`, `clippy::drop_copy`, `clippy::forget_ref` and `clippy::forget_copy` lints.

Those lints are/were declared in the correctness category of clippy because they lint on useless and most probably is not what the developer wanted.

## `drop_ref` and `forget_ref`

The `drop_ref` and `forget_ref` lint checks for calls to `std::mem::drop` or `std::mem::forget` with a reference instead of an owned value.

### Example

```rust
let mut lock_guard = mutex.lock();
std::mem::drop(&lock_guard) // Should have been drop(lock_guard), mutex
// still locked
operation_that_requires_mutex_to_be_unlocked();
```

### Explanation

Calling `drop` or `forget` on a reference will only drop the reference itself, which is a no-op. It will not call the `drop` or `forget` method on the underlying referenced value, which is likely what was intended.

## `drop_copy` and `forget_copy`

The `drop_copy` and `forget_copy` lint checks for calls to `std::mem::forget` or `std::mem::drop` with a value that derives the Copy trait.

### Example

```rust
let x: i32 = 42; // i32 implements Copy
std::mem::forget(x) // A copy of x is passed to the function, leaving the
                    // original unaffected
```

### Explanation

Calling `std::mem::forget` [does nothing for types that implement Copy](https://doc.rust-lang.org/std/mem/fn.drop.html) since the value will be copied and moved into the function on invocation.

-----

Followed the instructions for uplift a clippy describe here: https://github.com/rust-lang/rust/pull/99696#pullrequestreview-1134072751

cc `@m-ou-se` (as T-libs-api leader because the uplifting was discussed in a recent meeting)
Diffstat (limited to 'library/std')
-rw-r--r--library/std/src/panicking.rs2
-rw-r--r--library/std/src/sys/sgx/waitqueue/mod.rs16
-rw-r--r--library/std/src/sys/unix/fs.rs2
-rw-r--r--library/std/src/thread/tests.rs4
4 files changed, 18 insertions, 6 deletions
diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs
index a46a29cbad6..6d59266b6f8 100644
--- a/library/std/src/panicking.rs
+++ b/library/std/src/panicking.rs
@@ -541,7 +541,7 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {
             // Lazily, the first time this gets called, run the actual string formatting.
             self.string.get_or_insert_with(|| {
                 let mut s = String::new();
-                drop(s.write_fmt(*inner));
+                let _err = s.write_fmt(*inner);
                 s
             })
         }
diff --git a/library/std/src/sys/sgx/waitqueue/mod.rs b/library/std/src/sys/sgx/waitqueue/mod.rs
index 61bb11d9a6f..5e1d859ee99 100644
--- a/library/std/src/sys/sgx/waitqueue/mod.rs
+++ b/library/std/src/sys/sgx/waitqueue/mod.rs
@@ -202,12 +202,18 @@ impl WaitQueue {
     pub fn notify_one<T>(
         mut guard: SpinMutexGuard<'_, WaitVariable<T>>,
     ) -> Result<WaitGuard<'_, T>, SpinMutexGuard<'_, WaitVariable<T>>> {
+        // SAFETY: lifetime of the pop() return value is limited to the map
+        // closure (The closure return value is 'static). The underlying
+        // stack frame won't be freed until after the WaitGuard created below
+        // is dropped.
         unsafe {
-            if let Some(entry) = guard.queue.inner.pop() {
+            let tcs = guard.queue.inner.pop().map(|entry| -> Tcs {
                 let mut entry_guard = entry.lock();
-                let tcs = entry_guard.tcs;
                 entry_guard.wake = true;
-                drop(entry);
+                entry_guard.tcs
+            });
+
+            if let Some(tcs) = tcs {
                 Ok(WaitGuard { mutex_guard: Some(guard), notified_tcs: NotifiedTcs::Single(tcs) })
             } else {
                 Err(guard)
@@ -223,6 +229,9 @@ impl WaitQueue {
     pub fn notify_all<T>(
         mut guard: SpinMutexGuard<'_, WaitVariable<T>>,
     ) -> Result<WaitGuard<'_, T>, SpinMutexGuard<'_, WaitVariable<T>>> {
+        // SAFETY: lifetime of the pop() return values are limited to the
+        // while loop body. The underlying stack frames won't be freed until
+        // after the WaitGuard created below is dropped.
         unsafe {
             let mut count = 0;
             while let Some(entry) = guard.queue.inner.pop() {
@@ -230,6 +239,7 @@ impl WaitQueue {
                 let mut entry_guard = entry.lock();
                 entry_guard.wake = true;
             }
+
             if let Some(count) = NonZeroUsize::new(count) {
                 Ok(WaitGuard { mutex_guard: Some(guard), notified_tcs: NotifiedTcs::All { count } })
             } else {
diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs
index 22d2ae39713..09db5b11dbf 100644
--- a/library/std/src/sys/unix/fs.rs
+++ b/library/std/src/sys/unix/fs.rs
@@ -1210,7 +1210,7 @@ impl File {
                 // Redox doesn't appear to support `UTIME_OMIT`.
                 // ESP-IDF and HorizonOS do not support `futimens` at all and the behavior for those OS is therefore
                 // the same as for Redox.
-                drop(times);
+                let _ = times;
                 Err(io::const_io_error!(
                     io::ErrorKind::Unsupported,
                     "setting file times not supported",
diff --git a/library/std/src/thread/tests.rs b/library/std/src/thread/tests.rs
index 6c9ce6fa0dd..b65e2572cc5 100644
--- a/library/std/src/thread/tests.rs
+++ b/library/std/src/thread/tests.rs
@@ -375,7 +375,9 @@ fn test_scoped_threads_nll() {
     // this is mostly a *compilation test* for this exact function:
     fn foo(x: &u8) {
         thread::scope(|s| {
-            s.spawn(|| drop(x));
+            s.spawn(|| match x {
+                _ => (),
+            });
         });
     }
     // let's also run it for good measure