[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)

[Julian Schmidt via cfe-commits]

================
@@ -0,0 +1,82 @@
+//===--- EnumInitialValueCheck.cpp - clang-tidy ---------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "EnumInitialValueCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+namespace {
+
+AST_MATCHER(EnumDecl, isNoneEnumeratorsInitialized) {
+  return llvm::all_of(Node.enumerators(), [](EnumConstantDecl const *ECD) {
+    return ECD->getInitExpr() == nullptr;
+  });
+}
+
+AST_MATCHER(EnumDecl, isOnlyFirstEnumeratorsInitialized) {
+  for (EnumConstantDecl const *ECD : Node.enumerators())
+    if (ECD == *Node.enumerator_begin()) {
+      if (ECD->getInitExpr() == nullptr)
+        return false;
+    } else {
+      if (ECD->getInitExpr() != nullptr)
+        return false;
+    }
+  return true;
+}
+
+AST_MATCHER(EnumDecl, isAllEnumeratorsInitialized) {
+  return llvm::all_of(Node.enumerators(), [](EnumConstantDecl const *ECD) {
+    return ECD->getInitExpr() != nullptr;
+  });
+}
+
+} // namespace
+
+void EnumInitialValueCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(enumDecl(unless(anyOf(isNoneEnumeratorsInitialized(),
+                                           isOnlyFirstEnumeratorsInitialized(),
+                                           isAllEnumeratorsInitialized())))
----------------
5chmidti wrote:

I think it's possible to detect these linear enum initializations and remove a lot of false positives by placing some restrictions on which ones to diagnose.
I came up with:
- must be linear
- must include an enumerator whose value is not a power of two
- every enumeration value must be initialized with a (potentially negative) integer literal

Code:
```c++
/// Check if \p Enumerator is initialized with a (potentially negated)
/// \c IntegerLiteral.
bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) {
  const Expr *const Init = Enumerator->getInitExpr();
  if (!Init)
    return false;
  const auto *const Constant = llvm::dyn_cast<ConstantExpr>(Init);
  if (!Constant)
    return false;
  if (llvm::isa<IntegerLiteral>(Constant->getSubExpr()))
    return true;
  const auto *const NegativeValue =
      llvm::dyn_cast<UnaryOperator>(Constant->getSubExpr());
  if (!NegativeValue)
    return false;
  return llvm::isa<IntegerLiteral>(NegativeValue->getSubExpr());
}

/// Excludes bitfields because enumerators initialized with the result of a
/// bitwise operator on enumeration values or any other expr that is not a
/// potentially negative integer literal.
/// Enumerations where it is not directly clear if they are used with
/// bitmasking, evident when enumerators are only inintialized with (potentially
/// negative) integer literals, are ignored. This is also the case when all
/// enumerators are powers of two (e.g., 0, 1, 2).
bool isLinearAndDiagnosable(const EnumDecl *Enum) {
  if (Enum->enumerators().empty())
    return false;

  const EnumConstantDecl *const FirstEnumerator = *Enum->enumerator_begin();
  llvm::APSInt PrevValue = FirstEnumerator->getInitVal();
  if (!isInitializedByLiteral(FirstEnumerator))
    return false;
  bool AllEnumeratorsArePowersOfTwo = true;
  for (const EnumConstantDecl *Enumerator :
       llvm::drop_begin(Enum->enumerators())) {
    const llvm::APSInt NewValue = Enumerator->getInitVal();
    if (NewValue != ++PrevValue)
      return false;

    if (!isInitializedByLiteral(Enumerator))
      return false;

    PrevValue = NewValue;
    AllEnumeratorsArePowersOfTwo &= NewValue.isPowerOf2();
  }

  return !AllEnumeratorsArePowersOfTwo;
}
```
to test: 
Add 
```c++
  if (isLinearAndDiagnosable(Enum)) {
    diag(Loc, "linear enum %0") << Enum;
  }
  return; // to only test matching linear enums
```
after the `Loc.isInvalid()...` check in `check` and remove `isAllEnumeratorsInitialized` from the matcher.

<details><summary>Example Enums</summary>
enum class EAllUnclear {
  EAll_a = 0,
  EAll_b = 1,
  EAll_c = 2,
};

enum class EAllBitmask {
  EAll_a = 1,
  EAll_b = 2,
  EAll_c = EAll_a | EAll_b,
};

enum class EAllWithNegative {
  EAll_n2 = -2,
  EAll_n1 = -1,
  EAll_0 = 0,
  EAll_a = 1,
  EAll_b = 2,
};

enum class EAllWithNegative1 {
  EAll_n1 = -1,
  EAll_0 = 0,
  EAll_a = 1,
  EAll_b = 2,
};

enum class EAllWithNegative3 {
  EAll_n3 = -3,
  EAll_n2 = -2,
  EAll_n1 = -1,
  EAll_0 = 0,
  EAll_a = 1,
  EAll_b = 2,
};


enum class EAllOffset {
  EAll_a = 8,
  EAll_b = 9,
  EAll_c = 10,
  EAll_d = 11,
};
</details>

Findings when running clang-tidy over `clang/lib/Basic` with `-header-filter="-*"`:
https://github.com/llvm/llvm-project/blob/2f6b1b4b30e3a719b1744baa4cd1ece504998c6e/clang/include/clang/Basic/LangOptions.h#L276
https://github.com/llvm/llvm-project/blob/2f6b1b4b30e3a719b1744baa4cd1ece504998c6e/clang/include/clang/Basic/LangOptions.h#L388
https://github.com/llvm/llvm-project/blob/2f6b1b4b30e3a719b1744baa4cd1ece504998c6e/clang/include/clang/Basic/DiagnosticIDs.h#L85
https://github.com/llvm/llvm-project/blob/2f6b1b4b30e3a719b1744baa4cd1ece504998c6e/clang/include/clang/Basic/Specifiers.h#L388
https://github.com/llvm/llvm-project/blob/2f6b1b4b30e3a719b1744baa4cd1ece504998c6e/llvm/include/llvm/Support/CodeGen.h#L54
https://github.com/llvm/llvm-project/blob/2f6b1b4b30e3a719b1744baa4cd1ece504998c6e/llvm/include/llvm/Support/Unicode.h#L27
(and three in `OMP.h.inc` (table-gen)).

https://github.com/llvm/llvm-project/pull/86129