Author: Florin Iucha Date: 2019-12-07T12:33:10-05:00 New Revision: 6dcb1003f2022cba36e9f5a6d39648c3a3a213a0
URL: https://github.com/llvm/llvm-project/commit/6dcb1003f2022cba36e9f5a6d39648c3a3a213a0 DIFF: https://github.com/llvm/llvm-project/commit/6dcb1003f2022cba36e9f5a6d39648c3a3a213a0.diff LOG: Optionally exclude bitfield definitions from magic numbers check Adds the IgnoreBitFieldsWidths option to readability-magic-numbers. Added: clang-tools-extra/test/clang-tidy/checkers/readability-magic-numbers-bitfields.cpp 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 39aaf89901f2..6f6366cab6f6 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<clang::DeclaratorDecl>(); + const auto *AsDecl = Node.get<DeclaratorDecl>(); if (AsDecl) { if (AsDecl->getType().isConstQualified()) return true; @@ -34,7 +34,7 @@ bool isUsedToInitializeAConstant(const MatchFinder::MatchResult &Result, return AsDecl->isImplicit(); } - if (Node.get<clang::EnumConstantDecl>() != nullptr) + if (Node.get<EnumConstantDecl>() != 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<FieldDecl>(); + 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 e59ca1759670..0cf7419d703c 100644 --- a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h +++ b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h @@ -40,10 +40,17 @@ class MagicNumbersCheck : public ClangTidyCheck { 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 <typename L> void checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result, const char *BoundName) { @@ -64,6 +71,9 @@ class MagicNumbersCheck : public ClangTidyCheck { 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 @@ class MagicNumbersCheck : public ClangTidyCheck { } 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 91a196deb6f4..ec56c6d6a784 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 + <clang-tidy/checks/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 <clang-tidy/checks/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 9968809eef6d..1fac4220abea 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 000000000000..3c1fef939c63 --- /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 3cf9dc52ea2b..055ad76d09b5 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 */ _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
