Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unused_must_use is silenced when returning a #[must_use] value from an unsafe block and not using it #93925

Closed
PatchMixolydic opened this issue Feb 12, 2022 · 4 comments
Labels
A-FFI Area: Foreign function interface (FFI) C-bug Category: This is a bug.

Comments

@PatchMixolydic
Copy link
Contributor

Found while working on a fix for #93906.

I tried this code (playground):

#![feature(core_intrinsics)]

#[no_mangle]
#[must_use]
fn foo() -> i64 {
    4
}

extern "Rust" {
    #[link_name = "foo"]
    #[must_use]
    fn foreign_foo() -> i64;
}

fn main() {
    unsafe { foreign_foo() };
    unsafe { std::intrinsics::arith_offset::<isize>(std::ptr::null(), 0) };
}

I expected to see this happen:

Both lines in main should trip unused_must_use. Note that core::intrinsics::arith_offset is marked as #[must_use]:

#[must_use = "returns a new pointer rather than modifying its argument"]
#[rustc_const_unstable(feature = "const_ptr_offset", issue = "71499")]
pub fn arith_offset<T>(dst: *const T, offset: isize) -> *const T;

Instead, this happened:

nothing

Meta

rustc version: 1.60.0-nightly (2022-02-10 e646f3d2a95419523107)

@rustbot modify labels +A-ffi

@PatchMixolydic PatchMixolydic added the C-bug Category: This is a bug. label Feb 12, 2022
@rustbot rustbot added the A-FFI Area: Foreign function interface (FFI) label Feb 12, 2022
@PatchMixolydic PatchMixolydic changed the title #[must_use] on a foreign fn does nothing #[must_use] is silenced when returning from an unsafe block Feb 12, 2022
@PatchMixolydic
Copy link
Contributor Author

PatchMixolydic commented Feb 12, 2022

Note that adding a semicolon after the expressions within the unsafe blocks will cause #[must_use] to trip (playground):

-     unsafe { foreign_foo() };
+     unsafe { foreign_foo(); };
warning: unused return value of `foreign_foo` that must be used
  --> src/main.rs:16:14
   |
16 |     unsafe { foreign_foo(); };
   |              ^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_must_use)]` on by default

This is documented in the Rust Reference, but this case may still be an issue since one might not think of unsafe { foreign_foo() }; as a use, much like I did when initially filing this issue.

@PatchMixolydic PatchMixolydic changed the title #[must_use] is silenced when returning from an unsafe block #[must_use] is silenced when returning a #[must_use] value from an unsafe block and not using it Feb 12, 2022
@PatchMixolydic PatchMixolydic changed the title #[must_use] is silenced when returning a #[must_use] value from an unsafe block and not using it unused_must_use is silenced when returning a #[must_use] value from an unsafe block and not using it Feb 12, 2022
@asquared31415
Copy link
Contributor

minimal version with no extern or intrinsics:

#[must_use]
unsafe fn foo() -> i64 {
    4
}

fn main() {
    unsafe { foo() };
}

playground

@asquared31415
Copy link
Contributor

Oh the unsafe isn't even required. Just returning it from a block (and then not binding the block's value) fails to show the warning as well

#[must_use]
fn foo() -> i64 {
    4
}

fn main() {
   { foo() };
}

playground

@PatchMixolydic
Copy link
Contributor Author

PatchMixolydic commented Apr 13, 2022

Note that unused_must_used not firing on block returns is documented/intentional behaviour. Quoth the Reference:

Note: Trivial no-op expressions containing the value will not violate the lint. Examples include wrapping the value in a type that does not implement Drop and then not using that type and being the final expression of a block expression that is not used.

At the time I opened this issue, I thought that applying this lint to unsafe blocks greatly decreased the utility of #[must_use] on unsafe fn, as it is common practice to scope unsafe blocks as tightly as possible, resulting in constructions like:

let x = unsafe { get_x_unchecked() };

However, this lint only trips for unsafe blocks followed by a semicolon, which are uncommon. Like with most block-like expressions, statements which consist of an unsafe block and no semicolon must be of type () (playground for improper unsafe return):

unsafe fn x() -> i32 {
    4
}

fn main() {
    unsafe {
        x()
    }

    // avoid interpreting the unsafe block as `main`'s return expression
    ()
}
error[E0308]: mismatched types
 --> src/main.rs:7:9
  |
7 |         x()
  |         ^^^- help: consider using a semicolon here: `;`
  |         |
  |         expected `()`, found `i32`

Empirically, most unsafe blocks followed by a semicolon are already part of a larger statement (usually a let statement). Because of this, I don't think this issue is an actual issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-FFI Area: Foreign function interface (FFI) C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

3 participants