https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/136823
From 4ce7497bb0dc89de3b9f139177c295291e7d3e9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Thu, 17 Apr 2025 17:36:03 +0200 Subject: [PATCH 1/5] [clang-tidy] Add check 'bugprone-invalid-enum-default-initialization' --- .../bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../InvalidEnumDefaultInitializationCheck.cpp | 114 +++++++++++++++++ .../InvalidEnumDefaultInitializationCheck.h | 32 +++++ .../invalid-enum-default-initialization.rst | 50 ++++++++ .../invalid-enum-default-initialization.cpp | 116 ++++++++++++++++++ 6 files changed, 316 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index b780a85bdf3fe..893dd140c92b0 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -38,6 +38,7 @@ #include "IncorrectRoundingsCheck.h" #include "InfiniteLoopCheck.h" #include "IntegerDivisionCheck.h" +#include "InvalidEnumDefaultInitializationCheck.h" #include "LambdaFunctionNameCheck.h" #include "MacroParenthesesCheck.h" #include "MacroRepeatedSideEffectsCheck.h" @@ -164,6 +165,8 @@ class BugproneModule : public ClangTidyModule { CheckFactories.registerCheck<InfiniteLoopCheck>("bugprone-infinite-loop"); CheckFactories.registerCheck<IntegerDivisionCheck>( "bugprone-integer-division"); + CheckFactories.registerCheck<InvalidEnumDefaultInitializationCheck>( + "bugprone-invalid-enum-default-initialization"); CheckFactories.registerCheck<LambdaFunctionNameCheck>( "bugprone-lambda-function-name"); CheckFactories.registerCheck<MacroParenthesesCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index e310ea9c94543..bf142575becfb 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -30,6 +30,7 @@ add_clang_library(clangTidyBugproneModule STATIC InaccurateEraseCheck.cpp IncorrectEnableIfCheck.cpp IncorrectEnableSharedFromThisCheck.cpp + InvalidEnumDefaultInitializationCheck.cpp UnintendedCharOstreamOutputCheck.cpp ReturnConstRefFromParameterCheck.cpp SuspiciousStringviewDataUsageCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp new file mode 100644 index 0000000000000..152c2468cd121 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp @@ -0,0 +1,114 @@ +//===--- InvalidEnumDefaultInitializationCheck.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 "InvalidEnumDefaultInitializationCheck.h" +// #include "../utils/Matchers.h" +// #include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include <algorithm> + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +namespace { + +AST_MATCHER(EnumDecl, isCompleteAndHasNoZeroValue) { + const EnumDecl *Definition = Node.getDefinition(); + return Definition && Node.isComplete() && + std::none_of(Definition->enumerator_begin(), + Definition->enumerator_end(), + [](const EnumConstantDecl *Value) { + return Value->getInitVal().isZero(); + }); +} + +AST_MATCHER(Expr, isEmptyInit) { + if (isa<CXXScalarValueInitExpr>(&Node)) + return true; + if (isa<ImplicitValueInitExpr>(&Node)) + return true; + if (const auto *Init = dyn_cast<InitListExpr>(&Node)) + return Init->getNumInits() == 0; + return false; +} + +} // namespace + +InvalidEnumDefaultInitializationCheck::InvalidEnumDefaultInitializationCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + +bool InvalidEnumDefaultInitializationCheck::isLanguageVersionSupported( + const LangOptions &LangOpts) const { + return LangOpts.CPlusPlus; +} + +void InvalidEnumDefaultInitializationCheck::registerMatchers( + MatchFinder *Finder) { + Finder->addMatcher( + expr(isEmptyInit(), + hasType(hasUnqualifiedDesugaredType(enumType(hasDeclaration( + enumDecl(isCompleteAndHasNoZeroValue()).bind("enum")))))) + .bind("expr"), + this); +} + +void InvalidEnumDefaultInitializationCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *InitExpr = Result.Nodes.getNodeAs<Expr>("expr"); + const auto *Enum = Result.Nodes.getNodeAs<EnumDecl>("enum"); + if (!InitExpr || !Enum) + return; + + ASTContext &ACtx = Enum->getASTContext(); + SourceLocation Loc = InitExpr->getExprLoc(); + if (Loc.isInvalid()) { + if (isa<ImplicitValueInitExpr>(InitExpr) || isa<InitListExpr>(InitExpr)) { + DynTypedNodeList Parents = ACtx.getParents(*InitExpr); + if (Parents.empty()) + return; + + if (const auto *Ctor = Parents[0].get<CXXConstructorDecl>()) { + // Try to find member initializer with the found expression and get the + // source location from it. + CXXCtorInitializer *const *CtorInit = std::find_if( + Ctor->init_begin(), Ctor->init_end(), + [InitExpr](const CXXCtorInitializer *Init) { + return Init->isMemberInitializer() && Init->getInit() == InitExpr; + }); + if (!CtorInit) + return; + Loc = (*CtorInit)->getLParenLoc(); + } else if (const auto *InitList = Parents[0].get<InitListExpr>()) { + // The expression may be implicitly generated for an initialization. + // Search for a parent initialization list with valid source location. + while (InitList->getExprLoc().isInvalid()) { + DynTypedNodeList Parents = ACtx.getParents(*InitList); + if (Parents.empty()) + return; + InitList = Parents[0].get<InitListExpr>(); + if (!InitList) + return; + } + Loc = InitList->getExprLoc(); + } + } + // If still not found a source location, omit the warning. + // FIXME: All such cases should be fixed to make the checker more precise. + if (Loc.isInvalid()) + return; + } + diag(Loc, "enum value of type %0 initialized with invalid value of 0, " + "enum doesn't have a zero-value enumerator") + << Enum; + diag(Enum->getLocation(), "enum is defined here", DiagnosticIDs::Note); +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h new file mode 100644 index 0000000000000..fa5d068f2ae32 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h @@ -0,0 +1,32 @@ +//===--- InvalidEnumDefaultInitializationCheck.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_BUGPRONE_INVALIDENUMDEFAULTINITIALIZATIONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INVALIDENUMDEFAULTINITIALIZATIONCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Detect default initialization (to 0) of variables with `enum` type where +/// the enum has no enumerator with value of 0. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/.html +class InvalidEnumDefaultInitializationCheck : public ClangTidyCheck { +public: + InvalidEnumDefaultInitializationCheck(StringRef Name, + ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override; +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INVALIDENUMDEFAULTINITIALIZATIONCHECK_H diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst new file mode 100644 index 0000000000000..634dec2aecd25 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst @@ -0,0 +1,50 @@ +.. title:: clang-tidy - bugprone-invalid-enum-default-initialization + +bugprone-invalid-enum-default-initialization +============================================ + +Detect default initialization (to 0) of variables with `enum` type where +the enum has no enumerator with value of 0. + +In C++ a default initialization is performed if a variable is initialized with +initializer list or in other implicit ways, and no value is specified at the +initialization. In such cases the value 0 is used for the initialization. +This also applies to enumerations even if it does not have an enumerator with +value 0. In this way a variable with the enum type may contain initially an +invalid value (if the program expects that it contains only the listed +enumerator values). + +The checker emits a warning only if an enum variable is default-initialized +(contrary to not initialized) and the enum type does not have an enumerator with +value of 0. The enum type can be scoped or non-scoped enum. + +.. code-block:: c++ + + enum class Enum1: int { + A = 1, + B + }; + + enum class Enum0: int { + A = 0, + B + }; + + void f() { + Enum1 X1{}; // warn: 'X1' is initialized to 0 + Enum1 X2 = Enum1(); // warn: 'X2' is initialized to 0 + Enum1 X3; // no warning: 'X3' is not initialized + Enum0 X4{}; // no warning: type has an enumerator with value of 0 + } + + struct S1 { + Enum1 A; + S(): A() {} // warn: 'A' is initialized to 0 + }; + + struct S2 { + int A; + Enum1 B; + }; + + S2 VarS2{}; // warn: member 'B' is initialized to 0 diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp new file mode 100644 index 0000000000000..857093c2239cd --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp @@ -0,0 +1,116 @@ +// RUN: %check_clang_tidy -std=c++17 %s bugprone-invalid-enum-default-initialization %t + +enum class Enum0: int { + A = 0, + B +}; + +enum class Enum1: int { + A = 1, + B +}; + +enum Enum2 { + Enum_A = 4, + Enum_B +}; + +Enum0 E0_1{}; +Enum0 E0_2 = Enum0(); +Enum0 E0_3; +Enum0 E0_4{0}; +Enum0 E0_5{Enum0::A}; +Enum0 E0_6{Enum0::B}; + +Enum1 E1_1{}; +// CHECK-NOTES: :[[@LINE-1]]:11: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator +// CHECK-NOTES: :8:12: note: enum is defined here +Enum1 E1_2 = Enum1(); +// CHECK-NOTES: :[[@LINE-1]]:14: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator +// CHECK-NOTES: :8:12: note: enum is defined here +Enum1 E1_3; +Enum1 E1_4{0}; +Enum1 E1_5{Enum1::A}; +Enum1 E1_6{Enum1::B}; + +Enum2 E2_1{}; +// CHECK-NOTES: :[[@LINE-1]]:11: warning: enum value of type 'Enum2' initialized with invalid value of 0, enum doesn't have a zero-value enumerator +// CHECK-NOTES: :13:6: note: enum is defined here +Enum2 E2_2 = Enum2(); +// CHECK-NOTES: :[[@LINE-1]]:14: warning: enum value of type 'Enum2' initialized with invalid value of 0, enum doesn't have a zero-value enumerator +// CHECK-NOTES: :13:6: note: enum is defined here + +void f1() { + static Enum1 S; // FIMXE: warn for this? + Enum1 A; + Enum1 B = Enum1(); + // CHECK-NOTES: :[[@LINE-1]]:13: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator + // CHECK-NOTES: :8:12: note: enum is defined here + int C = int(); +} + +void f2() { + Enum1 A{}; + // CHECK-NOTES: :[[@LINE-1]]:10: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator + // CHECK-NOTES: :8:12: note: enum is defined here + Enum1 B = Enum1(); + // CHECK-NOTES: :[[@LINE-1]]:13: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator + // CHECK-NOTES: :8:12: note: enum is defined here + Enum1 C[5] = {{}}; + // CHECK-NOTES: :[[@LINE-1]]:17: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator + // CHECK-NOTES: :8:12: note: enum is defined here + Enum1 D[5] = {}; // FIMXE: warn for this? +} + +struct S1 { + Enum1 E_1{}; + // CHECK-NOTES: :[[@LINE-1]]:12: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator + // CHECK-NOTES: :8:12: note: enum is defined here + Enum1 E_2 = Enum1(); + // CHECK-NOTES: :[[@LINE-1]]:15: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator + // CHECK-NOTES: :8:12: note: enum is defined here + Enum1 E_3; + Enum1 E_4; + Enum1 E_5; + + S1() : + E_3{}, + // CHECK-NOTES: :[[@LINE-1]]:8: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator + // CHECK-NOTES: :8:12: note: enum is defined here + E_4(), + // CHECK-NOTES: :[[@LINE-1]]:8: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator + // CHECK-NOTES: :8:12: note: enum is defined here + E_5{Enum1::B} + {} +}; + +struct S2 { + Enum0 X; + Enum1 Y; + Enum2 Z; +}; + +struct S3 { + S2 X; + int Y; +}; + +struct S4 : public S3 { + int Z; +}; + +S2 VarS2{}; +// CHECK-NOTES: :[[@LINE-1]]:9: warning: enum value of type 'Enum1' initialized with invalid value of 0 +// CHECK-NOTES: :8:12: note: enum is defined here +// CHECK-NOTES: :[[@LINE-3]]:9: warning: enum value of type 'Enum2' initialized with invalid value of 0 +// CHECK-NOTES: :13:6: note: enum is defined here +S3 VarS3{}; +// CHECK-NOTES: :[[@LINE-1]]:10: warning: enum value of type 'Enum1' initialized with invalid value of 0 +// CHECK-NOTES: :8:12: note: enum is defined here +// CHECK-NOTES: :[[@LINE-3]]:10: warning: enum value of type 'Enum2' initialized with invalid value of 0 +// CHECK-NOTES: :13:6: note: enum is defined here +S4 VarS4{}; +// CHECK-NOTES: :[[@LINE-1]]:10: warning: enum value of type 'Enum1' initialized with invalid value of 0 +// CHECK-NOTES: :8:12: note: enum is defined here +// CHECK-NOTES: :[[@LINE-3]]:10: warning: enum value of type 'Enum2' initialized with invalid value of 0 +// CHECK-NOTES: :13:6: note: enum is defined here From e5744a80af9b75b0d17a00597d4512a62a0e8b7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Thu, 24 Apr 2025 15:45:21 +0200 Subject: [PATCH 2/5] small code fixes, added some tests, fixed doc --- .../InvalidEnumDefaultInitializationCheck.cpp | 4 +--- .../InvalidEnumDefaultInitializationCheck.h | 2 +- clang-tools-extra/docs/ReleaseNotes.rst | 6 ++++++ .../invalid-enum-default-initialization.rst | 2 +- .../invalid-enum-default-initialization.cpp | 17 +++++++++++++++++ 5 files changed, 26 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp index 152c2468cd121..38403396a7bf8 100644 --- a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp @@ -7,8 +7,6 @@ //===----------------------------------------------------------------------===// #include "InvalidEnumDefaultInitializationCheck.h" -// #include "../utils/Matchers.h" -// #include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include <algorithm> @@ -21,7 +19,7 @@ namespace { AST_MATCHER(EnumDecl, isCompleteAndHasNoZeroValue) { const EnumDecl *Definition = Node.getDefinition(); - return Definition && Node.isComplete() && + return Definition && Node.isComplete() && !Node.enumerators().empty() && std::none_of(Definition->enumerator_begin(), Definition->enumerator_end(), [](const EnumConstantDecl *Value) { diff --git a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h index fa5d068f2ae32..c053477372c43 100644 --- a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h @@ -17,7 +17,7 @@ namespace clang::tidy::bugprone { /// the enum has no enumerator with value of 0. /// /// For the user-facing documentation see: -/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/.html +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/invalid-enum-default-initialization.html class InvalidEnumDefaultInitializationCheck : public ClangTidyCheck { public: InvalidEnumDefaultInitializationCheck(StringRef Name, diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a8ae35c7f744e..5d6549f95a1a3 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -125,6 +125,12 @@ New checks Finds potentially erroneous calls to ``reset`` method on smart pointers when the pointee type also has a ``reset`` method. +- New :doc:`bugprone-invalid-enum-default-initialization + <clang-tidy/checks/bugprone/invalid-enum-default-initialization>` check. + + Detect default initialization (to 0) of variables with ``enum`` type where + the enum has no enumerator with value of 0. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst index 634dec2aecd25..f03f1743a9f1f 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst @@ -3,7 +3,7 @@ bugprone-invalid-enum-default-initialization ============================================ -Detect default initialization (to 0) of variables with `enum` type where +Detect default initialization (to 0) of variables with ``enum`` type where the enum has no enumerator with value of 0. In C++ a default initialization is performed if a variable is initialized with diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp index 857093c2239cd..b4e2d5367fea7 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp @@ -114,3 +114,20 @@ S4 VarS4{}; // CHECK-NOTES: :8:12: note: enum is defined here // CHECK-NOTES: :[[@LINE-3]]:10: warning: enum value of type 'Enum2' initialized with invalid value of 0 // CHECK-NOTES: :13:6: note: enum is defined here + +enum class EnumFwd; + +EnumFwd Fwd{}; + +enum class EnumEmpty {}; + +EnumEmpty Empty{}; + +template<typename T> +struct Templ { + T Mem1{}; + // CHECK-NOTES: :[[@LINE-1]]:9: warning: enum value of type 'Enum1' initialized with invalid value of 0 + // CHECK-NOTES: :8:12: note: enum is defined here +}; + +Templ<Enum1> TemplVar; From 6920e316f659f7eee22e44115d64c995cc02ab6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Fri, 25 Apr 2025 10:32:05 +0200 Subject: [PATCH 3/5] documentation fixes --- .../bugprone/InvalidEnumDefaultInitializationCheck.h | 2 +- clang-tools-extra/docs/ReleaseNotes.rst | 12 ++++++------ .../bugprone/invalid-enum-default-initialization.rst | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h index c053477372c43..c1c21f757ef53 100644 --- a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h @@ -13,7 +13,7 @@ namespace clang::tidy::bugprone { -/// Detect default initialization (to 0) of variables with `enum` type where +/// Detects default initialization (to 0) of variables with `enum` type where /// the enum has no enumerator with value of 0. /// /// For the user-facing documentation see: diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 5d6549f95a1a3..344773a98f184 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -113,6 +113,12 @@ New checks pointer and store it as class members without handle the copy and move constructors and the assignments. +- New :doc:`bugprone-invalid-enum-default-initialization + <clang-tidy/checks/bugprone/invalid-enum-default-initialization>` check. + + Detects default initialization (to 0) of variables with ``enum`` type where + the enum has no enumerator with value of 0. + - New :doc:`bugprone-unintended-char-ostream-output <clang-tidy/checks/bugprone/unintended-char-ostream-output>` check. @@ -125,12 +131,6 @@ New checks Finds potentially erroneous calls to ``reset`` method on smart pointers when the pointee type also has a ``reset`` method. -- New :doc:`bugprone-invalid-enum-default-initialization - <clang-tidy/checks/bugprone/invalid-enum-default-initialization>` check. - - Detect default initialization (to 0) of variables with ``enum`` type where - the enum has no enumerator with value of 0. - New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst index f03f1743a9f1f..cf24ad4783304 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst @@ -3,7 +3,7 @@ bugprone-invalid-enum-default-initialization ============================================ -Detect default initialization (to 0) of variables with ``enum`` type where +Detects default initialization (to 0) of variables with ``enum`` type where the enum has no enumerator with value of 0. In C++ a default initialization is performed if a variable is initialized with From 96209555de00b00eb3031f6c0ff38b8832be5513 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Fri, 9 May 2025 09:57:54 +0200 Subject: [PATCH 4/5] updated list.rst --- clang-tools-extra/docs/clang-tidy/checks/list.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 18f1467285fab..5e9be9684a984 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -106,6 +106,7 @@ Clang-Tidy Checks :doc:`bugprone-incorrect-roundings <bugprone/incorrect-roundings>`, :doc:`bugprone-infinite-loop <bugprone/infinite-loop>`, :doc:`bugprone-integer-division <bugprone/integer-division>`, + :doc:`bugprone-invalid-enum-default-initialization <bugprone/invalid-enum-default-initialization>`, :doc:`bugprone-lambda-function-name <bugprone/lambda-function-name>`, :doc:`bugprone-macro-parentheses <bugprone/macro-parentheses>`, "Yes" :doc:`bugprone-macro-repeated-side-effects <bugprone/macro-repeated-side-effects>`, From b726e03e2671cf10479f37ebe129b779b2169b49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Thu, 29 May 2025 08:55:37 +0200 Subject: [PATCH 5/5] improvements for initialization lists --- .../InvalidEnumDefaultInitializationCheck.cpp | 101 +++++++++++++++--- .../InvalidEnumDefaultInitializationCheck.h | 1 - .../invalid-enum-default-initialization.rst | 24 ++++- .../invalid-enum-default-initialization.c | 54 ++++++++++ .../invalid-enum-default-initialization.cpp | 14 ++- 5 files changed, 175 insertions(+), 19 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.c diff --git a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp index 38403396a7bf8..e8c0dd768a9d3 100644 --- a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.cpp @@ -8,6 +8,7 @@ #include "InvalidEnumDefaultInitializationCheck.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/TypeVisitor.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include <algorithm> @@ -17,9 +18,10 @@ namespace clang::tidy::bugprone { namespace { -AST_MATCHER(EnumDecl, isCompleteAndHasNoZeroValue) { - const EnumDecl *Definition = Node.getDefinition(); - return Definition && Node.isComplete() && !Node.enumerators().empty() && +bool isCompleteAndHasNoZeroValue(const EnumDecl *D) { + const EnumDecl *Definition = D->getDefinition(); + return Definition && Definition->isComplete() && + !Definition->enumerators().empty() && std::none_of(Definition->enumerator_begin(), Definition->enumerator_end(), [](const EnumConstantDecl *Value) { @@ -27,41 +29,107 @@ AST_MATCHER(EnumDecl, isCompleteAndHasNoZeroValue) { }); } +AST_MATCHER(EnumDecl, isCompleteAndHasNoZeroValue) { + return isCompleteAndHasNoZeroValue(&Node); +} + +// Find an initialization which initializes the value (if it has enum type) to a +// default zero value. AST_MATCHER(Expr, isEmptyInit) { if (isa<CXXScalarValueInitExpr>(&Node)) return true; if (isa<ImplicitValueInitExpr>(&Node)) return true; - if (const auto *Init = dyn_cast<InitListExpr>(&Node)) - return Init->getNumInits() == 0; + if (const auto *Init = dyn_cast<InitListExpr>(&Node)) { + if (Init->getNumInits() == 0) + return true; + } return false; } +AST_MATCHER(InitListExpr, hasArrayFiller) { return Node.hasArrayFiller(); } + +// Check if any type has a "child" type that is an enum without zero value. +// The "child" type can be an array element type or member type of a record +// type (or a recursive combination of these). In this case, if the "root" type +// is statically initialized, the enum component is initialized to zero. +class FindEnumMember : public TypeVisitor<FindEnumMember, bool> { +public: + const EnumType *FoundEnum = nullptr; + + bool VisitType(const Type *T) { + const Type *DesT = T->getUnqualifiedDesugaredType(); + if (DesT != T) + return Visit(DesT); + return false; + } + bool VisitArrayType(const ArrayType *T) { + return Visit(T->getElementType()->getUnqualifiedDesugaredType()); + } + bool VisitConstantArrayType(const ConstantArrayType *T) { + return Visit(T->getElementType()->getUnqualifiedDesugaredType()); + } + bool VisitEnumType(const EnumType *T) { + if (isCompleteAndHasNoZeroValue(T->getDecl())) { + FoundEnum = T; + return true; + } + return false; + } + bool VisitRecordType(const RecordType *T) { + const RecordDecl *RD = T->getDecl(); + if (RD->isUnion()) + return false; + auto VisitField = [this](const FieldDecl *F) { + return Visit(F->getType()->getUnqualifiedDesugaredType()); + }; + return llvm::any_of(RD->fields(), VisitField); + } +}; + } // namespace InvalidEnumDefaultInitializationCheck::InvalidEnumDefaultInitializationCheck( StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context) {} -bool InvalidEnumDefaultInitializationCheck::isLanguageVersionSupported( - const LangOptions &LangOpts) const { - return LangOpts.CPlusPlus; -} - void InvalidEnumDefaultInitializationCheck::registerMatchers( MatchFinder *Finder) { + auto EnumWithoutZeroValue = enumType( + hasDeclaration(enumDecl(isCompleteAndHasNoZeroValue()).bind("enum"))); + auto EnumOrArrayOfEnum = qualType(hasUnqualifiedDesugaredType( + anyOf(EnumWithoutZeroValue, + arrayType(hasElementType(qualType( + hasUnqualifiedDesugaredType(EnumWithoutZeroValue))))))); Finder->addMatcher( - expr(isEmptyInit(), - hasType(hasUnqualifiedDesugaredType(enumType(hasDeclaration( - enumDecl(isCompleteAndHasNoZeroValue()).bind("enum")))))) - .bind("expr"), - this); + expr(isEmptyInit(), hasType(EnumOrArrayOfEnum)).bind("expr"), this); + + // Array initialization can contain an "array filler" for the (syntactically) + // unspecified elements. This expression is not found by AST matchers and can + // have any type (the array's element type). This is an implicitly generated + // initialization, so if the type contains somewhere an enum without zero + // enumerator, the zero initialization applies here. We search this array + // element type for the specific enum type manually when this matcher matches. + Finder->addMatcher(initListExpr(hasArrayFiller()).bind("array_filler_expr"), + this); } void InvalidEnumDefaultInitializationCheck::check( const MatchFinder::MatchResult &Result) { const auto *InitExpr = Result.Nodes.getNodeAs<Expr>("expr"); const auto *Enum = Result.Nodes.getNodeAs<EnumDecl>("enum"); + if (!InitExpr) { + const auto *InitList = + Result.Nodes.getNodeAs<InitListExpr>("array_filler_expr"); + // Initialization of omitted array elements with array filler was found. + // Check the type for enum without zero value. + FindEnumMember Finder; + if (!Finder.Visit(InitList->getArrayFiller()->getType().getTypePtr())) + return; + InitExpr = InitList; + Enum = Finder.FoundEnum->getDecl(); + } + if (!InitExpr || !Enum) return; @@ -99,7 +167,8 @@ void InvalidEnumDefaultInitializationCheck::check( } } // If still not found a source location, omit the warning. - // FIXME: All such cases should be fixed to make the checker more precise. + // Ideally all such cases (if they exist) should be handled to make the + // check more precise. if (Loc.isInvalid()) return; } diff --git a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h index c1c21f757ef53..0746c4d025d1f 100644 --- a/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/InvalidEnumDefaultInitializationCheck.h @@ -24,7 +24,6 @@ class InvalidEnumDefaultInitializationCheck : public ClangTidyCheck { ClangTidyContext *Context); void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; - bool isLanguageVersionSupported(const LangOptions &LangOpts) const override; }; } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst index cf24ad4783304..7a020349eedd1 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/invalid-enum-default-initialization.rst @@ -16,7 +16,8 @@ enumerator values). The checker emits a warning only if an enum variable is default-initialized (contrary to not initialized) and the enum type does not have an enumerator with -value of 0. The enum type can be scoped or non-scoped enum. +value of 0. The enum type can be scoped or non-scoped enum. Unions are not +handled by the check (if it contains a member of enum type). .. code-block:: c++ @@ -48,3 +49,24 @@ value of 0. The enum type can be scoped or non-scoped enum. }; S2 VarS2{}; // warn: member 'B' is initialized to 0 + +The check applies to initialization of arrays or structures with initialization +lists in C code too. In these cases elements not specified in the list (and have +enum type) are set to 0. + +.. code-block:: c + + enum Enum1 { + Enum1_A = 1, + Enum1_B + }; + struct Struct1 { + int a; + enum Enum1 b; + }; + + enum Enum1 Array1[2] = {Enum1_A}; // warn: omitted elements are initialized to 0 + enum Enum1 Array2[2][2] = {{Enum1_A}, {Enum1_A}}; // warn: last element of both nested arrays is initialized to 0 + enum Enum1 Array3[2][2] = {{Enum1_A, Enum1_A}}; // warn: elements of second array are initialized to 0 + + struct Struct1 S1 = {1}; // warn: element 'b' is initialized to 0 diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.c new file mode 100644 index 0000000000000..55f588477a65e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.c @@ -0,0 +1,54 @@ +// RUN: %check_clang_tidy %s bugprone-invalid-enum-default-initialization %t + +enum Enum1 { + Enum1_A = 1, + Enum1_B +}; + +struct Struct1 { + int a; + enum Enum1 b; +}; + +struct Struct2 { + struct Struct1 a; + char b; +}; + +enum Enum1 E1 = {}; +// CHECK-NOTES: :[[@LINE-1]]:17: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator +// CHECK-NOTES: :3:6: note: enum is defined here +enum Enum1 E2[10] = {}; +// CHECK-NOTES: :[[@LINE-1]]:21: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator +// CHECK-NOTES: :3:6: note: enum is defined here +enum Enum1 E3[10] = {Enum1_A}; +// CHECK-NOTES: :[[@LINE-1]]:21: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator +// CHECK-NOTES: :3:6: note: enum is defined here +enum Enum1 E4[2][2] = {{Enum1_A}, {Enum1_A}}; +// CHECK-NOTES: :[[@LINE-1]]:24: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator +// CHECK-NOTES: :3:6: note: enum is defined here +// CHECK-NOTES: :[[@LINE-3]]:35: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator +// CHECK-NOTES: :3:6: note: enum is defined here +enum Enum1 E5[2][2] = {{Enum1_A, Enum1_A}}; +// CHECK-NOTES: :[[@LINE-1]]:23: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator +// CHECK-NOTES: :3:6: note: enum is defined here + + +struct Struct1 S1[2][2] = {{{1, Enum1_A}, {2, Enum1_A}}}; +// CHECK-NOTES: :[[@LINE-1]]:27: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator +// CHECK-NOTES: :3:6: note: enum is defined here + +struct Struct2 S2[3] = {{1}}; +// CHECK-NOTES: :[[@LINE-1]]:24: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator +// CHECK-NOTES: :3:6: note: enum is defined here +// CHECK-NOTES: :[[@LINE-3]]:26: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator +// CHECK-NOTES: :3:6: note: enum is defined here + +union Union1 { + enum Enum1 a; + int b; +}; + +// no warnings for union +union Union1 U1 = {}; +union Union1 U2[3] = {}; diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp index b4e2d5367fea7..eb3d5632eaef7 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/invalid-enum-default-initialization.cpp @@ -57,9 +57,13 @@ void f2() { // CHECK-NOTES: :[[@LINE-1]]:13: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator // CHECK-NOTES: :8:12: note: enum is defined here Enum1 C[5] = {{}}; - // CHECK-NOTES: :[[@LINE-1]]:17: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator + // CHECK-NOTES: :[[@LINE-1]]:16: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator + // CHECK-NOTES: :8:12: note: enum is defined here + // CHECK-NOTES: :[[@LINE-3]]:17: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator // CHECK-NOTES: :8:12: note: enum is defined here Enum1 D[5] = {}; // FIMXE: warn for this? + // CHECK-NOTES: :[[@LINE-1]]:16: warning: enum value of type 'Enum1' initialized with invalid value of 0, enum doesn't have a zero-value enumerator + // CHECK-NOTES: :8:12: note: enum is defined here } struct S1 { @@ -99,6 +103,11 @@ struct S4 : public S3 { int Z; }; +struct S5 { + S2 X[3]; + int Y; +}; + S2 VarS2{}; // CHECK-NOTES: :[[@LINE-1]]:9: warning: enum value of type 'Enum1' initialized with invalid value of 0 // CHECK-NOTES: :8:12: note: enum is defined here @@ -114,6 +123,9 @@ S4 VarS4{}; // CHECK-NOTES: :8:12: note: enum is defined here // CHECK-NOTES: :[[@LINE-3]]:10: warning: enum value of type 'Enum2' initialized with invalid value of 0 // CHECK-NOTES: :13:6: note: enum is defined here +S5 VarS5{}; +// CHECK-NOTES: :[[@LINE-1]]:10: warning: enum value of type 'Enum1' initialized with invalid value of 0 +// CHECK-NOTES: :8:12: note: enum is defined here enum class EnumFwd; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits