Extend `unused_must_use` to cover block exprs #112529

jieyouxu · 2023-06-11T15:52:37Z

Given code like

#[must_use]
fn foo() -> i32 {
    42
}

fn warns() {
    {
        foo();
    }
}

fn does_not_warn() {
    {
        foo()
    };
}

fn main() {
    warns();
    does_not_warn();
}

Before This PR

warning: unused return value of `foo` that must be used
 --> test.rs:8:9
  |
8 |         foo();
  |         ^^^^^
  |
  = note: `#[warn(unused_must_use)]` on by default
help: use `let _ = ...` to ignore the resulting value
  |
8 |         let _ = foo();
  |         +++++++

warning: 1 warning emitted

After This PR

warning: unused return value of `foo` that must be used
 --> test.rs:8:9
  |
8 |         foo();
  |         ^^^^^
  |
  = note: `#[warn(unused_must_use)]` on by default
help: use `let _ = ...` to ignore the resulting value
  |
8 |         let _ = foo();
  |         +++++++

warning: unused return value of `foo` that must be used
  --> test.rs:14:9
   |
14 |         foo()
   |         ^^^^^
   |
help: use `let _ = ...` to ignore the resulting value
   |
14 |         let _ = foo();
   |         +++++++      +

warning: 2 warnings emitted

Fixes #104253.

rustbot · 2023-06-11T15:52:45Z

r? @oli-obk

(rustbot has picked a reviewer for you, use r? to override)

jieyouxu · 2023-06-11T16:28:07Z

error: unused return value of `Box::<T>::from_raw` that must be used
  --> compiler/rustc_arena/src/lib.rs:70:18
   |
70 |         unsafe { Box::from_raw(self.storage.as_mut()) };
   |
   |
   = note: call `drop(Box::from_raw(ptr))` if you intend to drop the `Box`
   = note: `-D unused-must-use` implied by `-D warnings`
   |
   |
70 |         unsafe { let _ = Box::from_raw(self.storage.as_mut()); };

I think that's a true positive (can just surround with drop right?) but pedantic...?

compiler/rustc_arena/src/lib.rs

compiler/rustc_lint/src/unused.rs

oli-obk · 2023-06-15T08:14:06Z

@bors r+

bors · 2023-06-15T08:14:08Z

📌 Commit fc6b5315b2f2e4eedb923dbb25bf6a518e53e02e has been approved by oli-obk

It is now in the queue for this repository.

oli-obk · 2023-06-15T08:18:23Z

@bors r-

oli-obk · 2023-06-15T08:18:46Z

you need to bless the clippy tests: x test clippy --bless

rustbot · 2023-06-15T08:38:41Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

klensy · 2023-06-15T08:39:00Z

Can you fix first message? In your example foo() unused.

rustbot · 2023-06-15T09:59:22Z

The Miri subtree was changed

cc @rust-lang/miri

bors · 2023-06-15T10:02:12Z

📌 Commit 72b3b58 has been approved by oli-obk

It is now in the queue for this repository.

RalfJung · 2023-06-15T10:44:03Z

src/tools/miri/tests/fail/dangling_pointers/storage_dead_dangling.stderr

@@ -1,8 +1,8 @@
 error: Undefined Behavior: dereferencing pointer failed: $HEX[noalloc] is a dangling pointer (it has no provenance)
  --> $DIR/storage_dead_dangling.rs:LL:CC
   |
-LL |     unsafe { &mut *(LEAK as *mut i32) };
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^ dereferencing pointer failed: $HEX[noalloc] is a dangling pointer (it has no provenance)
+LL |     let _ = unsafe { &mut *(LEAK as *mut i32) };


FWIW, adding let _ here is not always obviously the right choice: there are cases where *y; is UB but let _ = *y; is fine. In terms of UB, using *y; as a statement is equivalent to let _unused = *y;, not to let _ = *y;. (In terms of drop order however it seems to behave like let _...)

However, only "fail" tests were changed in Miri, so if UB was accidentally removed we would have noticed.

…llaumeGomez Rollup of 8 pull requests Successful merges: - rust-lang#112403 (Prevent `.eh_frame` from being emitted for `-C panic=abort`) - rust-lang#112517 (`suspicious_double_ref_op`: don't lint on `.borrow()`) - rust-lang#112529 (Extend `unused_must_use` to cover block exprs) - rust-lang#112614 (tweak suggestion for argument-position `impl ?Sized`) - rust-lang#112654 (normalize closure output in equate_inputs_and_outputs) - rust-lang#112660 (Migrate GUI colors test to original CSS color format) - rust-lang#112664 (Add support for test tmpdir to fuchsia test runner) - rust-lang#112669 (Fix comment for ptr alignment checks in codegen) r? `@ghost` `@rustbot` modify labels: rollup

