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

Warn when a match can be made more specific #75235

Closed
jyn514 opened this issue Aug 6, 2020 · 3 comments
Closed

Warn when a match can be made more specific #75235

jyn514 opened this issue Aug 6, 2020 · 3 comments
Labels
A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Aug 6, 2020

Consider the following code (playground):

enum E {
    A(usize),
    B(usize),
}

impl E {
    fn as_usize(&self) -> usize {
        match *self {
            E::A(a) => a,
            E::B(b) => b,
        }
    }
}

fn main() {
    let _ = E::B(1); // ignore warn(dead_code) on the B variant
    let e = E::A(1);
    match e {
        E::A(x) => x,
        x => x.as_usize(),
    };
}

This code gives no warnings. However, the as_usize() method is entirely useless, because the match can be rewritten to be more specific:

    match e {
        E::A(x) => x,
        E::B(x) => x,
    };

The compiler should give a warning in such a case.

@jyn514 jyn514 added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Aug 6, 2020
@jyn514
Copy link
Member Author

jyn514 commented Aug 6, 2020

To be clear, the warning should be a suggestion to change x => x.as_usize() to E::B(x) => _. The warning about as_usize() (dead_code) will show up automatically after that.

@jyn514 jyn514 added A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 22, 2021
@Mark-Simulacrum
Copy link
Member

Hm, I think this likely wants to by allow by default and likely even a clippy warning. It can make sense to handle one case and provide for a general code path for all other cases, even if there is (currently) only one other case. This could also arise if other cases are conditionally available via cfg, for example.

@Nadrieril
Copy link
Member

That would require cross-function analysis of patterns, plus I don't see how to generalize it beyond this simple example to some notion of what is or isn't useful. Pretty confident this is beyond the kind of things we want to lint in the compiler, feel free to reopen if I'm making a mistake.

@Nadrieril Nadrieril closed this as not planned Won't fix, can't repro, duplicate, stale Apr 4, 2023
@fmease fmease added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. and removed A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. labels Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants