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

Overzealous unconditional_panic (denied) warning #78803

Closed
Nemo157 opened this issue Nov 6, 2020 · 2 comments · Fixed by #109792
Closed

Overzealous unconditional_panic (denied) warning #78803

Nemo157 opened this issue Nov 6, 2020 · 2 comments · Fixed by #109792
Labels
A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. A-control-flow Area: Control flow A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Nemo157
Copy link
Member

Nemo157 commented Nov 6, 2020

To try and workaround the lack of const Result::unwrap on 1.47 I implemented a basic variant of it manually, but this fails to compile without suppressing the unconditional_panic warning:

#[derive(Copy, Clone)]
pub struct Foo;

impl Foo {
    pub const fn new_unwrap(success: bool) -> Self {
        let result = if success { Ok(Foo) } else { Err(()) };
        [][match result {
            Ok(foo) => return foo,
            Err(_) => 0,
        }]
    }
}

fn main() {}
error: this operation will panic at runtime
  --> uwu.rs:7:9
   |
7  | /         [][match result {
8  | |             Ok(foo) => return foo,
9  | |             Err(_) => 0,
10 | |         }]
   | |__________^ index out of bounds: the length is 0 but the index is 0
   |
   = note: `#[deny(unconditional_panic)]` on by default

error: aborting due to previous error

This code panicking is conditional on the input success value.

@rustbot modify labels: +A-const-fn, +A-lint

@Nemo157 Nemo157 added the C-bug Category: This is a bug. label Nov 6, 2020
@rustbot rustbot added A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. labels Nov 6, 2020
@jyn514 jyn514 added A-control-flow Area: Control flow T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 6, 2020
@Noratrieb Noratrieb self-assigned this Mar 16, 2023
@Noratrieb
Copy link
Member

This falls under the general problem of unconditional_panic is unaware of actual control flow.
As a minimal example:

fn main() {
    if false {
        [()][1];
    }
}

this fires the lint, because of course this statement panics at runtime - unconditional_panic simply doesn't consider that. Fixing this would generally bring up the design question of this lint (how many false positives do we want to have?) and is some more effort that I don't have time for right now

@Noratrieb Noratrieb removed their assignment Mar 16, 2023
@Nemo157
Copy link
Member Author

Nemo157 commented Mar 16, 2023

There's a difference between that form and the more minimal example of this form:

fn main() {
  [()][if false { 1 } else { return }]
}

Specifically, in your example there is a sub-expression ([()][1]) which is an unconditional panic, it's just contained within an overall condition that avoids that expression being reached. In the form I was using there is no panicking sub-expression in the source code, only after some kind of MIR transformation does it make it look unconditional to the compiler (not one disabled by using -Zmir-opt-level=0, I think it might just be inherent in how if-expressions are handled).

Though I'm not sure if this distinction really matters, and we have const-panic! now to avoid having to use this weird form anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. A-control-flow Area: Control flow A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants