[clang-tidy] Add check 'bugprone-invalid-enum-default-initialization' #136823
+337
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: Balázs Kéri (balazske) ChangesFull diff: https://github.com/llvm/llvm-project/pull/136823.diff 6 Files Affected:
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
|
|
See also |
PiotrZSL
reviewed
Apr 23, 2025
| @@ -0,0 +1,116 @@ | |||
| // RUN: %check_clang_tidy -std=c++17 %s bugprone-invalid-enum-default-initialization %t | |||
|
|
|||
There was a problem hiding this comment.
Add test with enum forward declaration only, and with enum without enumerators.
For those there should be no requirement in initialization.
|
I wonder if behavior of this check should be an extension to optin.core.EnumCastOutOfRange. Sure, we don't have |
| if (!InitExpr || !Enum) | ||
| return; | ||
|
|
||
| ASTContext &ACtx = Enum->getASTContext(); |
There was a problem hiding this comment.
const? Same for Parents below.
There was a problem hiding this comment.
getParents requires it to be non-const.
| bugprone-invalid-enum-default-initialization | ||
| ============================================ | ||
|
|
||
| Detect default initialization (to 0) of variables with `enum` type where |
There was a problem hiding this comment.
Suggested change
| Detect default initialization (to 0) of variables with `enum` type where | |
| Detect default initialization (to 0) of variables with ``enum`` type where |
|
There's also a check in UBSAN which detects assignment of invalid values during run-time. So there's logic is various other places regarding this. |
This check is implemented as an AST based check that is different from the static analyzer check, even if there is a similarity in the found problems. (It can be possible to add this check as not AST-based static analyzer check but the results would be likely not as exact.) Turning on an option in an existing checker is not much different than turning on another checker. |
| @@ -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 | |||
There was a problem hiding this comment.
Please keep alphabetical order (by check name) in this list.
| - 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 |
There was a problem hiding this comment.
Suggested change
| Detect default initialization (to 0) of variables with ``enum`` type where | |
| Detects default initialization (to 0) of variables with ``enum`` type where |
| @@ -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 | |||
There was a problem hiding this comment.
Suggested change
| Detect default initialization (to 0) of variables with ``enum`` type where | |
| Detects default initialization (to 0) of variables with ``enum`` type where |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.