Add a new check to the readability module that flags uses of "magic numbers" (both floating-point and integral).

Patch by Florin Iucha <florin@signbit.net>


git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@339516 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 272 insertions, 0 deletions

diff --git a/clang-tidy/readability/CMakeLists.txt b/clang-tidy/readability/CMakeLists.txt
index 961ad88a..e7f56c55 100644
--- a/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tidy/readability/CMakeLists.txt
@@ -11,6 +11,7 @@ add_clang_library(clangTidyReadabilityModule
   IdentifierNamingCheck.cpp
   ImplicitBoolConversionCheck.cpp
   InconsistentDeclarationParameterNameCheck.cpp
+  MagicNumbersCheck.cpp
   MisleadingIndentationCheck.cpp
   MisplacedArrayIndexCheck.cpp
   NamedParameterCheck.cpp
diff --git a/clang-tidy/readability/MagicNumbersCheck.cpp b/clang-tidy/readability/MagicNumbersCheck.cpp
new file mode 100644
index 00000000..08021f32
--- /dev/null
+++ b/clang-tidy/readability/MagicNumbersCheck.cpp
@@ -0,0 +1,171 @@
+//===--- MagicNumbersCheck.cpp - clang-tidy-------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// A checker for magic numbers: integer or floating point literals embedded
+// in the code, outside the definition of a constant or an enumeration.
+//
+//===----------------------------------------------------------------------===//
+
+#include "MagicNumbersCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/STLExtras.h"
+#include <algorithm>
+
+using namespace clang::ast_matchers;
+using namespace clang::ast_type_traits;
+
+namespace {
+
+bool isUsedToInitializeAConstant(const MatchFinder::MatchResult &Result,
+                                 const DynTypedNode &Node) {
+
+  const auto *AsDecl = Node.get<clang::DeclaratorDecl>();
+  if (AsDecl) {
+    if (AsDecl->getType().isConstQualified())
+      return true;
+
+    return AsDecl->isImplicit();
+  }
+
+  if (Node.get<clang::EnumConstantDecl>() != nullptr)
+    return true;
+
+  return llvm::any_of(Result.Context->getParents(Node),
+                      [&Result](const DynTypedNode &Parent) {
+                        return isUsedToInitializeAConstant(Result, Parent);
+                      });
+}
+
+} // namespace
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+const char DefaultIgnoredIntegerValues[] = "1;2;3;4;";
+const char DefaultIgnoredFloatingPointValues[] = "1.0;100.0;";
+
+MagicNumbersCheck::MagicNumbersCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      IgnoreAllFloatingPointValues(
+          Options.get("IgnoreAllFloatingPointValues", false)),
+      IgnorePowersOf2IntegerValues(
+          Options.get("IgnorePowersOf2IntegerValues", false)) {
+  // Process the set of ignored integer values.
+  const std::vector<std::string> IgnoredIntegerValuesInput =
+      utils::options::parseStringList(
+          Options.get("IgnoredIntegerValues", DefaultIgnoredIntegerValues));
+  IgnoredIntegerValues.resize(IgnoredIntegerValuesInput.size());
+  llvm::transform(IgnoredIntegerValuesInput, IgnoredIntegerValues.begin(),
+                  [](const std::string &Value) { return std::stoll(Value); });
+  llvm::sort(IgnoredIntegerValues.begin(), IgnoredIntegerValues.end());
+
+  if (!IgnoreAllFloatingPointValues) {
+    // Process the set of ignored floating point values.
+    const std::vector<std::string> IgnoredFloatingPointValuesInput =
+        utils::options::parseStringList(Options.get(
+            "IgnoredFloatingPointValues", DefaultIgnoredFloatingPointValues));
+    IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size());
+    IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size());
+    for (const auto &InputValue : IgnoredFloatingPointValuesInput) {
+      llvm::APFloat FloatValue(llvm::APFloat::IEEEsingle());
+      FloatValue.convertFromString(InputValue, DefaultRoundingMode);
+      IgnoredFloatingPointValues.push_back(FloatValue.convertToFloat());
+
+      llvm::APFloat DoubleValue(llvm::APFloat::IEEEdouble());
+      DoubleValue.convertFromString(InputValue, DefaultRoundingMode);
+      IgnoredDoublePointValues.push_back(DoubleValue.convertToDouble());
+    }
+    llvm::sort(IgnoredFloatingPointValues.begin(),
+               IgnoredFloatingPointValues.end());
+    llvm::sort(IgnoredDoublePointValues.begin(),
+               IgnoredDoublePointValues.end());
+  }
+}
+
+void MagicNumbersCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoredIntegerValues", DefaultIgnoredIntegerValues);
+  Options.store(Opts, "IgnoredFloatingPointValues",
+                DefaultIgnoredFloatingPointValues);
+}
+
+void MagicNumbersCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(integerLiteral().bind("integer"), this);
+  if (!IgnoreAllFloatingPointValues)
+    Finder->addMatcher(floatLiteral().bind("float"), this);
+}
+
+void MagicNumbersCheck::check(const MatchFinder::MatchResult &Result) {
+  checkBoundMatch<IntegerLiteral>(Result, "integer");
+  checkBoundMatch<FloatingLiteral>(Result, "float");
+}
+
+bool MagicNumbersCheck::isConstant(const MatchFinder::MatchResult &Result,
+                                   const Expr &ExprResult) const {
+  return llvm::any_of(
+      Result.Context->getParents(ExprResult),
+      [&Result](const DynTypedNode &Parent) {
+        return isUsedToInitializeAConstant(Result, Parent) ||
+               // Ignore this instance, because this match reports the location
+               // where the template is defined, not where it is instantiated.
+               Parent.get<SubstNonTypeTemplateParmExpr>();
+      });
+}
+
+bool MagicNumbersCheck::isIgnoredValue(const IntegerLiteral *Literal) const {
+  const llvm::APInt IntValue = Literal->getValue();
+  const int64_t Value = IntValue.getZExtValue();
+  if (Value == 0)
+    return true;
+
+  if (IgnorePowersOf2IntegerValues && IntValue.isPowerOf2())
+    return true;
+
+  return std::binary_search(IgnoredIntegerValues.begin(),
+                            IgnoredIntegerValues.end(), Value);
+}
+
+bool MagicNumbersCheck::isIgnoredValue(const FloatingLiteral *Literal) const {
+  const llvm::APFloat FloatValue = Literal->getValue();
+  if (FloatValue.isZero())
+    return true;
+
+  if (&FloatValue.getSemantics() == &llvm::APFloat::IEEEsingle()) {
+    const float Value = FloatValue.convertToFloat();
+    return std::binary_search(IgnoredFloatingPointValues.begin(),
+                              IgnoredFloatingPointValues.end(), Value);
+  }
+
+  if (&FloatValue.getSemantics() == &llvm::APFloat::IEEEdouble()) {
+    const double Value = FloatValue.convertToDouble();
+    return std::binary_search(IgnoredDoublePointValues.begin(),
+                              IgnoredDoublePointValues.end(), Value);
+  }
+
+  return false;
+}
+
+bool MagicNumbersCheck::isSyntheticValue(const SourceManager *SourceManager,
+                                         const IntegerLiteral *Literal) const {
+  const std::pair<FileID, unsigned> FileOffset =
+      SourceManager->getDecomposedLoc(Literal->getLocation());
+  if (FileOffset.first.isInvalid())
+    return false;
+
+  const StringRef BufferIdentifier =
+      SourceManager->getBuffer(FileOffset.first)->getBufferIdentifier();
+
+  return BufferIdentifier.empty();
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tidy/readability/MagicNumbersCheck.h b/clang-tidy/readability/MagicNumbersCheck.h
new file mode 100644
index 00000000..db4cc886
--- /dev/null
+++ b/clang-tidy/readability/MagicNumbersCheck.h
@@ -0,0 +1,97 @@
+//===--- MagicNumbersCheck.h - clang-tidy-----------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MAGICNUMBERSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MAGICNUMBERSCHECK_H
+
+#include "../ClangTidy.h"
+#include <llvm/ADT/APFloat.h>
+#include <llvm/ADT/SmallVector.h>
+#include <vector>
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Detects magic numbers, integer and floating point literals embedded in code.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-magic-numbers.html
+class MagicNumbersCheck : public ClangTidyCheck {
+public:
+  MagicNumbersCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  bool isConstant(const clang::ast_matchers::MatchFinder::MatchResult &Result,
+                  const clang::Expr &ExprResult) const;
+
+  bool isIgnoredValue(const IntegerLiteral *Literal) const;
+  bool isIgnoredValue(const FloatingLiteral *Literal) const;
+
+  bool isSyntheticValue(const clang::SourceManager *,
+                        const FloatingLiteral *) const {
+    return false;
+  }
+
+  bool isSyntheticValue(const clang::SourceManager *SourceManager,
+                        const IntegerLiteral *Literal) const;
+
+  template <typename L>
+  void checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result,
+                       const char *BoundName) {
+    const L *MatchedLiteral = Result.Nodes.getNodeAs<L>(BoundName);
+    if (!MatchedLiteral)
+      return;
+
+    if (Result.SourceManager->isMacroBodyExpansion(
+            MatchedLiteral->getLocation()))
+      return;
+
+    if (isIgnoredValue(MatchedLiteral))
+      return;
+
+    if (isConstant(Result, *MatchedLiteral))
+      return;
+
+    if (isSyntheticValue(Result.SourceManager, MatchedLiteral))
+      return;
+
+    const StringRef LiteralSourceText = Lexer::getSourceText(
+        CharSourceRange::getTokenRange(MatchedLiteral->getSourceRange()),
+        *Result.SourceManager, getLangOpts());
+
+    diag(MatchedLiteral->getLocation(),
+         "%0 is a magic number; consider replacing it with a named constant")
+        << LiteralSourceText;
+  }
+
+  const bool IgnoreAllFloatingPointValues;
+  const bool IgnorePowersOf2IntegerValues;
+
+  constexpr static unsigned SensibleNumberOfMagicValueExceptions = 16;
+
+  constexpr static llvm::APFloat::roundingMode DefaultRoundingMode =
+      llvm::APFloat::rmNearestTiesToEven;
+
+  llvm::SmallVector<int64_t, SensibleNumberOfMagicValueExceptions>
+      IgnoredIntegerValues;
+  llvm::SmallVector<float, SensibleNumberOfMagicValueExceptions>
+      IgnoredFloatingPointValues;
+  llvm::SmallVector<double, SensibleNumberOfMagicValueExceptions>
+      IgnoredDoublePointValues;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MAGICNUMBERSCHECK_H
diff --git a/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tidy/readability/ReadabilityTidyModule.cpp
index be1ae438..a5b91544 100644
--- a/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -20,6 +20,7 @@
 #include "IdentifierNamingCheck.h"
 #include "ImplicitBoolConversionCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
+#include "MagicNumbersCheck.h"
 #include "MisleadingIndentationCheck.h"
 #include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
@@ -65,6 +66,8 @@ public:
         "readability-implicit-bool-conversion");
     CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
         "readability-inconsistent-declaration-parameter-name");
+    CheckFactories.registerCheck<MagicNumbersCheck>(
+        "readability-magic-numbers");
     CheckFactories.registerCheck<MisleadingIndentationCheck>(
         "readability-misleading-indentation");
     CheckFactories.registerCheck<MisplacedArrayIndexCheck>(