[clang-tools-extra] d4b45b9 - [clang-tidy] Change readability-magic-numbers to allow numbers in type aliases.

Piotr Zegar via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 10 06:00:40 PST 2023


Author: Florian Humblot
Date: 2023-03-10T13:56:28Z
New Revision: d4b45b93fa56622ee5f935c066f43b15b856f3b8

URL: https://github.com/llvm/llvm-project/commit/d4b45b93fa56622ee5f935c066f43b15b856f3b8
DIFF: https://github.com/llvm/llvm-project/commit/d4b45b93fa56622ee5f935c066f43b15b856f3b8.diff

LOG: [clang-tidy] Change readability-magic-numbers to allow numbers in type aliases.

This commit introduces an option to the readability-magic-values checker.
The option defaults to false so that the behavior of the checker doesn't change unless specifically enabled.
These commits are supposed to fix https://github.com/llvm/llvm-project/issues/61259

Reviewed By: PiotrZSL

Differential Revision: https://reviews.llvm.org/D145778

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
    clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
    clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
index 64940ede8e1c7..ca81b6e3c6e61 100644
--- a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
@@ -14,6 +14,8 @@
 #include "MagicNumbersCheck.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Type.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "llvm/ADT/STLExtras.h"
 #include <algorithm>
@@ -42,6 +44,18 @@ static bool isUsedToInitializeAConstant(const MatchFinder::MatchResult &Result,
                       });
 }
 
