0x8000-0000 created this revision.
0x8000-0000 added a reviewer: aaron.ballman.
0x8000-0000 added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Fix false positive in magic number checker
https://bugs.llvm.org/show_bug.cgi?id=40640:
cppcoreguidelines-avoid-magic-numbers should not warn about enum class
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D71686
Files:
clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-magic-numbers.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-magic-numbers.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-magic-numbers.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-magic-numbers.cpp
@@ -215,3 +215,14 @@
return Total;
}
+
+// prove that using enumerations values don't produce warnings (code by Pavel
Kryukov)
+enum class Letter : unsigned {
+ A, B, C, D, E, F, G, H, I, J
+};
+
+template<Letter x> struct holder { Letter letter = x; };
+template<Letter x> struct wrapper { using h_type = holder<x>; };
+
+template struct wrapper<Letter::A>;
+template struct wrapper<Letter::J>;
Index: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
@@ -34,7 +34,7 @@
return AsDecl->isImplicit();
}
- if (Node.get<EnumConstantDecl>() != nullptr)
+ if (Node.get<EnumConstantDecl>())
return true;
return llvm::any_of(Result.Context->getParents(Node),
@@ -119,25 +119,31 @@
bool MagicNumbersCheck::isConstant(const MatchFinder::MatchResult &Result,
const Expr &ExprResult) const {
- return llvm::any_of(
- Result.Context->getParents(ExprResult),
- [&Result](const DynTypedNode &Parent) {
- if (isUsedToInitializeAConstant(Result, Parent))
- return true;
-
- // Ignore this instance, because this match reports the location
- // where the template is defined, not where it is instantiated.
- if (Parent.get<SubstNonTypeTemplateParmExpr>())
- return true;
-
- // Don't warn on string user defined literals:
- // std::string s = "Hello World"s;
- if (const auto *UDL = Parent.get<UserDefinedLiteral>())
- if (UDL->getLiteralOperatorKind() == UserDefinedLiteral::LOK_String)
- return true;
-
- return false;
- });
+ return llvm::any_of(Result.Context->getParents(ExprResult),
+ [&Result](const DynTypedNode &Parent) {
+ if (isUsedToInitializeAConstant(Result, Parent))
+ return true;
+
+ // Ignore this instance, because this matches an
+ // expanded class enumeration value.
+ if (Parent.get<CStyleCastExpr>())
+ return true;
+
+ // Ignore this instance, because this match reports the
+ // location where the template is defined, not where it
+ // is instantiated.
+ if (Parent.get<SubstNonTypeTemplateParmExpr>())
+ return true;
+
+ // Don't warn on string user defined literals:
+ // std::string s = "Hello World"s;
+ if (const auto *UDL = Parent.get<UserDefinedLiteral>())
+ if (UDL->getLiteralOperatorKind() ==
+ UserDefinedLiteral::LOK_String)
+ return true;
+
+ return false;
+ });
}
bool MagicNumbersCheck::isIgnoredValue(const IntegerLiteral *Literal) const {
Index: clang-tools-extra/test/clang-tidy/checkers/readability-magic-numbers.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-magic-numbers.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-magic-numbers.cpp
@@ -215,3 +215,14 @@
return Total;
}
+
+// prove that using enumerations values don't produce warnings (code by Pavel Kryukov)
+enum class Letter : unsigned {
+ A, B, C, D, E, F, G, H, I, J
+};
+
+template<Letter x> struct holder { Letter letter = x; };
+template<Letter x> struct wrapper { using h_type = holder<x>; };
+
+template struct wrapper<Letter::A>;
+template struct wrapper<Letter::J>;
Index: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
@@ -34,7 +34,7 @@
return AsDecl->isImplicit();
}
- if (Node.get<EnumConstantDecl>() != nullptr)
+ if (Node.get<EnumConstantDecl>())
return true;
return llvm::any_of(Result.Context->getParents(Node),
@@ -119,25 +119,31 @@
bool MagicNumbersCheck::isConstant(const MatchFinder::MatchResult &Result,
const Expr &ExprResult) const {
- return llvm::any_of(
- Result.Context->getParents(ExprResult),
- [&Result](const DynTypedNode &Parent) {
- if (isUsedToInitializeAConstant(Result, Parent))
- return true;
-
- // Ignore this instance, because this match reports the location
- // where the template is defined, not where it is instantiated.
- if (Parent.get<SubstNonTypeTemplateParmExpr>())
- return true;
-
- // Don't warn on string user defined literals:
- // std::string s = "Hello World"s;
- if (const auto *UDL = Parent.get<UserDefinedLiteral>())
- if (UDL->getLiteralOperatorKind() == UserDefinedLiteral::LOK_String)
- return true;
-
- return false;
- });
+ return llvm::any_of(Result.Context->getParents(ExprResult),
+ [&Result](const DynTypedNode &Parent) {
+ if (isUsedToInitializeAConstant(Result, Parent))
+ return true;
+
+ // Ignore this instance, because this matches an
+ // expanded class enumeration value.
+ if (Parent.get<CStyleCastExpr>())
+ return true;
+
+ // Ignore this instance, because this match reports the
+ // location where the template is defined, not where it
+ // is instantiated.
+ if (Parent.get<SubstNonTypeTemplateParmExpr>())
+ return true;
+
+ // Don't warn on string user defined literals:
+ // std::string s = "Hello World"s;
+ if (const auto *UDL = Parent.get<UserDefinedLiteral>())
+ if (UDL->getLiteralOperatorKind() ==
+ UserDefinedLiteral::LOK_String)
+ return true;
+
+ return false;
+ });
}
bool MagicNumbersCheck::isIgnoredValue(const IntegerLiteral *Literal) const {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
