llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.85k stars 11.47k forks source link

[clang-tidy][check-request] Detect unnecessary casts to enum type #99030

Open chrchr-github opened 1 month ago

chrchr-github commented 1 month ago
enum E {
    E0
};

void f(int i, E e) {
    if ((E)i != e) {}
}

If the base types are the same, the cast does nothing.

jace0x21 commented 1 month ago

It seems like the following AST matcher might work? Unsure if there is a better way to determine the enum's underlying type.

explicitCastExpr(
    hasDestinationType(
        hasCanonicalType(enumType(
        hasDeclaration(hasDescendant(enumConstantDecl(
        hasType(type().bind("enum_type")))))))), 
    hasSourceExpression(
        hasType(
        type(equalsBoundNode("enum_type")))
    )
)
PiotrZSL commented 1 month ago

In this case that cast can be redundant, but it may be a must if operator != would be overloaded. Additional I would say that this cast could potentially help detect other issues (like cast big integer to small enum) and may be a nice thing to explicitly say what developer had in mind. Thing is that basiclly most of types are casted into int for binaryOperator, same could be this:

void f(int i, bool e) {
  if (i != e) {
  }
}

but we got an check readability-implicit-bool-conversion to detect such conversions.

I'm not sure about this, from one point of view it make sense, from other point of view I prefer version with cast.

chrchr-github commented 1 month ago

From what I've seen in real-world code, some people seem to think that a cast truncates to valid enum values. In the example with bool e, the types involved have at least a different size.

firewave commented 1 month ago

Possibly related: #38749.