Optionally exclude bitfield definitions from magic numbers check

Adds the IgnoreBitFieldsWidths option to readability-magic-numbers.

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

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 <typename L>
   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;

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.
 

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`.

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]
+};
+

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
  */