[PATCH] D71686: Fix false positive in magic number checker

[Florin Iucha via Phabricator via cfe-commits]

0x8000-0000 updated this revision to Diff 234818.
0x8000-0000 edited the summary of this revision.
0x8000-0000 added a comment.

Update release notes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71686/new/

https://reviews.llvm.org/D71686

Files:
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  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
+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/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -185,6 +185,10 @@
   The check now supports the ``IgnoreBitFieldsWidths`` option to suppress
   the warning for numbers used to specify bit field widths.
 
+  The check was updated to eliminate some false positives (such as using
+  class enumeration as non-type template parameters, or the syntethically
+  computed lengh of a static user string literal.)
+
 - New :doc:`readability-make-member-function-const
   <clang-tidy/checks/readability-make-member-function-const>` check.
 
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 {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D71686.234818.patch
Type: text/x-patch
Size: 4110 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191220/2c96ad92/attachment.bin>