florianhumblot created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, jeroen.dobbelaere, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
florianhumblot requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D145780
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,16 +97,15 @@
// 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
*/
-/*
- * No warning for type aliases
- */
-using NumberInTypeAlias = ValueBucket<int, 25>;
-typedef ValueBucket<char, 243> NumberInTypedef;
#define INT_MACRO 5
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
@@ -137,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
@@ -193,8 +193,9 @@
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. The check now allows for
- magic numbers in type aliases such as ``using`` and ``typedef`` declarations.
+ <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 false.
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,7 @@
#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"
@@ -37,12 +38,21 @@
if (Node.get<EnumConstantDecl>())
return true;
+ return llvm::any_of(Result.Context->getParents(Node),
+ [&Result](const DynTypedNode &Parent) {
+ return isUsedToInitializeAConstant(Result, Parent);
+ });
+}
+
+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 isUsedToInitializeAConstant(Result, Parent);
+ return isUsedToDefineATypeAlias(Result, Parent);
});
}
@@ -70,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(
@@ -118,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);
@@ -141,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
