[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/HerrCai0907 created https://github.com/llvm/llvm-project/pull/86129 Fixes: #85243. >From 4e0845a143a820d4a68ffbdced206654c7593359 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Fri, 15 Mar 2024 08:07:47 +0800 Subject: [PATCH] [clang-tidy] add new check readability-enum-initial-value Fixes: #85243. --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/EnumInitialValueCheck.cpp | 82 +++ .../readability/EnumInitialValueCheck.h | 31 +++ .../readability/ReadabilityTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../checks/readability/enum-initial-value.rst | 45 ++ .../checkers/readability/enum-initial-value.c | 27 ++ .../readability/enum-initial-value.cpp| 27 ++ 9 files changed, 223 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/enum-initial-value.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5728c9970fb65d..dd772d69202548 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -17,6 +17,7 @@ add_clang_library(clangTidyReadabilityModule DeleteNullPointerCheck.cpp DuplicateIncludeCheck.cpp ElseAfterReturnCheck.cpp + EnumInitialValueCheck.cpp FunctionCognitiveComplexityCheck.cpp FunctionSizeCheck.cpp IdentifierLengthCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp new file mode 100644 index 00..78d5101d439dde --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp @@ -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( + .bind("enum"), + this); +} + +void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Enum = Result.Nodes.getNodeAs("enum"); + assert(Enum != nullptr); + SourceLocation Loc = Enum->getBeginLoc(); + if (Loc.isInvalid() || Loc.isMacroID()) +return; + DiagnosticBuilder Diag = + diag(Loc, "inital value in enum %0 has readability issue, " +"explicit initialization of all of enumerators") + << Enum->getName(); + for (EnumConstantDecl const *ECD : Enum->enumerators()) +if (ECD->getInitExpr() == nullptr) { + SourceLocation ECDLoc = ECD->getEndLoc(); + if (ECDLoc.isInvalid() || ECDLoc.isMacroID()) +continue; + std::optional Next = utils::lexer::findNextTokenSkippingComments( + ECDLoc, *Result.SourceManager, getLangOpts()); + if (!Next.has_value() || Next->getLocation().isMacroID()) +continue; + llvm::SmallString<
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
llvmbot wrote: @llvm/pr-subscribers-clang-tidy Author: Congcong Cai (HerrCai0907) Changes Fixes: #85243. --- Full diff: https://github.com/llvm/llvm-project/pull/86129.diff 9 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/CMakeLists.txt (+1) - (added) clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp (+82) - (added) clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h (+31) - (modified) clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp (+3) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6) - (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) - (added) clang-tools-extra/docs/clang-tidy/checks/readability/enum-initial-value.rst (+45) - (added) clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c (+27) - (added) clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.cpp (+27) ``diff diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5728c9970fb65d..dd772d69202548 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -17,6 +17,7 @@ add_clang_library(clangTidyReadabilityModule DeleteNullPointerCheck.cpp DuplicateIncludeCheck.cpp ElseAfterReturnCheck.cpp + EnumInitialValueCheck.cpp FunctionCognitiveComplexityCheck.cpp FunctionSizeCheck.cpp IdentifierLengthCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp new file mode 100644 index 00..78d5101d439dde --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp @@ -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( + .bind("enum"), + this); +} + +void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Enum = Result.Nodes.getNodeAs("enum"); + assert(Enum != nullptr); + SourceLocation Loc = Enum->getBeginLoc(); + if (Loc.isInvalid() || Loc.isMacroID()) +return; + DiagnosticBuilder Diag = + diag(Loc, "inital value in enum %0 has readability issue, " +"explicit initialization of all of enumerators") + << Enum->getName(); + for (EnumConstantDecl const *ECD : Enum->enumerators()) +if (ECD->getInitExpr() == nullptr) { + SourceLocation ECDLoc = ECD->getEndLoc(); + if (ECDLoc.isInvalid() || ECDLoc.isMacroID()) +continue; + std::optional Next = utils::lexer::findNextTokenSkippingComments( + ECDLoc, *Result.SourceManager, getLangOpts()); + if (!Next.has_value() || Next->getLocation().isMacroID()) +continue; + llvm::SmallString<8> Str{" = "}; + ECD->getInitVal().toString(Str); + Diag << FixItHint::CreateInsertion(Next->getLocation(), Str); +} +} + +} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h new file mode 100644 index 00..6b4e0e28e35be0 --- /dev/null +++
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff ffe41819e58365dfbe85a22556c0d9d284e746b9 4e0845a143a820d4a68ffbdced206654c7593359 -- clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.cpp clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp `` View the diff from clang-format here. ``diff diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h index 6b4e0e28e3..8a0ddd2b38 100644 --- a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h +++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h @@ -13,8 +13,8 @@ namespace clang::tidy::readability { -/// Detects explicit initialization of a part of enumerators in an enumeration, and -/// relying on compiler to initialize the others. +/// Detects explicit initialization of a part of enumerators in an enumeration, +/// and relying on compiler to initialize the others. /// /// For the user-facing documentation see: /// http://clang.llvm.org/extra/clang-tidy/checks/readability/enum-initial-value.html `` 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/86129 >From 4e0845a143a820d4a68ffbdced206654c7593359 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Fri, 15 Mar 2024 08:07:47 +0800 Subject: [PATCH 1/2] [clang-tidy] add new check readability-enum-initial-value Fixes: #85243. --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/EnumInitialValueCheck.cpp | 82 +++ .../readability/EnumInitialValueCheck.h | 31 +++ .../readability/ReadabilityTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../checks/readability/enum-initial-value.rst | 45 ++ .../checkers/readability/enum-initial-value.c | 27 ++ .../readability/enum-initial-value.cpp| 27 ++ 9 files changed, 223 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/enum-initial-value.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5728c9970fb65d..dd772d69202548 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -17,6 +17,7 @@ add_clang_library(clangTidyReadabilityModule DeleteNullPointerCheck.cpp DuplicateIncludeCheck.cpp ElseAfterReturnCheck.cpp + EnumInitialValueCheck.cpp FunctionCognitiveComplexityCheck.cpp FunctionSizeCheck.cpp IdentifierLengthCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp new file mode 100644 index 00..78d5101d439dde --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp @@ -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( + .bind("enum"), + this); +} + +void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Enum = Result.Nodes.getNodeAs("enum"); + assert(Enum != nullptr); + SourceLocation Loc = Enum->getBeginLoc(); + if (Loc.isInvalid() || Loc.isMacroID()) +return; + DiagnosticBuilder Diag = + diag(Loc, "inital value in enum %0 has readability issue, " +"explicit initialization of all of enumerators") + << Enum->getName(); + for (EnumConstantDecl const *ECD : Enum->enumerators()) +if (ECD->getInitExpr() == nullptr) { + SourceLocation ECDLoc = ECD->getEndLoc(); + if (ECDLoc.isInvalid() || ECDLoc.isMacroID()) +continue; + std::optional Next = utils::lexer::findNextTokenSkippingComments( + ECDLoc, *Result.SourceManager, getLangOpts()); + if (!Next.has_value() || Next->getLocation().isMacroID()) +continue; + llvm::SmallString<8> Str{" = "}
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/PiotrZSL edited 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -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( + .bind("enum"), + this); +} + +void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Enum = Result.Nodes.getNodeAs("enum"); + assert(Enum != nullptr); + SourceLocation Loc = Enum->getBeginLoc(); + if (Loc.isInvalid() || Loc.isMacroID()) +return; + DiagnosticBuilder Diag = + diag(Loc, "inital value in enum %0 has readability issue, " PiotrZSL wrote: no need to mention "readability issue", better would be to say that Enum initialization is not consistent , and "initialize first, all or none" 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -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; +} PiotrZSL wrote: consider refactoring this to avoid comparing Node.enumerator_begin() constantly, maybe some local bool. Or merge all 3 matchers, and just take an 3 bools as arguments to figure what too check. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,31 @@ +//===--- EnumInitialValueCheck.h - clang-tidy ---*- C++ -*-===// +// +// 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 +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_ENUMINITIALVALUECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_ENUMINITIALVALUECHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::readability { + +/// Detects explicit initialization of a part of enumerators in an enumeration, +/// and relying on compiler to initialize the others. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability/enum-initial-value.html +class EnumInitialValueCheck : public ClangTidyCheck { +public: + EnumInitialValueCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; PiotrZSL wrote: Exclude implicit code here via TK_IgnoreUnlessSpelledInSource. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/PiotrZSL commented: Few things to consider. Overall looks +- fine. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,27 @@ +// RUN: %check_clang_tidy %s readability-enum-initial-value %t + +enum class EError { + // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: inital value in enum EError has readability issue, explicit initialization of all of enumerators + EError_a = 1, + EError_b, + // CHECK-FIXES: EError_b = 2, + EError_c = 3, +}; + +enum class ENone { + ENone_a, + ENone_b, + eENone_c, +}; + +enum class EFirst { + EFirst_a = 1, + EFirst_b, + EFirst_c, +}; + +enum class EAll { + EAll_a = 1, + EAll_b = 2, + EAll_c = 3, +}; PiotrZSL wrote: add some tests with macros, like where name of enum constant is behind macro. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,46 @@ +.. title:: clang-tidy - readability-enum-initial-value + +readability-enum-initial-value +== + +Detects explicit initialization of a part of enumerators in an enumeration, and +relying on compiler to initialize the others. + +It causes readability issues and reduces the maintainability. When adding new PiotrZSL wrote: no need for " readability issues and reduces the maintainability", just say that this is about consistency. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -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( + .bind("enum"), + this); +} + +void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Enum = Result.Nodes.getNodeAs("enum"); + assert(Enum != nullptr); + SourceLocation Loc = Enum->getBeginLoc(); + if (Loc.isInvalid() || Loc.isMacroID()) +return; + DiagnosticBuilder Diag = + diag(Loc, "inital value in enum %0 has readability issue, " +"explicit initialization of all of enumerators") + << Enum->getName(); + for (EnumConstantDecl const *ECD : Enum->enumerators()) +if (ECD->getInitExpr() == nullptr) { + SourceLocation ECDLoc = ECD->getEndLoc(); + if (ECDLoc.isInvalid() || ECDLoc.isMacroID()) +continue; + std::optional Next = utils::lexer::findNextTokenSkippingComments( PiotrZSL wrote: why not just find end location for enum constant and insert there regardless of comments (that could be multiline) 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -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( PiotrZSL wrote: would be nice to have an config for those, so i could set if none is allowed or not. Verify also if there is a check for situation if all enums are initialized but with consecutive values, in such case those inits could be removed and only first could be left. You may also add option to not allow first enumerator when it's initialized with 0 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -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( HerrCai0907 wrote: I will add some options to make it possible to let it strict. But I don't find the code guideline about "all enums are initialized but with consecutive values, in such case those inits could be removed and only first could be left". Inferring the usage of enum is hard and impossible, I think adding this check will create lots of false positive. For example ```c++ enum A { a = 0, b = 1, c = 2, } ``` can also be a bitmap. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/paperchalice updated https://github.com/llvm/llvm-project/pull/86129 >From 4e0845a143a820d4a68ffbdced206654c7593359 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Fri, 15 Mar 2024 08:07:47 +0800 Subject: [PATCH 1/2] [clang-tidy] add new check readability-enum-initial-value Fixes: #85243. --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/EnumInitialValueCheck.cpp | 82 +++ .../readability/EnumInitialValueCheck.h | 31 +++ .../readability/ReadabilityTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../checks/readability/enum-initial-value.rst | 45 ++ .../checkers/readability/enum-initial-value.c | 27 ++ .../readability/enum-initial-value.cpp| 27 ++ 9 files changed, 223 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/enum-initial-value.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5728c9970fb65d..dd772d69202548 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -17,6 +17,7 @@ add_clang_library(clangTidyReadabilityModule DeleteNullPointerCheck.cpp DuplicateIncludeCheck.cpp ElseAfterReturnCheck.cpp + EnumInitialValueCheck.cpp FunctionCognitiveComplexityCheck.cpp FunctionSizeCheck.cpp IdentifierLengthCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp new file mode 100644 index 00..78d5101d439dde --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp @@ -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( + .bind("enum"), + this); +} + +void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Enum = Result.Nodes.getNodeAs("enum"); + assert(Enum != nullptr); + SourceLocation Loc = Enum->getBeginLoc(); + if (Loc.isInvalid() || Loc.isMacroID()) +return; + DiagnosticBuilder Diag = + diag(Loc, "inital value in enum %0 has readability issue, " +"explicit initialization of all of enumerators") + << Enum->getName(); + for (EnumConstantDecl const *ECD : Enum->enumerators()) +if (ECD->getInitExpr() == nullptr) { + SourceLocation ECDLoc = ECD->getEndLoc(); + if (ECDLoc.isInvalid() || ECDLoc.isMacroID()) +continue; + std::optional Next = utils::lexer::findNextTokenSkippingComments( + ECDLoc, *Result.SourceManager, getLangOpts()); + if (!Next.has_value() || Next->getLocation().isMacroID()) +continue; + llvm::SmallString<8> Str{" = "
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,27 @@ +// RUN: %check_clang_tidy %s readability-enum-initial-value %t + +enum EError { + // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: inital value in enum EError has readability issue, explicit initialization of all of enumerators + EError_a = 1, + EError_b, + // CHECK-FIXES: EError_b = 2, + EError_c = 3, +}; + +enum ENone { + ENone_a, + ENone_b, + eENone_c, 5chmidti wrote: The leading `e` was probably a typo, same in the cpp test file. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/5chmidti commented: Just nits and a comment regarding the linear, all initialized enum. and all other checks use west-const from what I can tell. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -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(Init); if (!Constant) return false; if (llvm::isa(Constant->getSubExpr())) return true; const auto *const NegativeValue = llvm::dyn_cast(Constant->getSubExpr()); if (!NegativeValue) return false; return llvm::isa(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. Example Enums 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, }; Findings when running clang-tidy over `clang/lib/Basic` with `-header-filter="-*"`: https://github.com/llvm/llvm-project/blob/2f6b1b4b30e3a719b1744baa4cd1e
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/5chmidti edited 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -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( + .bind("enum"), + this); +} + +void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Enum = Result.Nodes.getNodeAs("enum"); + assert(Enum != nullptr); + SourceLocation Loc = Enum->getBeginLoc(); + if (Loc.isInvalid() || Loc.isMacroID()) +return; + DiagnosticBuilder Diag = + diag(Loc, "inital value in enum %0 has readability issue, " +"explicit initialization of all of enumerators") + << Enum->getName(); 5chmidti wrote: No need for the `->getName()`. The name will also be `'quoted'` automatically. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/5chmidti edited 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -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( + .bind("enum"), + this); +} + +void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Enum = Result.Nodes.getNodeAs("enum"); + assert(Enum != nullptr); 5chmidti wrote: `Enum` can't be a `nullptr` (single matcher binding to the top-level matcher) 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -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( + .bind("enum"), + this); +} + +void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Enum = Result.Nodes.getNodeAs("enum"); + assert(Enum != nullptr); + SourceLocation Loc = Enum->getBeginLoc(); + if (Loc.isInvalid() || Loc.isMacroID()) +return; + DiagnosticBuilder Diag = + diag(Loc, "inital value in enum %0 has readability issue, " +"explicit initialization of all of enumerators") + << Enum->getName(); + for (EnumConstantDecl const *ECD : Enum->enumerators()) +if (ECD->getInitExpr() == nullptr) { + SourceLocation ECDLoc = ECD->getEndLoc(); + if (ECDLoc.isInvalid() || ECDLoc.isMacroID()) +continue; + std::optional Next = utils::lexer::findNextTokenSkippingComments( HerrCai0907 wrote: fix in the end location of enum constant will insert the string before enum constant. `a3,` will be changed to `= 3a3,`. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/86129 >From 4e0845a143a820d4a68ffbdced206654c7593359 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Fri, 15 Mar 2024 08:07:47 +0800 Subject: [PATCH 1/5] [clang-tidy] add new check readability-enum-initial-value Fixes: #85243. --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/EnumInitialValueCheck.cpp | 82 +++ .../readability/EnumInitialValueCheck.h | 31 +++ .../readability/ReadabilityTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../checks/readability/enum-initial-value.rst | 45 ++ .../checkers/readability/enum-initial-value.c | 27 ++ .../readability/enum-initial-value.cpp| 27 ++ 9 files changed, 223 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/enum-initial-value.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5728c9970fb65d..dd772d69202548 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -17,6 +17,7 @@ add_clang_library(clangTidyReadabilityModule DeleteNullPointerCheck.cpp DuplicateIncludeCheck.cpp ElseAfterReturnCheck.cpp + EnumInitialValueCheck.cpp FunctionCognitiveComplexityCheck.cpp FunctionSizeCheck.cpp IdentifierLengthCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp new file mode 100644 index 00..78d5101d439dde --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp @@ -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( + .bind("enum"), + this); +} + +void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Enum = Result.Nodes.getNodeAs("enum"); + assert(Enum != nullptr); + SourceLocation Loc = Enum->getBeginLoc(); + if (Loc.isInvalid() || Loc.isMacroID()) +return; + DiagnosticBuilder Diag = + diag(Loc, "inital value in enum %0 has readability issue, " +"explicit initialization of all of enumerators") + << Enum->getName(); + for (EnumConstantDecl const *ECD : Enum->enumerators()) +if (ECD->getInitExpr() == nullptr) { + SourceLocation ECDLoc = ECD->getEndLoc(); + if (ECDLoc.isInvalid() || ECDLoc.isMacroID()) +continue; + std::optional Next = utils::lexer::findNextTokenSkippingComments( + ECDLoc, *Result.SourceManager, getLangOpts()); + if (!Next.has_value() || Next->getLocation().isMacroID()) +continue; + llvm::SmallString<8> Str{" = "}
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -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( + .bind("enum"), + this); +} + +void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Enum = Result.Nodes.getNodeAs("enum"); + assert(Enum != nullptr); + SourceLocation Loc = Enum->getBeginLoc(); + if (Loc.isInvalid() || Loc.isMacroID()) +return; + DiagnosticBuilder Diag = + diag(Loc, "inital value in enum %0 has readability issue, " +"explicit initialization of all of enumerators") + << Enum->getName(); + for (EnumConstantDecl const *ECD : Enum->enumerators()) +if (ECD->getInitExpr() == nullptr) { + SourceLocation ECDLoc = ECD->getEndLoc(); + if (ECDLoc.isInvalid() || ECDLoc.isMacroID()) +continue; + std::optional Next = utils::lexer::findNextTokenSkippingComments( PiotrZSL wrote: this is because you just need to get location of end of token 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/PiotrZSL edited 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/PiotrZSL requested changes to this pull request. Overall looks fine. Some minor style issues. Try getting rid of findNextTokenSkippingComments, and maybe just jump to begin() + token width 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,193 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", true)), + AllowExplicitLinearInitialValues( + Options.get("AllowExplicitLinearInitialValues", true)) {} + +void EnumInitialValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowExplicitZeroFirstInitialValue", +AllowExplicitZeroFirstInitialValue); + Options.store(Opts, "AllowExplicitLinearInitialValues", +AllowExplicitLinearInitialValues); +} + +namespace { + +bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { PiotrZSL wrote: maybe `areEnumeratorsNotInitialized` for name ? 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,193 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", true)), + AllowExplicitLinearInitialValues( + Options.get("AllowExplicitLinearInitialValues", true)) {} + +void EnumInitialValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowExplicitZeroFirstInitialValue", +AllowExplicitZeroFirstInitialValue); + Options.store(Opts, "AllowExplicitLinearInitialValues", +AllowExplicitLinearInitialValues); +} + +namespace { + +bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { PiotrZSL wrote: for functions we should use ``static`` instead of anonymous namespace. For AST_MATCHER there should be anonymous namespace just because static is not possible. Also move this code to beginning of file, to not cut in between of class methods. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,193 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", true)), + AllowExplicitLinearInitialValues( + Options.get("AllowExplicitLinearInitialValues", true)) {} + +void EnumInitialValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowExplicitZeroFirstInitialValue", +AllowExplicitZeroFirstInitialValue); + Options.store(Opts, "AllowExplicitLinearInitialValues", +AllowExplicitLinearInitialValues); +} + +namespace { + +bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +bool isOnlyFirstEnumeratorsInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) +if (IsFirst) { + IsFirst = false; + if (ECD->getInitExpr() == nullptr) +return false; +} else { + if (ECD->getInitExpr() != nullptr) +return false; +} + return !IsFirst; +} + +bool isAllEnumeratorsInitialized(const EnumDecl &Node) { PiotrZSL wrote: isAllEnumeratorsInitialized -> areAllEnumeratorsInitialized 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,193 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", true)), + AllowExplicitLinearInitialValues( + Options.get("AllowExplicitLinearInitialValues", true)) {} + +void EnumInitialValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowExplicitZeroFirstInitialValue", +AllowExplicitZeroFirstInitialValue); + Options.store(Opts, "AllowExplicitLinearInitialValues", +AllowExplicitLinearInitialValues); +} + +namespace { + +bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +bool isOnlyFirstEnumeratorsInitialized(const EnumDecl &Node) { PiotrZSL wrote: Enumerators -> Enumerator 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,75 @@ +.. title:: clang-tidy - readability-enum-initial-value + +readability-enum-initial-value +== + +Detects explicit initialization of a part of enumerators in an enumeration, and +relying on compiler to initialize the others. + +When adding new enumerations, inconsistent initial value will cause potential +enumeration value conflicts. + +In an enumeration, the following three cases are accepted. +1. none of enumerators are explicit initialized. +2. the first enumerator is explicit initialized. +3. all of enumerators are explicit initialized. + +.. code-block:: c++ + + // valid, none of enumerators are initialized. + enum A { +e0, +e1, +e2, + }; + + // valid, the first enumerator is initialized. + enum A { +e0 = 0, +e1, +e2, + }; + + // valid, all of enumerators are initialized. + enum A { +e0 = 0, +e1 = 1, +e2 = 2, + }; + + // invalid, e1 is not explicit initialized. + enum A { +e0 = 0, +e1, +e2 = 2, + }; + +Options +--- + +.. option:: AllowExplicitZeroFirstInitialValue + + If set to `false`, explicit initialized first enumerator is not allowed. PiotrZSL wrote: ```suggestion If set to `false`, the first enumerator must not be explicitly initialized. ``` 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,193 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", true)), + AllowExplicitLinearInitialValues( + Options.get("AllowExplicitLinearInitialValues", true)) {} + +void EnumInitialValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowExplicitZeroFirstInitialValue", +AllowExplicitZeroFirstInitialValue); + Options.store(Opts, "AllowExplicitLinearInitialValues", +AllowExplicitLinearInitialValues); +} + +namespace { + +bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +bool isOnlyFirstEnumeratorsInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) +if (IsFirst) { + IsFirst = false; + if (ECD->getInitExpr() == nullptr) +return false; +} else { + if (ECD->getInitExpr() != nullptr) +return false; +} + return !IsFirst; +} + +bool isAllEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() != nullptr; + }); +} + +/// 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; + return Init->isIntegerConstantExpr(Enumerator->getASTContext()); +} + +AST_MATCHER(EnumDecl, hasConsistentInitialValues) { + return isNoneEnumeratorsInitialized(Node) || + isOnlyFirstEnumeratorsInitialized(Node) || + isAllEnumeratorsInitialized(Node); +} + +AST_MATCHER(EnumDecl, hasZeroFirstInitialValue) { + if (Node.enumerators().empty()) +return false; + const EnumConstantDecl *ECD = *Node.enumerators().begin(); + return isOnlyFirstEnumeratorsInitialized(Node) && + isInitializedByLiteral(ECD) && ECD->getInitVal().isZero(); +} + +/// 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 +/// bitmask, evident when enumerators are only initialized with (potentially +/// negative) integer literals, are ignored. This is also the case when all +/// enumerators are powers of two (e.g., 0, 1, 2). +AST_MATCHER(EnumDecl, hasLinearInitialValues) { + if (Node.enumerators().empty()) +return false; + const EnumConstantDecl *const FirstEnumerator = *Node.enumerator_begin(); PiotrZSL wrote: `enumerator_begin -> enumerators.begin()`, be consistent, even better do: `auto Enumerators = Node.enumerators()` 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,75 @@ +.. title:: clang-tidy - readability-enum-initial-value + +readability-enum-initial-value +== + +Detects explicit initialization of a part of enumerators in an enumeration, and +relying on compiler to initialize the others. + +When adding new enumerations, inconsistent initial value will cause potential +enumeration value conflicts. + +In an enumeration, the following three cases are accepted. +1. none of enumerators are explicit initialized. +2. the first enumerator is explicit initialized. +3. all of enumerators are explicit initialized. + +.. code-block:: c++ + + // valid, none of enumerators are initialized. + enum A { +e0, +e1, +e2, + }; + + // valid, the first enumerator is initialized. + enum A { +e0 = 0, +e1, +e2, + }; + + // valid, all of enumerators are initialized. + enum A { +e0 = 0, +e1 = 1, +e2 = 2, + }; + + // invalid, e1 is not explicit initialized. + enum A { +e0 = 0, +e1, +e2 = 2, + }; + +Options +--- + +.. option:: AllowExplicitZeroFirstInitialValue + + If set to `false`, explicit initialized first enumerator is not allowed. + See examples below. Default is `true`. + + .. code-block:: c++ + +enum A { + e0 = 0, // not allowed if AllowExplicitZeroFirstInitialValue is false + e1, + e2, +}; + + +.. option:: AllowExplicitLinearInitialValues PiotrZSL wrote: Consider using `sequential` instead of `linear`. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,193 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", true)), + AllowExplicitLinearInitialValues( + Options.get("AllowExplicitLinearInitialValues", true)) {} + +void EnumInitialValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowExplicitZeroFirstInitialValue", +AllowExplicitZeroFirstInitialValue); + Options.store(Opts, "AllowExplicitLinearInitialValues", +AllowExplicitLinearInitialValues); +} + +namespace { + +bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +bool isOnlyFirstEnumeratorsInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) +if (IsFirst) { + IsFirst = false; + if (ECD->getInitExpr() == nullptr) +return false; +} else { + if (ECD->getInitExpr() != nullptr) +return false; +} + return !IsFirst; +} + +bool isAllEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() != nullptr; + }); +} + +/// 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; + return Init->isIntegerConstantExpr(Enumerator->getASTContext()); +} + +AST_MATCHER(EnumDecl, hasConsistentInitialValues) { + return isNoneEnumeratorsInitialized(Node) || + isOnlyFirstEnumeratorsInitialized(Node) || + isAllEnumeratorsInitialized(Node); +} + +AST_MATCHER(EnumDecl, hasZeroFirstInitialValue) { PiotrZSL wrote: maybe hasZeroInitialValueForFirstEnumerator 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,75 @@ +.. title:: clang-tidy - readability-enum-initial-value + +readability-enum-initial-value +== + +Detects explicit initialization of a part of enumerators in an enumeration, and +relying on compiler to initialize the others. PiotrZSL wrote: Maybe: ``` Enforces consistent style for enumerators' initialization, covering three styles: none, first only, or all initialized explicitly. ``` 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,193 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", true)), + AllowExplicitLinearInitialValues( + Options.get("AllowExplicitLinearInitialValues", true)) {} + +void EnumInitialValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowExplicitZeroFirstInitialValue", +AllowExplicitZeroFirstInitialValue); + Options.store(Opts, "AllowExplicitLinearInitialValues", +AllowExplicitLinearInitialValues); +} + +namespace { + +bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +bool isOnlyFirstEnumeratorsInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) +if (IsFirst) { + IsFirst = false; + if (ECD->getInitExpr() == nullptr) +return false; +} else { + if (ECD->getInitExpr() != nullptr) +return false; +} + return !IsFirst; +} + +bool isAllEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() != nullptr; + }); +} + +/// 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; + return Init->isIntegerConstantExpr(Enumerator->getASTContext()); +} + +AST_MATCHER(EnumDecl, hasConsistentInitialValues) { + return isNoneEnumeratorsInitialized(Node) || + isOnlyFirstEnumeratorsInitialized(Node) || + isAllEnumeratorsInitialized(Node); +} + +AST_MATCHER(EnumDecl, hasZeroFirstInitialValue) { + if (Node.enumerators().empty()) +return false; + const EnumConstantDecl *ECD = *Node.enumerators().begin(); + return isOnlyFirstEnumeratorsInitialized(Node) && + isInitializedByLiteral(ECD) && ECD->getInitVal().isZero(); +} + +/// 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 +/// bitmask, evident when enumerators are only initialized with (potentially +/// negative) integer literals, are ignored. This is also the case when all +/// enumerators are powers of two (e.g., 0, 1, 2). +AST_MATCHER(EnumDecl, hasLinearInitialValues) { + if (Node.enumerators().empty()) +return false; + const EnumConstantDecl *const FirstEnumerator = *Node.enumerator_begin(); + llvm::APSInt PrevValue = FirstEnumerator->getInitVal(); + if (!isInitializedByLiteral(FirstEnumerator)) +return false; + bool AllEnumeratorsArePowersOfTwo = true; + for (const EnumConstantDecl *Enumerator : + llvm::drop_begin(Node.enumerators())) { +const llvm::APSInt NewValue = Enumerator->getInitVal(); +if (NewValue != ++PrevValue) + return false; +if (!isInitializedByLiteral(Enumerator)) + return false; +PrevValue = NewValue; +AllEnumeratorsArePowersOfTwo &= NewValue.isPowerOf2(); + } + return !AllEnumeratorsArePowersOfTwo; +} + +} // namespace + +void EnumInitialValueCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + enumDecl(unless(hasConsistentInitialValues())).bind("inconsistent"), + this); + if (!AllowExplicitZeroFirstInitialValue) +Finder->addMatcher(enumDecl(hasZeroFirstInitialValue()).bind("zero_first"), + this); + if (!AllowExplicitLinearInitialValues) +Finder->addMatcher(enumDecl(hasLinearInitialValues()).bind("linear"), this); +} + +static void cleanInitialValue(DiagnosticBuilder &Diag, + const EnumConstantDecl *ECD, + c
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,193 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", true)), + AllowExplicitLinearInitialValues( + Options.get("AllowExplicitLinearInitialValues", true)) {} + +void EnumInitialValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowExplicitZeroFirstInitialValue", +AllowExplicitZeroFirstInitialValue); + Options.store(Opts, "AllowExplicitLinearInitialValues", +AllowExplicitLinearInitialValues); +} + +namespace { + +bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +bool isOnlyFirstEnumeratorsInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) +if (IsFirst) { + IsFirst = false; + if (ECD->getInitExpr() == nullptr) +return false; +} else { + if (ECD->getInitExpr() != nullptr) +return false; +} + return !IsFirst; +} + +bool isAllEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() != nullptr; + }); +} + +/// 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; + return Init->isIntegerConstantExpr(Enumerator->getASTContext()); +} + +AST_MATCHER(EnumDecl, hasConsistentInitialValues) { + return isNoneEnumeratorsInitialized(Node) || + isOnlyFirstEnumeratorsInitialized(Node) || + isAllEnumeratorsInitialized(Node); +} + +AST_MATCHER(EnumDecl, hasZeroFirstInitialValue) { + if (Node.enumerators().empty()) +return false; + const EnumConstantDecl *ECD = *Node.enumerators().begin(); + return isOnlyFirstEnumeratorsInitialized(Node) && + isInitializedByLiteral(ECD) && ECD->getInitVal().isZero(); +} + +/// 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 +/// bitmask, evident when enumerators are only initialized with (potentially +/// negative) integer literals, are ignored. This is also the case when all +/// enumerators are powers of two (e.g., 0, 1, 2). +AST_MATCHER(EnumDecl, hasLinearInitialValues) { + if (Node.enumerators().empty()) +return false; + const EnumConstantDecl *const FirstEnumerator = *Node.enumerator_begin(); + llvm::APSInt PrevValue = FirstEnumerator->getInitVal(); + if (!isInitializedByLiteral(FirstEnumerator)) +return false; + bool AllEnumeratorsArePowersOfTwo = true; + for (const EnumConstantDecl *Enumerator : + llvm::drop_begin(Node.enumerators())) { +const llvm::APSInt NewValue = Enumerator->getInitVal(); +if (NewValue != ++PrevValue) + return false; +if (!isInitializedByLiteral(Enumerator)) + return false; +PrevValue = NewValue; +AllEnumeratorsArePowersOfTwo &= NewValue.isPowerOf2(); + } + return !AllEnumeratorsArePowersOfTwo; +} + +} // namespace + +void EnumInitialValueCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + enumDecl(unless(hasConsistentInitialValues())).bind("inconsistent"), + this); + if (!AllowExplicitZeroFirstInitialValue) +Finder->addMatcher(enumDecl(hasZeroFirstInitialValue()).bind("zero_first"), + this); + if (!AllowExplicitLinearInitialValues) +Finder->addMatcher(enumDecl(hasLinearInitialValues()).bind("linear"), this); +} + +static void cleanInitialValue(DiagnosticBuilder &Diag, + const EnumConstantDecl *ECD, + c
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,193 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", true)), + AllowExplicitLinearInitialValues( + Options.get("AllowExplicitLinearInitialValues", true)) {} + +void EnumInitialValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowExplicitZeroFirstInitialValue", +AllowExplicitZeroFirstInitialValue); + Options.store(Opts, "AllowExplicitLinearInitialValues", +AllowExplicitLinearInitialValues); +} + +namespace { + +bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +bool isOnlyFirstEnumeratorsInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) +if (IsFirst) { + IsFirst = false; + if (ECD->getInitExpr() == nullptr) +return false; +} else { + if (ECD->getInitExpr() != nullptr) +return false; +} PiotrZSL wrote: Looks like: ``` if (IsFirst == (ECD->getInitExpr() == nullptr)) return false; IsFirst = false; ``` 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,193 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", true)), + AllowExplicitLinearInitialValues( + Options.get("AllowExplicitLinearInitialValues", true)) {} + +void EnumInitialValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowExplicitZeroFirstInitialValue", +AllowExplicitZeroFirstInitialValue); + Options.store(Opts, "AllowExplicitLinearInitialValues", +AllowExplicitLinearInitialValues); +} + +namespace { + +bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +bool isOnlyFirstEnumeratorsInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) +if (IsFirst) { + IsFirst = false; + if (ECD->getInitExpr() == nullptr) +return false; +} else { + if (ECD->getInitExpr() != nullptr) +return false; +} HerrCai0907 wrote: I think simplify it to `if ((IsFirst && ECD->getInitExpr() == nullptr) || (!IsFirst && ECD->getInitExpr() != nullptr))` is enough. `if (IsFirst == (ECD->getInitExpr() == nullptr))` make code hard to understand. And after optimization, the result should be the same. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/86129 >From 4e0845a143a820d4a68ffbdced206654c7593359 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Fri, 15 Mar 2024 08:07:47 +0800 Subject: [PATCH 1/6] [clang-tidy] add new check readability-enum-initial-value Fixes: #85243. --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/EnumInitialValueCheck.cpp | 82 +++ .../readability/EnumInitialValueCheck.h | 31 +++ .../readability/ReadabilityTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../checks/readability/enum-initial-value.rst | 45 ++ .../checkers/readability/enum-initial-value.c | 27 ++ .../readability/enum-initial-value.cpp| 27 ++ 9 files changed, 223 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/enum-initial-value.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5728c9970fb65d..dd772d69202548 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -17,6 +17,7 @@ add_clang_library(clangTidyReadabilityModule DeleteNullPointerCheck.cpp DuplicateIncludeCheck.cpp ElseAfterReturnCheck.cpp + EnumInitialValueCheck.cpp FunctionCognitiveComplexityCheck.cpp FunctionSizeCheck.cpp IdentifierLengthCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp new file mode 100644 index 00..78d5101d439dde --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp @@ -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( + .bind("enum"), + this); +} + +void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Enum = Result.Nodes.getNodeAs("enum"); + assert(Enum != nullptr); + SourceLocation Loc = Enum->getBeginLoc(); + if (Loc.isInvalid() || Loc.isMacroID()) +return; + DiagnosticBuilder Diag = + diag(Loc, "inital value in enum %0 has readability issue, " +"explicit initialization of all of enumerators") + << Enum->getName(); + for (EnumConstantDecl const *ECD : Enum->enumerators()) +if (ECD->getInitExpr() == nullptr) { + SourceLocation ECDLoc = ECD->getEndLoc(); + if (ECDLoc.isInvalid() || ECDLoc.isMacroID()) +continue; + std::optional Next = utils::lexer::findNextTokenSkippingComments( + ECDLoc, *Result.SourceManager, getLangOpts()); + if (!Next.has_value() || Next->getLocation().isMacroID()) +continue; + llvm::SmallString<8> Str{" = "}
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
HerrCai0907 wrote: > Try getting rid of findNextTokenSkippingComments, and maybe just jump to > begin() + token width Do you think it is a good idea to use `ECD->getLocation().getLocWithOffset(ECD->getName().size())`? 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
HerrCai0907 wrote: > > Try getting rid of findNextTokenSkippingComments, and maybe just jump to > > begin() + token width > > Do you think it is a good idea to use > `ECD->getLocation().getLocWithOffset(ECD->getName().size())`? It doesn't work for macro. I think the most precise way is to fix it is using `findNextTokenSkippingComments`. Why should we avoid to use `findNextTokenSkippingComments`? Is it for performance reasons? 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +static bool isOnlyFirstEnumeratorInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) { +if ((IsFirst && ECD->getInitExpr() == nullptr) || +(!IsFirst && ECD->getInitExpr() != nullptr)) + return false; +IsFirst = false; + } + return !IsFirst; +} + +static bool areAllEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() != nullptr; + }); +} + +/// Check if \p Enumerator is initialized with a (potentially negated) \c +/// IntegerLiteral. +static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) { + const Expr *const Init = Enumerator->getInitExpr(); + if (!Init) +return false; + return Init->isIntegerConstantExpr(Enumerator->getASTContext()); +} + +static void cleanInitialValue(DiagnosticBuilder &Diag, + const EnumConstantDecl *ECD, + const SourceManager &SM, + const LangOptions &LangOpts) { + std::optional EqualToken = utils::lexer::findNextTokenSkippingComments( + ECD->getLocation(), SM, LangOpts); + if (!EqualToken.has_value()) +return; + SourceLocation EqualLoc{EqualToken->getLocation()}; + if (EqualLoc.isInvalid() || EqualLoc.isMacroID()) +return; + SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange(); + if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() || + InitExprRange.getEnd().isMacroID()) +return; + Diag << FixItHint::CreateRemoval(EqualLoc) + << FixItHint::CreateRemoval(InitExprRange); + return; +} + +namespace { + +AST_MATCHER(EnumDecl, isMacro) { + SourceLocation Loc = Node.getBeginLoc(); + return Loc.isMacroID(); +} + +AST_MATCHER(EnumDecl, hasConsistentInitialValues) { + return isNoneEnumeratorsInitialized(Node) || + isOnlyFirstEnumeratorInitialized(Node) || + areAllEnumeratorsInitialized(Node); +} + +AST_MATCHER(EnumDecl, hasZeroInitialValueForFirstEnumerator) { + EnumDecl::enumerator_range Enumerators = Node.enumerators(); + if (Enumerators.empty()) +return false; + const EnumConstantDecl *ECD = *Enumerators.begin(); + return isOnlyFirstEnumeratorInitialized(Node) && + isInitializedByLiteral(ECD) && ECD->getInitVal().isZero(); +} + +/// 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 +/// bitmask, evident when enumerators are only initialized with (potentially +/// negative) integer literals, are ignored. This is also the case when all +/// enumerators are powers of two (e.g., 0, 1, 2). +AST_MATCHER(EnumDecl, hasSequentialInitialValues) { + EnumDecl::enumerator_range Enumerators = Node.enumerators(); + if (Enumerators.empty()) +return false; + const EnumConstantDecl *const FirstEnumerator = *Node.enumerator_begin(); + llvm::APSInt PrevValue = FirstEnumerator->getInitVal(); + if (!isInitializedByLiteral(FirstEnumerator)) +return false; + bool AllEnumeratorsArePowersOfTwo = true; + for (const EnumConstantDecl *Enumerator : llvm::drop_begin(Enumerators)) { +const llvm::APSInt NewValue = Enumerator->getInitVal(); +if (NewValue != ++PrevValue) + return false; +if (!isInitializedByLiteral(Enumerator)) + return false; +PrevValue = NewValue; +AllEnumeratorsArePowersOfTwo &= NewValue.isPowerOf2(); + } + return !AllEnumeratorsArePowersOfTwo; +} + +} // namespace + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", tru
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/5chmidti edited 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/5chmidti commented: I only have some minor comments, otherwise looks good to me 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +static bool isOnlyFirstEnumeratorInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) { +if ((IsFirst && ECD->getInitExpr() == nullptr) || +(!IsFirst && ECD->getInitExpr() != nullptr)) + return false; +IsFirst = false; + } + return !IsFirst; +} + +static bool areAllEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() != nullptr; + }); +} + +/// Check if \p Enumerator is initialized with a (potentially negated) \c +/// IntegerLiteral. +static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) { + const Expr *const Init = Enumerator->getInitExpr(); + if (!Init) +return false; + return Init->isIntegerConstantExpr(Enumerator->getASTContext()); +} + +static void cleanInitialValue(DiagnosticBuilder &Diag, + const EnumConstantDecl *ECD, + const SourceManager &SM, + const LangOptions &LangOpts) { + std::optional EqualToken = utils::lexer::findNextTokenSkippingComments( + ECD->getLocation(), SM, LangOpts); + if (!EqualToken.has_value()) +return; + SourceLocation EqualLoc{EqualToken->getLocation()}; + if (EqualLoc.isInvalid() || EqualLoc.isMacroID()) +return; + SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange(); + if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() || + InitExprRange.getEnd().isMacroID()) +return; + Diag << FixItHint::CreateRemoval(EqualLoc) + << FixItHint::CreateRemoval(InitExprRange); + return; +} + +namespace { + +AST_MATCHER(EnumDecl, isMacro) { + SourceLocation Loc = Node.getBeginLoc(); + return Loc.isMacroID(); +} + +AST_MATCHER(EnumDecl, hasConsistentInitialValues) { + return isNoneEnumeratorsInitialized(Node) || + isOnlyFirstEnumeratorInitialized(Node) || + areAllEnumeratorsInitialized(Node); +} + +AST_MATCHER(EnumDecl, hasZeroInitialValueForFirstEnumerator) { + EnumDecl::enumerator_range Enumerators = Node.enumerators(); 5chmidti wrote: Please add `const` 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +static bool isOnlyFirstEnumeratorInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) { +if ((IsFirst && ECD->getInitExpr() == nullptr) || +(!IsFirst && ECD->getInitExpr() != nullptr)) + return false; +IsFirst = false; + } + return !IsFirst; +} + +static bool areAllEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() != nullptr; + }); +} + +/// Check if \p Enumerator is initialized with a (potentially negated) \c +/// IntegerLiteral. +static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) { + const Expr *const Init = Enumerator->getInitExpr(); + if (!Init) +return false; + return Init->isIntegerConstantExpr(Enumerator->getASTContext()); +} + +static void cleanInitialValue(DiagnosticBuilder &Diag, + const EnumConstantDecl *ECD, + const SourceManager &SM, + const LangOptions &LangOpts) { + std::optional EqualToken = utils::lexer::findNextTokenSkippingComments( + ECD->getLocation(), SM, LangOpts); + if (!EqualToken.has_value()) +return; + SourceLocation EqualLoc{EqualToken->getLocation()}; + if (EqualLoc.isInvalid() || EqualLoc.isMacroID()) +return; + SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange(); 5chmidti wrote: 2x `const SourceRange` 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +static bool isOnlyFirstEnumeratorInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) { +if ((IsFirst && ECD->getInitExpr() == nullptr) || +(!IsFirst && ECD->getInitExpr() != nullptr)) + return false; +IsFirst = false; + } + return !IsFirst; +} + +static bool areAllEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() != nullptr; + }); +} + +/// Check if \p Enumerator is initialized with a (potentially negated) \c +/// IntegerLiteral. +static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) { + const Expr *const Init = Enumerator->getInitExpr(); + if (!Init) +return false; + return Init->isIntegerConstantExpr(Enumerator->getASTContext()); +} + +static void cleanInitialValue(DiagnosticBuilder &Diag, + const EnumConstantDecl *ECD, + const SourceManager &SM, + const LangOptions &LangOpts) { + std::optional EqualToken = utils::lexer::findNextTokenSkippingComments( + ECD->getLocation(), SM, LangOpts); + if (!EqualToken.has_value()) +return; + SourceLocation EqualLoc{EqualToken->getLocation()}; + if (EqualLoc.isInvalid() || EqualLoc.isMacroID()) +return; + SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange(); + if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() || + InitExprRange.getEnd().isMacroID()) +return; 5chmidti wrote: This check can be done before the call to `findNextTokenSkippingComments`, which is cheaper than going through the lexer first. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +static bool isOnlyFirstEnumeratorInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) { +if ((IsFirst && ECD->getInitExpr() == nullptr) || +(!IsFirst && ECD->getInitExpr() != nullptr)) + return false; +IsFirst = false; + } + return !IsFirst; +} + +static bool areAllEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() != nullptr; + }); +} + +/// Check if \p Enumerator is initialized with a (potentially negated) \c +/// IntegerLiteral. +static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) { + const Expr *const Init = Enumerator->getInitExpr(); + if (!Init) +return false; + return Init->isIntegerConstantExpr(Enumerator->getASTContext()); +} + +static void cleanInitialValue(DiagnosticBuilder &Diag, + const EnumConstantDecl *ECD, + const SourceManager &SM, + const LangOptions &LangOpts) { + std::optional EqualToken = utils::lexer::findNextTokenSkippingComments( + ECD->getLocation(), SM, LangOpts); + if (!EqualToken.has_value()) +return; + SourceLocation EqualLoc{EqualToken->getLocation()}; + if (EqualLoc.isInvalid() || EqualLoc.isMacroID()) +return; + SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange(); + if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() || + InitExprRange.getEnd().isMacroID()) +return; + Diag << FixItHint::CreateRemoval(EqualLoc) + << FixItHint::CreateRemoval(InitExprRange); + return; +} + +namespace { + +AST_MATCHER(EnumDecl, isMacro) { + SourceLocation Loc = Node.getBeginLoc(); + return Loc.isMacroID(); +} + +AST_MATCHER(EnumDecl, hasConsistentInitialValues) { + return isNoneEnumeratorsInitialized(Node) || + isOnlyFirstEnumeratorInitialized(Node) || + areAllEnumeratorsInitialized(Node); +} + +AST_MATCHER(EnumDecl, hasZeroInitialValueForFirstEnumerator) { + EnumDecl::enumerator_range Enumerators = Node.enumerators(); + if (Enumerators.empty()) +return false; + const EnumConstantDecl *ECD = *Enumerators.begin(); + return isOnlyFirstEnumeratorInitialized(Node) && + isInitializedByLiteral(ECD) && ECD->getInitVal().isZero(); +} + +/// 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 +/// bitmask, evident when enumerators are only initialized with (potentially +/// negative) integer literals, are ignored. This is also the case when all +/// enumerators are powers of two (e.g., 0, 1, 2). +AST_MATCHER(EnumDecl, hasSequentialInitialValues) { + EnumDecl::enumerator_range Enumerators = Node.enumerators(); + if (Enumerators.empty()) +return false; + const EnumConstantDecl *const FirstEnumerator = *Node.enumerator_begin(); + llvm::APSInt PrevValue = FirstEnumerator->getInitVal(); + if (!isInitializedByLiteral(FirstEnumerator)) +return false; + bool AllEnumeratorsArePowersOfTwo = true; + for (const EnumConstantDecl *Enumerator : llvm::drop_begin(Enumerators)) { +const llvm::APSInt NewValue = Enumerator->getInitVal(); +if (NewValue != ++PrevValue) + return false; +if (!isInitializedByLiteral(Enumerator)) + return false; +PrevValue = NewValue; +AllEnumeratorsArePowersOfTwo &= NewValue.isPowerOf2(); + } + return !AllEnumeratorsArePowersOfTwo; +} + +} // namespace + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", tru
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +static bool isOnlyFirstEnumeratorInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) { +if ((IsFirst && ECD->getInitExpr() == nullptr) || +(!IsFirst && ECD->getInitExpr() != nullptr)) + return false; +IsFirst = false; + } + return !IsFirst; +} + +static bool areAllEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() != nullptr; + }); +} + +/// Check if \p Enumerator is initialized with a (potentially negated) \c +/// IntegerLiteral. +static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) { + const Expr *const Init = Enumerator->getInitExpr(); + if (!Init) +return false; + return Init->isIntegerConstantExpr(Enumerator->getASTContext()); +} + +static void cleanInitialValue(DiagnosticBuilder &Diag, + const EnumConstantDecl *ECD, + const SourceManager &SM, + const LangOptions &LangOpts) { + std::optional EqualToken = utils::lexer::findNextTokenSkippingComments( + ECD->getLocation(), SM, LangOpts); + if (!EqualToken.has_value()) +return; + SourceLocation EqualLoc{EqualToken->getLocation()}; + if (EqualLoc.isInvalid() || EqualLoc.isMacroID()) +return; + SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange(); + if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() || + InitExprRange.getEnd().isMacroID()) +return; + Diag << FixItHint::CreateRemoval(EqualLoc) + << FixItHint::CreateRemoval(InitExprRange); + return; +} + +namespace { + +AST_MATCHER(EnumDecl, isMacro) { + SourceLocation Loc = Node.getBeginLoc(); + return Loc.isMacroID(); +} + +AST_MATCHER(EnumDecl, hasConsistentInitialValues) { + return isNoneEnumeratorsInitialized(Node) || + isOnlyFirstEnumeratorInitialized(Node) || + areAllEnumeratorsInitialized(Node); +} + +AST_MATCHER(EnumDecl, hasZeroInitialValueForFirstEnumerator) { + EnumDecl::enumerator_range Enumerators = Node.enumerators(); + if (Enumerators.empty()) +return false; + const EnumConstantDecl *ECD = *Enumerators.begin(); + return isOnlyFirstEnumeratorInitialized(Node) && + isInitializedByLiteral(ECD) && ECD->getInitVal().isZero(); +} + +/// 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 +/// bitmask, evident when enumerators are only initialized with (potentially +/// negative) integer literals, are ignored. This is also the case when all +/// enumerators are powers of two (e.g., 0, 1, 2). +AST_MATCHER(EnumDecl, hasSequentialInitialValues) { + EnumDecl::enumerator_range Enumerators = Node.enumerators(); + if (Enumerators.empty()) +return false; + const EnumConstantDecl *const FirstEnumerator = *Node.enumerator_begin(); + llvm::APSInt PrevValue = FirstEnumerator->getInitVal(); + if (!isInitializedByLiteral(FirstEnumerator)) +return false; + bool AllEnumeratorsArePowersOfTwo = true; + for (const EnumConstantDecl *Enumerator : llvm::drop_begin(Enumerators)) { +const llvm::APSInt NewValue = Enumerator->getInitVal(); +if (NewValue != ++PrevValue) + return false; +if (!isInitializedByLiteral(Enumerator)) + return false; +PrevValue = NewValue; +AllEnumeratorsArePowersOfTwo &= NewValue.isPowerOf2(); + } + return !AllEnumeratorsArePowersOfTwo; +} + +} // namespace + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", tru
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +static bool isOnlyFirstEnumeratorInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) { +if ((IsFirst && ECD->getInitExpr() == nullptr) || +(!IsFirst && ECD->getInitExpr() != nullptr)) + return false; +IsFirst = false; + } + return !IsFirst; +} + +static bool areAllEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() != nullptr; + }); +} + +/// Check if \p Enumerator is initialized with a (potentially negated) \c +/// IntegerLiteral. +static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) { + const Expr *const Init = Enumerator->getInitExpr(); + if (!Init) +return false; + return Init->isIntegerConstantExpr(Enumerator->getASTContext()); +} + +static void cleanInitialValue(DiagnosticBuilder &Diag, + const EnumConstantDecl *ECD, + const SourceManager &SM, + const LangOptions &LangOpts) { + std::optional EqualToken = utils::lexer::findNextTokenSkippingComments( + ECD->getLocation(), SM, LangOpts); + if (!EqualToken.has_value()) +return; + SourceLocation EqualLoc{EqualToken->getLocation()}; + if (EqualLoc.isInvalid() || EqualLoc.isMacroID()) +return; + SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange(); + if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() || + InitExprRange.getEnd().isMacroID()) +return; + Diag << FixItHint::CreateRemoval(EqualLoc) + << FixItHint::CreateRemoval(InitExprRange); + return; +} + +namespace { + +AST_MATCHER(EnumDecl, isMacro) { + SourceLocation Loc = Node.getBeginLoc(); + return Loc.isMacroID(); +} + +AST_MATCHER(EnumDecl, hasConsistentInitialValues) { + return isNoneEnumeratorsInitialized(Node) || + isOnlyFirstEnumeratorInitialized(Node) || + areAllEnumeratorsInitialized(Node); +} + +AST_MATCHER(EnumDecl, hasZeroInitialValueForFirstEnumerator) { + EnumDecl::enumerator_range Enumerators = Node.enumerators(); + if (Enumerators.empty()) +return false; + const EnumConstantDecl *ECD = *Enumerators.begin(); + return isOnlyFirstEnumeratorInitialized(Node) && + isInitializedByLiteral(ECD) && ECD->getInitVal().isZero(); +} + +/// 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 +/// bitmask, evident when enumerators are only initialized with (potentially +/// negative) integer literals, are ignored. This is also the case when all +/// enumerators are powers of two (e.g., 0, 1, 2). +AST_MATCHER(EnumDecl, hasSequentialInitialValues) { + EnumDecl::enumerator_range Enumerators = Node.enumerators(); + if (Enumerators.empty()) +return false; + const EnumConstantDecl *const FirstEnumerator = *Node.enumerator_begin(); + llvm::APSInt PrevValue = FirstEnumerator->getInitVal(); + if (!isInitializedByLiteral(FirstEnumerator)) +return false; + bool AllEnumeratorsArePowersOfTwo = true; + for (const EnumConstantDecl *Enumerator : llvm::drop_begin(Enumerators)) { +const llvm::APSInt NewValue = Enumerator->getInitVal(); +if (NewValue != ++PrevValue) + return false; +if (!isInitializedByLiteral(Enumerator)) + return false; +PrevValue = NewValue; +AllEnumeratorsArePowersOfTwo &= NewValue.isPowerOf2(); + } + return !AllEnumeratorsArePowersOfTwo; +} + +} // namespace + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", tru
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/86129 >From 4e0845a143a820d4a68ffbdced206654c7593359 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Fri, 15 Mar 2024 08:07:47 +0800 Subject: [PATCH 1/7] [clang-tidy] add new check readability-enum-initial-value Fixes: #85243. --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/EnumInitialValueCheck.cpp | 82 +++ .../readability/EnumInitialValueCheck.h | 31 +++ .../readability/ReadabilityTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../checks/readability/enum-initial-value.rst | 45 ++ .../checkers/readability/enum-initial-value.c | 27 ++ .../readability/enum-initial-value.cpp| 27 ++ 9 files changed, 223 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/enum-initial-value.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5728c9970fb65d..dd772d69202548 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -17,6 +17,7 @@ add_clang_library(clangTidyReadabilityModule DeleteNullPointerCheck.cpp DuplicateIncludeCheck.cpp ElseAfterReturnCheck.cpp + EnumInitialValueCheck.cpp FunctionCognitiveComplexityCheck.cpp FunctionSizeCheck.cpp IdentifierLengthCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp new file mode 100644 index 00..78d5101d439dde --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp @@ -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( + .bind("enum"), + this); +} + +void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Enum = Result.Nodes.getNodeAs("enum"); + assert(Enum != nullptr); + SourceLocation Loc = Enum->getBeginLoc(); + if (Loc.isInvalid() || Loc.isMacroID()) +return; + DiagnosticBuilder Diag = + diag(Loc, "inital value in enum %0 has readability issue, " +"explicit initialization of all of enumerators") + << Enum->getName(); + for (EnumConstantDecl const *ECD : Enum->enumerators()) +if (ECD->getInitExpr() == nullptr) { + SourceLocation ECDLoc = ECD->getEndLoc(); + if (ECDLoc.isInvalid() || ECDLoc.isMacroID()) +continue; + std::optional Next = utils::lexer::findNextTokenSkippingComments( + ECDLoc, *Result.SourceManager, getLangOpts()); + if (!Next.has_value() || Next->getLocation().isMacroID()) +continue; + llvm::SmallString<8> Str{" = "}
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +static bool isOnlyFirstEnumeratorInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) { +if ((IsFirst && ECD->getInitExpr() == nullptr) || +(!IsFirst && ECD->getInitExpr() != nullptr)) + return false; +IsFirst = false; + } + return !IsFirst; +} + +static bool areAllEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() != nullptr; + }); +} + +/// Check if \p Enumerator is initialized with a (potentially negated) \c +/// IntegerLiteral. +static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) { + const Expr *const Init = Enumerator->getInitExpr(); + if (!Init) +return false; + return Init->isIntegerConstantExpr(Enumerator->getASTContext()); +} + +static void cleanInitialValue(DiagnosticBuilder &Diag, + const EnumConstantDecl *ECD, + const SourceManager &SM, + const LangOptions &LangOpts) { + std::optional EqualToken = utils::lexer::findNextTokenSkippingComments( + ECD->getLocation(), SM, LangOpts); + if (!EqualToken.has_value()) +return; + SourceLocation EqualLoc{EqualToken->getLocation()}; + if (EqualLoc.isInvalid() || EqualLoc.isMacroID()) +return; + SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange(); + if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() || + InitExprRange.getEnd().isMacroID()) +return; + Diag << FixItHint::CreateRemoval(EqualLoc) + << FixItHint::CreateRemoval(InitExprRange); + return; +} + +namespace { + +AST_MATCHER(EnumDecl, isMacro) { + SourceLocation Loc = Node.getBeginLoc(); + return Loc.isMacroID(); +} + +AST_MATCHER(EnumDecl, hasConsistentInitialValues) { + return isNoneEnumeratorsInitialized(Node) || + isOnlyFirstEnumeratorInitialized(Node) || + areAllEnumeratorsInitialized(Node); +} + +AST_MATCHER(EnumDecl, hasZeroInitialValueForFirstEnumerator) { + EnumDecl::enumerator_range Enumerators = Node.enumerators(); + if (Enumerators.empty()) +return false; + const EnumConstantDecl *ECD = *Enumerators.begin(); + return isOnlyFirstEnumeratorInitialized(Node) && + isInitializedByLiteral(ECD) && ECD->getInitVal().isZero(); +} + +/// 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 +/// bitmask, evident when enumerators are only initialized with (potentially +/// negative) integer literals, are ignored. This is also the case when all +/// enumerators are powers of two (e.g., 0, 1, 2). +AST_MATCHER(EnumDecl, hasSequentialInitialValues) { + EnumDecl::enumerator_range Enumerators = Node.enumerators(); + if (Enumerators.empty()) +return false; + const EnumConstantDecl *const FirstEnumerator = *Node.enumerator_begin(); + llvm::APSInt PrevValue = FirstEnumerator->getInitVal(); + if (!isInitializedByLiteral(FirstEnumerator)) +return false; + bool AllEnumeratorsArePowersOfTwo = true; + for (const EnumConstantDecl *Enumerator : llvm::drop_begin(Enumerators)) { +const llvm::APSInt NewValue = Enumerator->getInitVal(); +if (NewValue != ++PrevValue) + return false; +if (!isInitializedByLiteral(Enumerator)) + return false; +PrevValue = NewValue; +AllEnumeratorsArePowersOfTwo &= NewValue.isPowerOf2(); + } + return !AllEnumeratorsArePowersOfTwo; +} + +} // namespace + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplicitZeroFirstInitialValue( + Options.get("AllowExplicitZeroFirstInitialValue", tru
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/86129 >From 4e0845a143a820d4a68ffbdced206654c7593359 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Fri, 15 Mar 2024 08:07:47 +0800 Subject: [PATCH 1/8] [clang-tidy] add new check readability-enum-initial-value Fixes: #85243. --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/EnumInitialValueCheck.cpp | 82 +++ .../readability/EnumInitialValueCheck.h | 31 +++ .../readability/ReadabilityTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../checks/readability/enum-initial-value.rst | 45 ++ .../checkers/readability/enum-initial-value.c | 27 ++ .../readability/enum-initial-value.cpp| 27 ++ 9 files changed, 223 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/enum-initial-value.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.c create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/enum-initial-value.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5728c9970fb65d..dd772d69202548 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -17,6 +17,7 @@ add_clang_library(clangTidyReadabilityModule DeleteNullPointerCheck.cpp DuplicateIncludeCheck.cpp ElseAfterReturnCheck.cpp + EnumInitialValueCheck.cpp FunctionCognitiveComplexityCheck.cpp FunctionSizeCheck.cpp IdentifierLengthCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp new file mode 100644 index 00..78d5101d439dde --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/EnumInitialValueCheck.cpp @@ -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( + .bind("enum"), + this); +} + +void EnumInitialValueCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Enum = Result.Nodes.getNodeAs("enum"); + assert(Enum != nullptr); + SourceLocation Loc = Enum->getBeginLoc(); + if (Loc.isInvalid() || Loc.isMacroID()) +return; + DiagnosticBuilder Diag = + diag(Loc, "inital value in enum %0 has readability issue, " +"explicit initialization of all of enumerators") + << Enum->getName(); + for (EnumConstantDecl const *ECD : Enum->enumerators()) +if (ECD->getInitExpr() == nullptr) { + SourceLocation ECDLoc = ECD->getEndLoc(); + if (ECDLoc.isInvalid() || ECDLoc.isMacroID()) +continue; + std::optional Next = utils::lexer::findNextTokenSkippingComments( + ECDLoc, *Result.SourceManager, getLangOpts()); + if (!Next.has_value() || Next->getLocation().isMacroID()) +continue; + llvm::SmallString<8> Str{" = "}
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/PiotrZSL edited 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/PiotrZSL approved this pull request. For me looks fine (had to check how it behave when coma is in next line). Some ideas for separate check or future options: - enforce that enums are sorted by value - enforce that enums are sorted by name - enforce that there is no duplicated values (bugprone), not there is warning for implicit duplicate, but not explicit. 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,200 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +static bool isOnlyFirstEnumeratorInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) { +if ((IsFirst && ECD->getInitExpr() == nullptr) || +(!IsFirst && ECD->getInitExpr() != nullptr)) + return false; +IsFirst = false; + } + return !IsFirst; +} + +static bool areAllEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() != nullptr; + }); +} + +/// Check if \p Enumerator is initialized with a (potentially negated) \c +/// IntegerLiteral. +static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) { + const Expr *const Init = Enumerator->getInitExpr(); + if (!Init) +return false; + return Init->isIntegerConstantExpr(Enumerator->getASTContext()); +} + +static void cleanInitialValue(DiagnosticBuilder &Diag, + const EnumConstantDecl *ECD, + const SourceManager &SM, + const LangOptions &LangOpts) { + const SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange(); + if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() || + InitExprRange.getEnd().isMacroID()) +return; + std::optional EqualToken = utils::lexer::findNextTokenSkippingComments( + ECD->getLocation(), SM, LangOpts); + if (!EqualToken.has_value() || + EqualToken.value().getKind() != tok::TokenKind::equal) +return; + const SourceLocation EqualLoc{EqualToken->getLocation()}; + if (EqualLoc.isInvalid() || EqualLoc.isMacroID()) +return; + Diag << FixItHint::CreateRemoval(EqualLoc) + << FixItHint::CreateRemoval(InitExprRange); + return; +} + +namespace { + +AST_MATCHER(EnumDecl, isMacro) { + SourceLocation Loc = Node.getBeginLoc(); + return Loc.isMacroID(); +} + +AST_MATCHER(EnumDecl, hasConsistentInitialValues) { + return isNoneEnumeratorsInitialized(Node) || + isOnlyFirstEnumeratorInitialized(Node) || + areAllEnumeratorsInitialized(Node); +} + +AST_MATCHER(EnumDecl, hasZeroInitialValueForFirstEnumerator) { + const EnumDecl::enumerator_range Enumerators = Node.enumerators(); + if (Enumerators.empty()) +return false; + const EnumConstantDecl *ECD = *Enumerators.begin(); + return isOnlyFirstEnumeratorInitialized(Node) && + isInitializedByLiteral(ECD) && ECD->getInitVal().isZero(); +} + +/// 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 +/// bitmask, evident when enumerators are only initialized with (potentially +/// negative) integer literals, are ignored. This is also the case when all +/// enumerators are powers of two (e.g., 0, 1, 2). +AST_MATCHER(EnumDecl, hasSequentialInitialValues) { + const EnumDecl::enumerator_range Enumerators = Node.enumerators(); + if (Enumerators.empty()) +return false; + const EnumConstantDecl *const FirstEnumerator = *Node.enumerator_begin(); + llvm::APSInt PrevValue = FirstEnumerator->getInitVal(); + if (!isInitializedByLiteral(FirstEnumerator)) +return false; + bool AllEnumeratorsArePowersOfTwo = true; + for (const EnumConstantDecl *Enumerator : llvm::drop_begin(Enumerators)) { +const llvm::APSInt NewValue = Enumerator->getInitVal(); +if (NewValue != ++PrevValue) + return false; +if (!isInitializedByLiteral(Enumerator)) + return false; +PrevValue = NewValue; +AllEnumeratorsArePowersOfTwo &= NewValue.isPowerOf2(); + } + return !AllEnumeratorsArePowersOfTwo; +} + +} // namespace + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplici
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/PiotrZSL edited 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
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,200 @@ +//===--- 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 "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static bool isNoneEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() == nullptr; + }); +} + +static bool isOnlyFirstEnumeratorInitialized(const EnumDecl &Node) { + bool IsFirst = true; + for (const EnumConstantDecl *ECD : Node.enumerators()) { +if ((IsFirst && ECD->getInitExpr() == nullptr) || +(!IsFirst && ECD->getInitExpr() != nullptr)) + return false; +IsFirst = false; + } + return !IsFirst; +} + +static bool areAllEnumeratorsInitialized(const EnumDecl &Node) { + return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) { +return ECD->getInitExpr() != nullptr; + }); +} + +/// Check if \p Enumerator is initialized with a (potentially negated) \c +/// IntegerLiteral. +static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) { + const Expr *const Init = Enumerator->getInitExpr(); + if (!Init) +return false; + return Init->isIntegerConstantExpr(Enumerator->getASTContext()); +} + +static void cleanInitialValue(DiagnosticBuilder &Diag, + const EnumConstantDecl *ECD, + const SourceManager &SM, + const LangOptions &LangOpts) { + const SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange(); + if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() || + InitExprRange.getEnd().isMacroID()) +return; + std::optional EqualToken = utils::lexer::findNextTokenSkippingComments( + ECD->getLocation(), SM, LangOpts); + if (!EqualToken.has_value() || + EqualToken.value().getKind() != tok::TokenKind::equal) +return; + const SourceLocation EqualLoc{EqualToken->getLocation()}; + if (EqualLoc.isInvalid() || EqualLoc.isMacroID()) +return; + Diag << FixItHint::CreateRemoval(EqualLoc) + << FixItHint::CreateRemoval(InitExprRange); + return; +} + +namespace { + +AST_MATCHER(EnumDecl, isMacro) { + SourceLocation Loc = Node.getBeginLoc(); + return Loc.isMacroID(); +} + +AST_MATCHER(EnumDecl, hasConsistentInitialValues) { + return isNoneEnumeratorsInitialized(Node) || + isOnlyFirstEnumeratorInitialized(Node) || + areAllEnumeratorsInitialized(Node); +} + +AST_MATCHER(EnumDecl, hasZeroInitialValueForFirstEnumerator) { + const EnumDecl::enumerator_range Enumerators = Node.enumerators(); + if (Enumerators.empty()) +return false; + const EnumConstantDecl *ECD = *Enumerators.begin(); + return isOnlyFirstEnumeratorInitialized(Node) && + isInitializedByLiteral(ECD) && ECD->getInitVal().isZero(); +} + +/// 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 +/// bitmask, evident when enumerators are only initialized with (potentially +/// negative) integer literals, are ignored. This is also the case when all +/// enumerators are powers of two (e.g., 0, 1, 2). +AST_MATCHER(EnumDecl, hasSequentialInitialValues) { + const EnumDecl::enumerator_range Enumerators = Node.enumerators(); + if (Enumerators.empty()) +return false; + const EnumConstantDecl *const FirstEnumerator = *Node.enumerator_begin(); + llvm::APSInt PrevValue = FirstEnumerator->getInitVal(); + if (!isInitializedByLiteral(FirstEnumerator)) +return false; + bool AllEnumeratorsArePowersOfTwo = true; + for (const EnumConstantDecl *Enumerator : llvm::drop_begin(Enumerators)) { +const llvm::APSInt NewValue = Enumerator->getInitVal(); +if (NewValue != ++PrevValue) + return false; +if (!isInitializedByLiteral(Enumerator)) + return false; +PrevValue = NewValue; +AllEnumeratorsArePowersOfTwo &= NewValue.isPowerOf2(); + } + return !AllEnumeratorsArePowersOfTwo; +} + +} // namespace + +EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name, + ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + AllowExplici
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/HerrCai0907 closed 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