rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.62k stars 12.48k forks source link

Deriving Debug on an enum with uninhabited type triggers warning #38885

Open seanmonstar opened 7 years ago

seanmonstar commented 7 years ago
// no warning
#[derive(Debug)]
enum Void {}

// warns about unreachable pattern
#[derive(Debug)]
enum Foo {
    Bar(u8),
    Void(Void),
}

Caused by https://github.com/rust-lang/rust/pull/38069

scottmcm commented 7 years ago

Not seeing this in playground. Maybe it got fixed somewhere along the way?

Havvy commented 5 years ago

The code gives the following compiler output (with a #[crate_type="lib"]\n\n prefix)

warning: enum is never used: `Void`
 --> src/lib.rs:5:1
  |
5 | enum Void {}
  | ^^^^^^^^^
  |
  = note: #[warn(dead_code)] on by default

warning: enum is never used: `Foo`
 --> src/lib.rs:9:1
  |
9 | enum Foo {
  | ^^^^^^^^

There's no warning about an unreachable pattern anymore. As such, proposing this be closed.

Havvy commented 5 years ago

cc @estebank

estebank commented 5 years ago

@Havvy thanks for the heads up! Lets close with a test to avoid regressions from the current behavior.

LukasKalbertodt commented 5 years ago

This problem resurfaced, but only when actually using !. This code (Playground):

#![feature(never_type)]

#[derive(Debug)]
pub struct Foo(!);

#[derive(Debug)]
pub struct Bar(Empty);

#[derive(Debug)]
pub enum Empty {}

Leads to this output (with nightly-2019-02-06):

warning: unreachable expression
 --> src/lib.rs:5:16
  |
5 | pub struct Foo(!);
  |                ^
  |
  = note: #[warn(unreachable_code)] on by default

So should we reopen this issue?

estebank commented 5 years ago

Yes, reopen it or refile.

On Thu, Feb 7, 2019, 4:03 PM Lukas Kalbertodt notifications@github.com wrote:

This problem resurfaced, but only when actually using !. This code ( Playground https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=76b116e886c0a1d1af7e87b52d1b33e6 ):

![feature(never_type)]

[derive(Debug)]pub struct Foo(!);

[derive(Debug)]pub struct Bar(Empty);

[derive(Debug)]pub enum Empty {}

Leads to this output (with nightly-2019-02-06):

warning: unreachable expression --> src/lib.rs:5:16 5 pub struct Foo(!); ^

= note: #[warn(unreachable_code)] on by default

So should we reopen this issue?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/38885#issuecomment-461457064, or mute the thread https://github.com/notifications/unsubscribe-auth/ABiDIhaypq1z1kmjXnqwRGQcMxTJYowEks5vLEAtgaJpZM4LdAcT .

iluuu1994 commented 5 years ago

What's the reasoning behind a type like struct Foo(!)?

The warning comes from the expanded macro:

pub struct Foo(!);
#[automatically_derived]
#[allow(unused_qualifications)]
impl ::std::fmt::Debug for Foo {
    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
        match *self {
            Foo(ref __self_0_0) => {
                let mut debug_trait_builder = f.debug_tuple("Foo");
                let _ = debug_trait_builder.field(&&(*__self_0_0));
                                                   ^^^^^^^^^^^^^^
                                                   warning: unreachable expression
                debug_trait_builder.finish()
            }
        }
    }
}
LukasKalbertodt commented 5 years ago

What's the reasoning behind a type like struct Foo(!)?

Mainly some strange type level things. Adding a ! field makes sure that the struct is never created and thus only used at type level. There are some uses.


So to solve this one could simply add an #[allow(unreachable_code)] attribute to the method, right? Do you think this solution is OK? Or shouldn't that be done for some reason?

If that solution is acceptable, I could prepare a PR I guess. The debug-derive definition is in this file. One would probably just need to add an attribute here. Creating the attribute is probably the most complicated part about this. Looks like mk_attr_id, mk_spanned_attr_outer and some of these methods would be required for that.

iluuu1994 commented 5 years ago

I honestly don't understand enough about the never type. Why is it useful for Foo to implement Debug when it can never be instantiated? Is it something like this?

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=c06cdaff756d196380f250ec163105c6

But then why not just use never?

Do you think this solution is OK?

I'm new to rustc so probably not the person to answer this. It could theoretically hide other bugs in the future but I'm not sure if that's an issue.

LukasKalbertodt commented 5 years ago

Why is it useful for Foo to implement Debug when it can never be instantiated?

Yeah, of course that's kinda strange. But you already provided a nice example! Generics is where you just want to derive Debug and it's fine if the Debug impl for your type instantiated with ! is never called.

I discovered this issue because I have a #![deny(missing_debug_implementations)] in my crate and tried to slap a #[derive(Debug)] on my Foo instead of #[allow(missing_debug_implementations)]. So yeah, I found another solution in my specific situation. But the generated code shouldn't create a warning either way. This is probably a low-priority issue, but still.

I'm new to rustc so probably not the person to answer this. It could theoretically hide other bugs in the future but I'm not sure if that's an issue.

Let's just hope someone else is also reading this :D But yeah, that's the reason I asked: maybe someone knows about potential real warnings that would wrongfully be suppressed.

iluuu1994 commented 5 years ago

Let's just hope someone else is also reading this :D

Well, stupid English language, wasn't sure if "you" means "you" singular or "you" plural 😄

There's some discussion on whether never types should be convertible to all types (https://github.com/rust-lang/rfcs/issues/2619). Apparently there are some concerns but this would make the #[derive(Debug)] unnecessary. The error is technically correct as the generated code can never be executed.

Either way, those were just my two cents 🙂

estebank commented 5 years ago

https://github.com/rust-lang/rust/pull/63317 changes the output to be the following, which in my mind makes more sense than complaining about the non-use of Void, as it'd be used if Foo::Void were used:

warning: variant is never constructed: `Void`
  --> $DIR/derive-uninhabited-enum-38885.rs:13:5
   |
LL |     Void(Void),
   |     ^^^^^^^^^^
   |
   = note: `-W dead-code` implied by `-W unused`
augusto2112 commented 4 years ago

@rustbot claim

augusto2112 commented 4 years ago

@rustbot release-assignment

MilesConn commented 2 years ago

I might try and take this up and investigate it but if my understanding of #2619 is correct this is currently the intended behavior. ! does not blanket implement all traits. That said I did want to provide an additional use case.

Currently I'm writing a compiler for school and I wanted to use phantom types to differentiate between 32-bit and 64-bit registers like so

#[derive(Debug, Hash, PartialEq, Eq, Clone, Copy, EnumIter, Serialize)]
pub enum Register<T: Default> {
  RAX,
  RBX,
  RCX,
  RDX,
  RDI,
  RSI,
  RBP,
  RSP,
  R8,
  R9,
  R10,
  R11,
  R12,
  R13,
  R14,
  R15,
  _Unreachable(PhantomData<T>),
}

however I'd also like to make _Unreachable an unconstructable variant so I could exclude it from matches ie something like _Unreachable(!, PhantomData<T>) or _Unreachable(Infallible, PhantomData<T>). However, since ! doesn't auto implement every trait I get complaints about it not being serializable despite the variant not being able to be constructed.

For now one work around is to implement your own never equivalent type

enum MyNever {}

and then implementing your desired traits.

scottmcm commented 2 years ago

@MilesConn Sounds like you might be more interested in https://github.com/serde-rs/serde/issues/2073 than this issue?

MilesConn commented 2 years ago

@MilesConn Sounds like you might be more interested in serde-rs/serde#2073 than this issue?

Yeah that's more on topic! Sorry I had a lot of tabs open when I was going down this rabbit hole. Ended up with a slightly different implementation but thank you.

kpreid commented 3 months ago

Possibly of interest: The pedantic Clippy lint empty_enum currently proposes replacing enum Foo {} with struct Foo(!) (when possible), but doing so triggers this bug. I've started a discussion of whether that's actually good, but supposing it is, this bug will become much more significant once ! is stable.