rust-lang / rust-lang/rust-clippy

large_enum_variant proposed Box solution prohibits destructuring

Open
#7,700 0 comments 5 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
13.5k
Forks
2.2k
Avg merge
2d 10h
Merged PRs (30d)
32

Description

Using Rust 1.55.0 via Rustup on Ubuntu 21.04.

I hit the large_enum_variant warning on an enum where one variant contains a bunch of Option<String>s. Here's a super simple example:

struct Data {
    sec1: Option<[String; 4]>,
    sec2: Option<[String; 4]>,
    sec3: Option<[String; 4]>,
    sec4: Option<[String; 4]>,
}

enum ClippyWarn {
    One,
    Two,
    Three(Data),
    Four,
}

My actual code uses a lot of destructuring, and is extremely well suited to a "flat" kind of match where the big data is a kind of "primary" key, ie.

match (thing, other) {
    (Three(Data {sec1: Some(sec1), sec2: None, .. }), Other::A) => ...
    (Three(Data {sec1: Some(sec1), sec2: Some(sec2), .. }), Other::B) => ...
    // etc
}

I don't think this is particularly uncommon, I could be wrong. The point is, inverting this to match on the other fields would be an order of magnitude more verbose, and more error prone (basically, un-solving a problem that enums are great at solving).

large_enum_variant suggests putting the Data in a Box to avoid performance issues. However, once I did that, I was completely unable to match on it. I had no idea that directly matching on Box contents was impossible, since I rarely use them. This led me down a complete rabbit hole of nightly/unstable features and deprecated syntax (where I'm restricted to stable). Eventually I learned that it can't be done, and ignored the lint.

So why the issue? Well, I wouldn't have spent so much time looking, except that I assumed that because it was a Clippy lint, it was (a) widely applicable (is it?) and (b) would not make pattern matching harder (I think it necessarily does?).

My suggestions are:

  1. This lint seems a bit pre-emptive. If Box matching is slated for stabilisation, I think it would be wiser to wait for that to land.

  2. Make the online entry remind people that you would be sacrificing matchability, perhaps under the known problems subheading.

  3. Even the online entry for it says to measure first - if a Clippy lint needs code to be profiled before it should even be considered, and has syntactical and potential performance drawbacks, should it really be suggested by default?

  4. The online info also says:

    This lint obviously cannot take the distribution of variants in your running program into account. It is possible that the smaller variants make up less than 1% of all instances, in which case the overhead is negligible and the boxing is counter-productive.

    I'd argue that it's not really to do with the distribution of variants. If the variant is used 99% of the time that enum is used, but the program rarely uses that enum overall, or spends most of its time blocking, or doesn't need this kind performance tuning, it probably doesn't actually matter. Mentioning enum-variant-frequency is a bit of a red herring.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the large_enum_variant lint and its online entry, then review the reported Rust pattern-matching limitation around boxed enum contents. Determine whether the lint guidance should explain the matchability tradeoff or whether its recommendation needs reconsideration; done means an agreed direction for the lint behavior or documentation.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
documentation, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.