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

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

Open
seanmonstar opened this issue Jan 6, 2017 · 18 comments · Fixed by #55567
Open

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

seanmonstar opened this issue Jan 6, 2017 · 18 comments · Fixed by #55567
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@seanmonstar
Copy link
Contributor

seanmonstar commented Jan 6, 2017

// no warning
#[derive(Debug)]
enum Void {}

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

Caused by #38069

@Mark-Simulacrum Mark-Simulacrum added the A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. label May 19, 2017
@scottmcm
Copy link
Member

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

@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 26, 2017
@Havvy
Copy link
Contributor

Havvy commented Sep 29, 2018

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
Copy link
Contributor

Havvy commented Sep 30, 2018

cc @estebank

@estebank estebank added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. labels Oct 1, 2018
@estebank
Copy link
Contributor

estebank commented Oct 1, 2018

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

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Nov 3, 2018
…ikomatsakis

add test for deriving Debug on uninhabited enum

Adds a test to close rust-lang#38885.
@LukasKalbertodt
Copy link
Member

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
Copy link
Contributor

estebank commented Feb 7, 2019 via email

@iluuu1994
Copy link
Contributor

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
Copy link
Member

LukasKalbertodt commented Jul 26, 2019

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor

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 (rust-lang/rfcs#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
Copy link
Contributor

estebank commented Aug 6, 2019

#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
Copy link

@rustbot claim

@rustbot rustbot self-assigned this Apr 18, 2020
@augusto2112
Copy link

@rustbot release-assignment

@rustbot rustbot removed their assignment May 10, 2020
@JohnTitor JohnTitor added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. labels May 18, 2020
@MilesConn
Copy link

MilesConn commented Oct 25, 2021

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
Copy link
Member

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

@MilesConn
Copy link

@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
Copy link
Contributor

kpreid commented May 20, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. 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.