From 6dcb1003f2022cba36e9f5a6d39648c3a3a213a0 Mon Sep 17 00:00:00 2001 From: Florin Iucha Date: Sat, 7 Dec 2019 12:33:10 -0500 Subject: [PATCH] Optionally exclude bitfield definitions from magic numbers check Adds the IgnoreBitFieldsWidths option to readability-magic-numbers. --- .../clang-tidy/readability/MagicNumbersCheck.cpp | 34 +++++++++++++++++----- .../clang-tidy/readability/MagicNumbersCheck.h | 13 ++++++++- clang-tools-extra/docs/ReleaseNotes.rst | 6 ++++ .../checks/readability-magic-numbers.rst | 5 ++++ .../readability-magic-numbers-bitfields.cpp | 22 ++++++++++++++ .../checkers/readability-magic-numbers.cpp | 18 ++++++++++++ 6 files changed, 90 insertions(+), 8 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability-magic-numbers-bitfields.cpp diff --git a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp index 39aaf89..6f6366c 100644 --- a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp @@ -21,12 +21,12 @@ using namespace clang::ast_matchers; using namespace clang::ast_type_traits; -namespace { +namespace clang { -bool isUsedToInitializeAConstant(const MatchFinder::MatchResult &Result, - const DynTypedNode &Node) { +static bool isUsedToInitializeAConstant(const MatchFinder::MatchResult &Result, + const DynTypedNode &Node) { - const auto *AsDecl = Node.get(); + const auto *AsDecl = Node.get(); if (AsDecl) { if (AsDecl->getType().isConstQualified()) return true; @@ -34,7 +34,7 @@ bool isUsedToInitializeAConstant(const MatchFinder::MatchResult &Result, return AsDecl->isImplicit(); } - if (Node.get() != nullptr) + if (Node.get() != nullptr) return true; return llvm::any_of(Result.Context->getParents(Node), @@ -43,9 +43,18 @@ bool isUsedToInitializeAConstant(const MatchFinder::MatchResult &Result, }); } -} // namespace +static bool isUsedToDefineABitField(const MatchFinder::MatchResult &Result, + const DynTypedNode &Node) { + const auto *AsFieldDecl = Node.get(); + if (AsFieldDecl && AsFieldDecl->isBitField()) + return true; + + return llvm::any_of(Result.Context->getParents(Node), + [&Result](const DynTypedNode &Parent) { + return isUsedToDefineABitField(Result, Parent); + }); +} -namespace clang { namespace tidy { namespace readability { @@ -56,6 +65,7 @@ MagicNumbersCheck::MagicNumbersCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), IgnoreAllFloatingPointValues( Options.get("IgnoreAllFloatingPointValues", false)), + IgnoreBitFieldsWidths(Options.get("IgnoreBitFieldsWidths", true)), IgnorePowersOf2IntegerValues( Options.get("IgnorePowersOf2IntegerValues", false)) { // Process the set of ignored integer values. @@ -165,6 +175,16 @@ bool MagicNumbersCheck::isSyntheticValue(const SourceManager *SourceManager, return BufferIdentifier.empty(); } +bool MagicNumbersCheck::isBitFieldWidth( + const clang::ast_matchers::MatchFinder::MatchResult &Result, + const IntegerLiteral &Literal) const { + return IgnoreBitFieldsWidths && + llvm::any_of(Result.Context->getParents(Literal), + [&Result](const DynTypedNode &Parent) { + return isUsedToDefineABitField(Result, Parent); + }); +} + } // namespace readability } // namespace tidy } // namespace clang diff --git a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h index e59ca17..0cf7419 100644 --- a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h +++ b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h @@ -40,10 +40,17 @@ private: const FloatingLiteral *) const { return false; } - bool isSyntheticValue(const clang::SourceManager *SourceManager, const IntegerLiteral *Literal) const; + bool isBitFieldWidth(const clang::ast_matchers::MatchFinder::MatchResult &, + const FloatingLiteral &) const { + return false; + } + + bool isBitFieldWidth(const clang::ast_matchers::MatchFinder::MatchResult &Result, + const IntegerLiteral &Literal) const; + template void checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result, const char *BoundName) { @@ -64,6 +71,9 @@ private: if (isSyntheticValue(Result.SourceManager, MatchedLiteral)) return; + if (isBitFieldWidth(Result, *MatchedLiteral)) + return; + const StringRef LiteralSourceText = Lexer::getSourceText( CharSourceRange::getTokenRange(MatchedLiteral->getSourceRange()), *Result.SourceManager, getLangOpts()); @@ -74,6 +84,7 @@ private: } const bool IgnoreAllFloatingPointValues; + const bool IgnoreBitFieldsWidths; const bool IgnorePowersOf2IntegerValues; constexpr static unsigned SensibleNumberOfMagicValueExceptions = 16; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 91a196d..ec56c6d 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -173,6 +173,12 @@ Improvements to clang-tidy Finds classes, structs, and unions that contain redundant member access specifiers. +- Improved :doc:`readability-magic-numbers + ` check. + + The check now supports the ``IgnoreBitFieldsWidths`` option to suppress + the warning for numbers used to specify bit field widths. + - New :doc:`readability-make-member-function-const ` check. 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 9968809..1fac422 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 @@ -111,3 +111,8 @@ Options Boolean value indicating whether to accept all floating point values without warning. Default value is `false`. +.. option:: IgnoreBitFieldsWidths + + 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`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-magic-numbers-bitfields.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-magic-numbers-bitfields.cpp new file mode 100644 index 0000000..3c1fef9 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability-magic-numbers-bitfields.cpp @@ -0,0 +1,22 @@ +// RUN: %check_clang_tidy %s readability-magic-numbers %t \ +// RUN: -config='{CheckOptions: \ +// RUN: [{key: readability-magic-numbers.IgnoredIntegerValues, value: "1;2;10;100;"}]}' \ +// RUN: -- + +struct HardwareGateway { + /* + * The configuration suppresses the warnings for the bitfields... + */ + unsigned int Some: 5; + unsigned int Bits: 7; + unsigned int: 7; + unsigned int: 0; + unsigned int Rest: 13; + + /* + * ... but other fields trigger the warning. + */ + unsigned int Another[3]; + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 3 is a magic number; consider replacing it with a named constant [readability-magic-numbers] +}; + 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 3cf9dc5..055ad76 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 @@ -2,6 +2,7 @@ // RUN: -config='{CheckOptions: \ // 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: 0}, \ // RUN: {key: readability-magic-numbers.IgnorePowersOf2IntegerValues, value: 1}]}' \ // RUN: -- @@ -79,6 +80,23 @@ int getAnswer() { // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 3 is a magic number; consider replacing it with a named constant [readability-magic-numbers] } +struct HardwareGateway { + unsigned int Some: 5; + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 5 is a magic number; consider replacing it with a named constant [readability-magic-numbers] + unsigned int Bits: 7; + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 7 is a magic number; consider replacing it with a named constant [readability-magic-numbers] + unsigned int: 6; + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 6 is a magic number; consider replacing it with a named constant [readability-magic-numbers] + unsigned int Flag: 1; // no warning since this is suppressed by IgnoredIntegerValues rule + unsigned int: 0; // no warning since this is suppressed by IgnoredIntegerValues rule + unsigned int Rest: 13; + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 13 is a magic number; consider replacing it with a named constant [readability-magic-numbers] + // + unsigned int Another[3]; + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 3 is a magic number; consider replacing it with a named constant [readability-magic-numbers] +}; + + /* * Clean code */ -- 2.7.4