-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[clang-tidy] Add check 'bugprone-invalid-enum-default-initialization' #136823
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
4ce7497
e5744a8
6920e31
9620955
cea08d8
b726e03
05f3e6e
c7fa29f
d164305
26c0a34
c7615e0
245d7b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| 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 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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; | ||
| }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is relevant for C, C++ and Objective-C languages as well, so I think you are right to not override the isLanguageVersionSupported method here.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| } // namespace clang::tidy::bugprone | ||
|
|
||
| #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INVALIDENUMDEFAULTINITIALIZATIONCHECK_H | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -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 | ||||||
|
||||||
| Detect default initialization (to 0) of variables with `enum` type where | |
| Detect default initialization (to 0) of variables with ``enum`` type where |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| value 0. In this way a variable with the enum type may contain initially an | |
| value 0. In this way a variable with the ``enum`` type may contain initially an |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| // RUN: %check_clang_tidy -std=c++17 %s bugprone-invalid-enum-default-initialization %t | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add test with enum forward declaration only, and with enum without enumerators.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are added now. |
||
| 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 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this check also valuable for C?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check was improved to handle initializer lists that can be used in C code too.