florianhumblot updated this revision to Diff 504089.
florianhumblot edited the summary of this revision.
florianhumblot added a comment.
Made it so that the proposed change doesn't change the default behavior of the
check.
Introduce an option to allow enabling the exclusion.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145778/new/
https://reviews.llvm.org/D145778
Files:
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-type-aliases.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
@@ -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 @@
// 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
Index: clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-type-aliases.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-type-aliases.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s readability-magic-numbers %t \
+// RUN: -config='{CheckOptions: \
+// RUN: [{key: readability-magic-numbers.IgnoreTypeAliases, value: true}]}' \
+// RUN: --
+
+template <typename T, int V>
+struct ValueBucket {
+ T value[V];
+};
+
+int BadGlobalInt = 5;
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 5 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+
+/**
+ * With the flag enabled these should produce no warning
+ */
+using NumberInTypeAlias = ValueBucket<int, 25>;
+typedef ValueBucket<char, 243> NumberInTypedef;
Index: clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
@@ -24,6 +24,16 @@
.. 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,17 @@
.. code-block:: c++
+ template<typename T, size_t N>
+ struct CustomType {
+ T arr[N];
+ };
+
+ using containerType = CustomType<int, 30>;
+ 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 +137,8 @@
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`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,6 +192,11 @@
<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
^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
+++ clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
@@ -84,6 +84,7 @@
const bool IgnoreAllFloatingPointValues;
const bool IgnoreBitFieldsWidths;
const bool IgnorePowersOf2IntegerValues;
+ const bool IgnoreTypeAliases;
const StringRef RawIgnoredIntegerValues;
const StringRef RawIgnoredFloatingPointValues;
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
@@ -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 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 @@
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 @@
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 @@
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>() &&
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
