================ @@ -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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits