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

necessary unsafe keywords marked as unnecessary when inside incomplete match #115348

Closed
sam0x17 opened this issue Aug 29, 2023 · 5 comments · Fixed by #115587 or #115615
Closed

necessary unsafe keywords marked as unnecessary when inside incomplete match #115348

sam0x17 opened this issue Aug 29, 2023 · 5 comments · Fixed by #115587 or #115615
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@sam0x17
Copy link

sam0x17 commented Aug 29, 2023

Code

enum MyEnum {
    Red,
    Blue,
    Purple,
    Gray,
    Orange,
}

fn my_fn(input: MyEnum) -> u32 {
    let bytes1 = [1u8, 2u8, 3u8, 4u8];
    let bytes2 = [1u8, 3u8, 3u8, 4u8];
    let bytes3 = [1u8, 2u8, 0u8, 4u8];
    let bytes4 = [1u8, 7u8, 3u8, 4u8];
    match input {
        MyEnum::Red => unsafe { core::mem::transmute_copy(&bytes1) },
        MyEnum::Blue => unsafe { core::mem::transmute_copy(&bytes2) },
        MyEnum::Purple => unsafe { core::mem::transmute_copy(&bytes3) },
        MyEnum::Gray => unsafe { core::mem::transmute_copy(&bytes4) },
    }
}

Current output

error[E0004]: non-exhaustive patterns: `MyEnum::Orange` not covered
  --> src/lib.rs:15:11
   |
15 |     match input {
   |           ^^^^^ pattern `MyEnum::Orange` not covered
   |
note: `MyEnum` defined here
  --> src/lib.rs:7:5
   |
2  | enum MyEnum {
   |      ------
...
7  |     Orange,
   |     ^^^^^^ not covered
   = note: the matched value is of type `MyEnum`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
   |
19 ~         MyEnum::Gray => unsafe { core::mem::transmute_copy(&bytes4) },
20 ~         MyEnum::Orange => todo!(),
   |

warning: unnecessary `unsafe` block
  --> src/lib.rs:16:24
   |
16 |         MyEnum::Red => unsafe { core::mem::transmute_copy(&bytes1) },
   |                        ^^^^^^ unnecessary `unsafe` block
   |
   = note: `#[warn(unused_unsafe)]` on by default

warning: unnecessary `unsafe` block
  --> src/lib.rs:17:25
   |
17 |         MyEnum::Blue => unsafe { core::mem::transmute_copy(&bytes2) },
   |                         ^^^^^^ unnecessary `unsafe` block

warning: unnecessary `unsafe` block
  --> src/lib.rs:18:27
   |
18 |         MyEnum::Purple => unsafe { core::mem::transmute_copy(&bytes3) },
   |                           ^^^^^^ unnecessary `unsafe` block

warning: unnecessary `unsafe` block
  --> src/lib.rs:19:25
   |
19 |         MyEnum::Gray => unsafe { core::mem::transmute_copy(&bytes4) },
   |                         ^^^^^^ unnecessary `unsafe` block

For more information about this error, try `rustc --explain E0004`.
warning: `playground` (lib) generated 4 warnings
error: could not compile `playground` (lib) due to previous error; 4 warnings emitted

Desired output

error[E0004]: non-exhaustive patterns: `MyEnum::Orange` not covered
  --> src/lib.rs:15:11
   |
15 |     match input {
   |           ^^^^^ pattern `MyEnum::Orange` not covered
   |
note: `MyEnum` defined here
  --> src/lib.rs:7:5
   |
2  | enum MyEnum {
   |      ------
...
7  |     Orange,
   |     ^^^^^^ not covered
   = note: the matched value is of type `MyEnum`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
   |
19 ~         MyEnum::Gray => unsafe { core::mem::transmute_copy(&bytes4) },
20 ~         MyEnum::Orange => todo!(),
   |
For more information about this error, try `rustc --explain E0004`.
warning: `playground` (lib) generated 4 warnings
error: could not compile `playground` (lib) due to previous error; 4 warnings emitted

Rationale and extra context

Each of the above unsafe keywords gets marked with an "unnecessary" warning until the missing match arms are added at which point the warnings disappear. This is confusing since many will see those warnings and remove the unsafe blocks only to need to re-add them when they finish writing their match.

Originally reported on RA here: rust-lang/rust-analyzer#15514 (comment)

Other cases

No response

Anything else?

Any scenario where you have a match statement with necessary unsafe blocks in any of the arms, and the match statement is incomplete.

@sam0x17 sam0x17 added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 29, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 29, 2023
@asquared31415
Copy link
Contributor

Minimized:

unsafe fn uwu() {}

fn foo(x: Option<u32>) {
    match x {
        Some(_) => unsafe { uwu() },
    }
}

@rustbot label -needs-triage +D-incorrect

@rustbot rustbot added D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 29, 2023
@SNCPlay42
Copy link
Contributor

No "unnecessary unsafe block" warning is emitted on 1.70 and before.

@rustbot label regression-from-stable-to-stable

@rustbot rustbot added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Aug 30, 2023
@dtolnay
Copy link
Member

dtolnay commented Aug 30, 2023

@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Aug 30, 2023
@Noratrieb Noratrieb added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. labels Aug 30, 2023
@kiscad
Copy link
Contributor

kiscad commented Sep 5, 2023

@rustbot claim

@kiscad kiscad mentioned this issue Sep 6, 2023
fmease added a commit to fmease/rust that referenced this issue Sep 6, 2023
fix rust-lang#115348

fix rust-lang#115348
It looks that:

- In `rustc_mir_build::build`, the body of function will not be built, when the `tcx.check_match(def)` fails due to `non-exhaustive patterns`
- In `rustc_mir_transform::check_unsafety`, the `UnsafetyChecker` collects all `used_unsafe_blocks` in the MIR of a function, and the `UnusedUnsafeVisitor` will visit all `UnsafeBlock`s in the HIR and collect `unused_unsafes`, which are not contained in `used_unsafe_blocks`, and report `unnecessary_unsafe`s
- So the unsafe block in the issue example code will be reported as `unnecessary_unsafe`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 6, 2023
fix rust-lang#115348

fix rust-lang#115348
It looks that:

- In `rustc_mir_build::build`, the body of function will not be built, when the `tcx.check_match(def)` fails due to `non-exhaustive patterns`
- In `rustc_mir_transform::check_unsafety`, the `UnsafetyChecker` collects all `used_unsafe_blocks` in the MIR of a function, and the `UnusedUnsafeVisitor` will visit all `UnsafeBlock`s in the HIR and collect `unused_unsafes`, which are not contained in `used_unsafe_blocks`, and report `unnecessary_unsafe`s
- So the unsafe block in the issue example code will be reported as `unnecessary_unsafe`.
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 6, 2023
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#114511 (Remove the unhelpful let binding diag comes from FormatArguments)
 - rust-lang#115473 (Add explanatory note to 'expected item' error)
 - rust-lang#115574 (Replace `rustc_data_structures` dependency with `rustc_index` in `rustc_parse_format`)
 - rust-lang#115578 (Clarify cryptic comments)
 - rust-lang#115587 (fix rust-lang#115348)
 - rust-lang#115596 (A small change)
 - rust-lang#115598 (Fix log formatting in bootstrap)
 - rust-lang#115605 (Better Debug for `Ty` in smir)
 - rust-lang#115614 (Fix minor grammar typo)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors closed this as completed in df6e6a6 Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
8 participants