large_enum_variant false positive (incorrect size calculation?)

[djc]

Summary

There seems to be a regression in calculating the size for the type contained in a variant here.

https://github.com/tokio-rs/tls/actions/runs/3391306304/jobs/5636351880

Lint Name

large_enum_variant

Reproducer

I tried this code:

pub enum TlsStream<T> {
    Client(client::TlsStream<T>),
    Server(server::TlsStream<T>),
}

I saw this happen:

Error:    --> tokio-rustls/src/lib.rs:393:1
    |
393 | / pub enum TlsStream<T> {
394 | |     Client(client::TlsStream<T>),
    | |     ---------------------------- the second-largest variant contains at least 0 bytes
395 | |     Server(server::TlsStream<T>),
    | |     ---------------------------- the largest variant contains at least 608 bytes
396 | | }
    | |_^ the entire enum is at least 0 bytes
    |
    = note: `-D clippy::large-enum-variant` implied by `-D warnings`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant
help: consider boxing the large fields to reduce the total size of the enum
    |
395 |     Server(Box<server::TlsStream<T>>),
    |            ~~~~~~~~~~~~~~~~~~~~~~~~~

I expected to see this happen:

Smaller size difference, or maybe no lint at all.

Version

rustc 1.65.0 (897e37553 2022-11-02)
binary: rustc
commit-hash: 897e37553bba8b42751c67658967889d11ecd120
commit-date: 2022-11-02
host: aarch64-apple-darwin
release: 1.65.0
LLVM version: 15.0.0

Additional Labels

Seems to be a regression in 1.65.

[lukaslueg]

Before 1.65 large_enum_variant could not determine the size of enums that have type parameters on them. Since 1.65 it will de-facto assume a ZST for any type parameter. In the example above client::TlsStream<T> will be a ZST/indeterminate, but server::TlsStream<T> will be at least 608 bytes for any T, and only grow from there, triggering the lint-threshold of 200 bytes. That is, no matter what T is, the enum would lint.

[djc]

Why do you think client::TlsStream<T> will be a ZST/indeterminate? This is client::TlsStream:

pub struct TlsStream<IO> {
    pub(crate) io: IO,
    pub(crate) session: ClientConnection,
    pub(crate) state: TlsState,

    #[cfg(feature = "early-data")]
    pub(crate) early_waker: Option<std::task::Waker>,
}

While this is server::TlsStream<T>:

pub struct TlsStream<IO> {
    pub(crate) io: IO,
    pub(crate) session: ServerConnection,
    pub(crate) state: TlsState,
}

If it is able to deduce that server::TlsStream is at least 608 bytes, why does it think client::TlsStream is at least 0 bytes, instead of something more? (ServerConnection and ClientConnection should be similar in size.)

[kangalio]

In serenity, this bug is happening even without any generics.

/// A container for any channel.
#[derive(Clone, Debug, Serialize)]
#[serde(untagged)]
#[non_exhaustive]
pub enum Channel {
    /// A channel within a [`Guild`].
    Guild(GuildChannel),
    /// A private channel to another [`User`] (Direct Message). No other users may access the
    /// channel. For multi-user "private channels", use a group.
    Private(PrivateChannel),
}

warning: large size difference between variants
  --> src/model/channel/mod.rs:47:1
   |
47 | / pub enum Channel {
48 | |     /// A channel within a [`Guild`].
49 | |     Guild(GuildChannel),
   | |     ------------------- the largest variant contains at least 312 bytes
50 | |     /// A private channel to another [`User`] (Direct Message). No other users may access the
51 | |     /// channel. For multi-user "private channels", use a group.
52 | |     Private(PrivateChannel),
   | |     ----------------------- the second-largest variant contains at least 0 bytes
53 | | }
   | |_^ the entire enum is at least 0 bytes
   |
   = note: `#[warn(clippy::large_enum_variant)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant
help: consider boxing the large fields to reduce the total size of the enum
   |
49 |     Guild(Box<GuildChannel>),

[kangalio]

Apparently, clippy's size calculation chokes on recursive types:

pub struct Recursive(Box<Recursive>);

pub enum Foo {
    A([u8; 201]),
    B(Recursive),
}

// 5 | |     B(Recursive),
//   | |     ------------ the second-largest variant contains at least 0 bytes

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=43e0a41a5cc088d5aaba28fcada26ed9

(In serenity's case, PrivateChannel contained a User, which contained a Box<PartialMember>, which contained a User)

[Wilfred]

I hit this today, and the text "the second-largest variant contains at least 0 bytes" makes it hard to trust this specific lint.

I believe this is indicative of a genuine issue with my code, but just that the reporting is confused by a recursive type.