+static bool isUsedToDefineATypeAlias(const MatchFinder::MatchResult &Result,
+                                     const DynTypedNode &Node) {
+
+  if (Node.get<TypeAliasDecl>() || Node.get<TypedefNameDecl>())
+    return true;
+
+  return llvm::any_of(Result.Context->getParents(Node),
+                      [&Result](const DynTypedNode &Parent) {
+                        return isUsedToDefineATypeAlias(Result, Parent);
+                      });
+}
+
 static bool isUsedToDefineABitField(const MatchFinder::MatchResult &Result,
                                     const DynTypedNode &Node) {
   const auto *AsFieldDecl = Node.get<FieldDecl>();
@@ -66,6 +80,7 @@ MagicNumbersCheck::MagicNumbersCheck(StringRef Name, ClangTidyContext *Context)
       IgnoreBitFieldsWidths(Options.get("IgnoreBitFieldsWidths", true)),
       IgnorePowersOf2IntegerValues(
           Options.get("IgnorePowersOf2IntegerValues", false)),
+      IgnoreTypeAliases(Options.get("IgnoreTypeAliases", false)),
       RawIgnoredIntegerValues(
           Options.get("IgnoredIntegerValues", DefaultIgnoredIntegerValues)),
       RawIgnoredFloatingPointValues(Options.get(
@@ -114,6 +129,7 @@ void MagicNumbersCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IgnoreBitFieldsWidths", IgnoreBitFieldsWidths);
   Options.store(Opts, "IgnorePowersOf2IntegerValues",
                 IgnorePowersOf2IntegerValues);
+  Options.store(Opts, "IgnoreTypeAliases", IgnoreTypeAliases);
   Options.store(Opts, "IgnoredIntegerValues", RawIgnoredIntegerValues);
   Options.store(Opts, "IgnoredFloatingPointValues",
                 RawIgnoredFloatingPointValues);
@@ -137,10 +153,13 @@ bool MagicNumbersCheck::isConstant(const MatchFinder::MatchResult &Result,
                                    const Expr &ExprResult) const {
   return llvm::any_of(
       Result.Context->getParents(ExprResult),
-      [&Result](const DynTypedNode &Parent) {
+      [this, &Result](const DynTypedNode &Parent) {
         if (isUsedToInitializeAConstant(Result, Parent))
           return true;
 
+        if (IgnoreTypeAliases && isUsedToDefineATypeAlias(Result, Parent))
+          return true;
+
         // Ignore this instance, because this matches an
         // expanded class enumeration value.
         if (Parent.get<CStyleCastExpr>() &&

diff  --git a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
index ba39f17f4d500..39e9baea3adc8 100644
--- a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
@@ -84,6 +84,7 @@ class MagicNumbersCheck : public ClangTidyCheck {
   const bool IgnoreAllFloatingPointValues;
   const bool IgnoreBitFieldsWidths;
   const bool IgnorePowersOf2IntegerValues;
+  const bool IgnoreTypeAliases;
   const StringRef RawIgnoredIntegerValues;
   const StringRef RawIgnoredFloatingPointValues;
 

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 426b92ab950cc..e8a74f8402460 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,6 +192,11 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/too-small-loop-variable>` check. Basic support
   for bit-field and integer members as a loop variable or upper limit were added.
 
+- Improved :doc:`readability-magic-numbers 
+  <clang-tidy/checks/readability/magic-numbers>` check, now allows for
+  magic numbers in type aliases such as ``using`` and ``typedef`` declarations if
+  the new ``IgnoreTypeAliases`` option is set to true.
+
 Removed checks
 ^^^^^^^^^^^^^^
 

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
index b00ddd149cece..b5017f81d0a59 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
@@ -24,6 +24,16 @@ Examples of magic values:
 
 .. code-block:: c++
 
+   template<typename T, size_t N>
+   struct CustomType {
+      T arr[N];
+   };
+
+   struct OtherType {
+      CustomType<int, 30> container;
+   }
+   CustomType<int, 30> values;
+
    double circleArea = 3.1415926535 * radius * radius;
 
    double totalCharge = 1.08 * itemPrice;
@@ -40,6 +50,19 @@ Example with magic values refactored:
 
 .. code-block:: c++
 
+   template<typename T, size_t N>
+   struct CustomType {
+      T arr[N];
+   };
+
+   const size_t NUMBER_OF_ELEMENTS = 30;
+   using containerType = CustomType<int, NUMBER_OF_ELEMENTS>;
+   
+   struct OtherType {
+      containerType container;
+   }
+   containerType values;
+
    double circleArea = M_PI * radius * radius;
 
    const double TAX_RATE = 0.08;  // or make it variable and read from a file
@@ -116,3 +139,8 @@ Options
    Boolean value indicating whether to accept magic numbers as bit field widths
    without warning. This is useful for example for register definitions which
    are generated from hardware specifications. Default value is `true`.
+
+.. option:: IgnoreTypeAliases
+
+   Boolean value indicating whether to accept magic numbers in ``typedef`` or
+   ``using`` declarations. Default value is `false`.

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
index 638b0183d8242..57547177f5230 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
@@ -3,7 +3,8 @@
 // RUN:  [{key: readability-magic-numbers.IgnoredIntegerValues, value: "0;1;2;10;100;"}, \
 // RUN:   {key: readability-magic-numbers.IgnoredFloatingPointValues, value: "3.14;2.71828;9.81;10000.0;101.0;0x1.2p3"}, \
 // RUN:   {key: readability-magic-numbers.IgnoreBitFieldsWidths, value: false}, \
-// RUN:   {key: readability-magic-numbers.IgnorePowersOf2IntegerValues, value: true}]}' \
+// RUN:   {key: readability-magic-numbers.IgnorePowersOf2IntegerValues, value: true}, \
+// RUN:   {key: readability-magic-numbers.IgnoreTypeAliases, value: false}]}' \
 // RUN: --
 
 template <typename T, int V>
@@ -96,6 +97,10 @@ struct HardwareGateway {
    // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 3 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
 };
 
+using NumberInTypeAlias = ValueBucket<int, 25>;
+// CHECK-MESSAGES: :[[@LINE-1]]:44: warning: 25 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+typedef ValueBucket<char, 243> NumberInTypedef;
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 243 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
 
 /*
  * Clean code


        


More information about the cfe-commits mailing list