`Box::from_raw()` is `#[must_use]`, which means the result cannot go unused. In Rust 1.71.0, this was not detected because the block expression swallows the diagnostic [1]: unsafe { Box::from_raw(self.ptr.as_ptr()) }; It would have been detected, however, if the line had been instead: unsafe { Box::from_raw(self.ptr.as_ptr()); } i.e. the semicolon being inside the `unsafe` block, rather than outside. In Rust 1.72.0, the compiler started warning about this [2], so without this patch we will get: error: unused return value of `alloc::boxed::Box::<T>::from_raw` that must be used --> rust/kernel/sync/arc.rs:302:22 | 302 | unsafe { Box::from_raw(self.ptr.as_ptr()) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: call `drop(Box::from_raw(ptr))` if you intend to drop the `Box` = note: `-D unused-must-use` implied by `-D warnings` help: use `let _ = ...` to ignore the resulting value | 302 | unsafe { let _ = Box::from_raw(self.ptr.as_ptr()); }; | +++++++ + Thus add an add an explicit `drop()` as the `#[must_use]`'s annotation suggests (instead of the more general help line). Link: rust-lang/rust#104253 [1] Link: rust-lang/rust#112529 [2] Signed-off-by: Miguel Ojeda <[email protected]>

`Box::from_raw()` is `#[must_use]`, which means the result cannot go unused. In Rust 1.71.0, this was not detected because the block expression swallows the diagnostic [1]: unsafe { Box::from_raw(self.ptr.as_ptr()) }; It would have been detected, however, if the line had been instead: unsafe { Box::from_raw(self.ptr.as_ptr()); } i.e. the semicolon being inside the `unsafe` block, rather than outside. In Rust 1.72.0, the compiler started warning about this [2], so without this patch we will get: error: unused return value of `alloc::boxed::Box::<T>::from_raw` that must be used --> rust/kernel/sync/arc.rs:302:22 | 302 | unsafe { Box::from_raw(self.ptr.as_ptr()) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: call `drop(Box::from_raw(ptr))` if you intend to drop the `Box` = note: `-D unused-must-use` implied by `-D warnings` help: use `let _ = ...` to ignore the resulting value | 302 | unsafe { let _ = Box::from_raw(self.ptr.as_ptr()); }; | +++++++ + Thus add an add an explicit `drop()` as the `#[must_use]`'s annotation suggests (instead of the more general help line). Link: rust-lang/rust#104253 [1] Link: rust-lang/rust#112529 [2] Signed-off-by: Miguel Ojeda <[email protected]> Link: https://lore.kernel.org/r/[email protected]

`Box::from_raw()` is `#[must_use]`, which means the result cannot go unused. In Rust 1.71.0, this was not detected because the block expression swallows the diagnostic [1]: unsafe { Box::from_raw(self.ptr.as_ptr()) }; It would have been detected, however, if the line had been instead: unsafe { Box::from_raw(self.ptr.as_ptr()); } i.e. the semicolon being inside the `unsafe` block, rather than outside. In Rust 1.72.0, the compiler started warning about this [2], so without this patch we will get: error: unused return value of `alloc::boxed::Box::<T>::from_raw` that must be used --> rust/kernel/sync/arc.rs:302:22 | 302 | unsafe { Box::from_raw(self.ptr.as_ptr()) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: call `drop(Box::from_raw(ptr))` if you intend to drop the `Box` = note: `-D unused-must-use` implied by `-D warnings` help: use `let _ = ...` to ignore the resulting value | 302 | unsafe { let _ = Box::from_raw(self.ptr.as_ptr()); }; | +++++++ + Thus add an add an explicit `drop()` as the `#[must_use]`'s annotation suggests (instead of the more general help line). Link: rust-lang/rust#104253 [1] Link: rust-lang/rust#112529 [2] Reviewed-by: Martin Rodriguez Reboredo <[email protected]> Reviewed-by: Gary Guo <[email protected]> Reviewed-by: Alice Ryhl <[email protected]> Reviewed-by: Andreas Hindborg <[email protected]> Reviewed-by: Björn Roy Baron <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>

`Box::from_raw()` is `#[must_use]`, which means the result cannot go unused. In Rust 1.71.0, this was not detected because the block expression swallows the diagnostic [1]: unsafe { Box::from_raw(self.ptr.as_ptr()) }; It would have been detected, however, if the line had been instead: unsafe { Box::from_raw(self.ptr.as_ptr()); } i.e. the semicolon being inside the `unsafe` block, rather than outside. In Rust 1.72.0, the compiler started warning about this [2], so without this patch we will get: error: unused return value of `alloc::boxed::Box::<T>::from_raw` that must be used --> rust/kernel/sync/arc.rs:302:22 | 302 | unsafe { Box::from_raw(self.ptr.as_ptr()) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: call `drop(Box::from_raw(ptr))` if you intend to drop the `Box` = note: `-D unused-must-use` implied by `-D warnings` help: use `let _ = ...` to ignore the resulting value | 302 | unsafe { let _ = Box::from_raw(self.ptr.as_ptr()); }; | +++++++ + Thus add an add an explicit `drop()` as the `#[must_use]`'s annotation suggests (instead of the more general help line). Link: rust-lang/rust#104253 [1] Link: rust-lang/rust#112529 [2] Reviewed-by: Martin Rodriguez Reboredo <[email protected]> Reviewed-by: Gary Guo <[email protected]> Reviewed-by: Alice Ryhl <[email protected]> Reviewed-by: Andreas Hindborg <[email protected]> Reviewed-by: Björn Roy Baron <[email protected]> Link: https://lore.kernel.org/r/[email protected] Change-Id: Iecf642296b8fdf52f03531ff45b8c97f7f5ea372 Signed-off-by: Miguel Ojeda <[email protected]> Signed-off-by: Matthew Maurer <[email protected]> (cherry picked from commit 828176d)

commit 828176d upstream. `Box::from_raw()` is `#[must_use]`, which means the result cannot go unused. In Rust 1.71.0, this was not detected because the block expression swallows the diagnostic [1]: unsafe { Box::from_raw(self.ptr.as_ptr()) }; It would have been detected, however, if the line had been instead: unsafe { Box::from_raw(self.ptr.as_ptr()); } i.e. the semicolon being inside the `unsafe` block, rather than outside. In Rust 1.72.0, the compiler started warning about this [2], so without this patch we will get: error: unused return value of `alloc::boxed::Box::<T>::from_raw` that must be used --> rust/kernel/sync/arc.rs:302:22 | 302 | unsafe { Box::from_raw(self.ptr.as_ptr()) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: call `drop(Box::from_raw(ptr))` if you intend to drop the `Box` = note: `-D unused-must-use` implied by `-D warnings` help: use `let _ = ...` to ignore the resulting value | 302 | unsafe { let _ = Box::from_raw(self.ptr.as_ptr()); }; | +++++++ + Thus add an add an explicit `drop()` as the `#[must_use]`'s annotation suggests (instead of the more general help line). Link: rust-lang/rust#104253 [1] Link: rust-lang/rust#112529 [2] Reviewed-by: Martin Rodriguez Reboredo <[email protected]> Reviewed-by: Gary Guo <[email protected]> Reviewed-by: Alice Ryhl <[email protected]> Reviewed-by: Andreas Hindborg <[email protected]> Reviewed-by: Björn Roy Baron <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>

BugLink: https://bugs.launchpad.net/bugs/2059991 commit 828176d upstream. `Box::from_raw()` is `#[must_use]`, which means the result cannot go unused. In Rust 1.71.0, this was not detected because the block expression swallows the diagnostic [1]: unsafe { Box::from_raw(self.ptr.as_ptr()) }; It would have been detected, however, if the line had been instead: unsafe { Box::from_raw(self.ptr.as_ptr()); } i.e. the semicolon being inside the `unsafe` block, rather than outside. In Rust 1.72.0, the compiler started warning about this [2], so without this patch we will get: error: unused return value of `alloc::boxed::Box::<T>::from_raw` that must be used --> rust/kernel/sync/arc.rs:302:22 | 302 | unsafe { Box::from_raw(self.ptr.as_ptr()) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: call `drop(Box::from_raw(ptr))` if you intend to drop the `Box` = note: `-D unused-must-use` implied by `-D warnings` help: use `let _ = ...` to ignore the resulting value | 302 | unsafe { let _ = Box::from_raw(self.ptr.as_ptr()); }; | +++++++ + Thus add an add an explicit `drop()` as the `#[must_use]`'s annotation suggests (instead of the more general help line). Link: rust-lang/rust#104253 [1] Link: rust-lang/rust#112529 [2] Reviewed-by: Martin Rodriguez Reboredo <[email protected]> Reviewed-by: Gary Guo <[email protected]> Reviewed-by: Alice Ryhl <[email protected]> Reviewed-by: Andreas Hindborg <[email protected]> Reviewed-by: Björn Roy Baron <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]> Signed-off-by: Portia Stephens <[email protected]> Signed-off-by: Roxana Nicolescu <[email protected]>

rustbot assigned oli-obk Jun 11, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 11, 2023