0x8000-0000 created this revision.
0x8000-0000 added reviewers: Wizard, aaron.ballman, alexfh, hokein.
Herald added subscribers: cfe-commits, mgorny.
Add a clang-tidy check for "magic numbers", integers and floating point values
embedded in the code instead of using symbols or constants.
Bad example:
double circleArea = 3.1415926535 * radius * radius;
Good example:
double circleArea = M_PI * radius * radius;
This version detects and report integers only. If there is interest of merging
the tool I can add the functionality for floats as well.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D49114
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/MagicNumbersCheck.cpp
clang-tidy/readability/MagicNumbersCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-magic-numbers.rst
test/clang-tidy/readability-magic-numbers.cpp
Index: test/clang-tidy/readability-magic-numbers.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/readability-magic-numbers.cpp
@@ -0,0 +1,80 @@
+// RUN: %check_clang_tidy %s readability-magic-numbers %t
+
+template <typename T, int V>
+struct ValueBucket {
+ T value[V];
+};
+
+int BadGlobalInt = 5;
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: magic number integer literal 5 [readability-magic-numbers]
+
+int IntSquarer(int param) {
+ return param * param;
+}
+
+void BuggyFunction() {
+ int BadLocalInt = 6;
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: magic number integer literal 6 [readability-magic-numbers]
+
+ (void)IntSquarer(7);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: magic number integer literal 7 [readability-magic-numbers]
+
+ int LocalArray[8];
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: magic number integer literal 8 [readability-magic-numbers]
+
+ for (int ii = 0; ii < 8; ++ii)
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: magic number integer literal 8 [readability-magic-numbers]
+ {
+ LocalArray[ii] = 3 * ii;
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: magic number integer literal 3 [readability-magic-numbers]
+ }
+
+ ValueBucket<int, 4> Bucket;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: magic number integer literal 4 [readability-magic-numbers]
+}
+
+class TwoIntContainer {
+public:
+ TwoIntContainer(int val) : anotherMember(val * val), yetAnotherMember(2), anotherConstant(val + val) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:73: warning: magic number integer literal 2 [readability-magic-numbers]
+
+ int getValue() const;
+
+private:
+ int oneMember = 9;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: magic number integer literal 9 [readability-magic-numbers]
+
+ int anotherMember;
+
+ int yetAnotherMember;
+
+ const int oneConstant = 2;
+
+ const int anotherConstant;
+};
+
+int ValueArray[] = {3, 5};
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: magic number integer literal 3 [readability-magic-numbers]
+// CHECK-MESSAGES: :[[@LINE-2]]:24: warning: magic number integer literal 5 [readability-magic-numbers]
+
+/*
+ * Clean code
+ */
+
+#define INT_MACRO 5
+
+const int GoodGlobalIntContant = 42;
+
+constexpr int AlsoGoodGlobalIntContant = 42;
+
+void SolidFunction() {
+ const int GoodLocalIntContant = 43;
+
+ (void)IntSquarer(GoodLocalIntContant);
+
+ int LocalArray[INT_MACRO];
+
+ ValueBucket<int, INT_MACRO> Bucket;
+}
+
+const int ConstValueArray[] = {7, 9};
Index: docs/clang-tidy/checks/readability-magic-numbers.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/readability-magic-numbers.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - readability-magic-numbers
+
+readability-magic-numbers
+==========================
+
+Detects magic numbers, integer or floating point literal that are embedded in
+code and not introduced via constants or symbols.
+
+Bad example:
+
+ double circleArea = 3.1415926535 * radius * radius;
+
+Good example:
+
+ double circleArea = M_PI * radius * radius;
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -217,6 +217,7 @@
readability-identifier-naming
readability-implicit-bool-conversion
readability-inconsistent-declaration-parameter-name
+ readability-magic-numbers
readability-misleading-indentation
readability-misplaced-array-index
readability-named-parameter
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,12 @@
Improvements to clang-tidy
--------------------------
+- New `readability-magic-numbers
+ <http://clang.llvm.org/extra/clang-tidy/checks/readability-magic-numbers.html>`_ check
+
+ Detect usage of magic numbers, numbers that are used as literals instead of
+ introduced via constants or symbols.
+
- The checks profiling info can now be stored as JSON files for futher
post-processing and analysis.
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ 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 @@
"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>(
Index: clang-tidy/readability/MagicNumbersCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/readability/MagicNumbersCheck.h
@@ -0,0 +1,35 @@
+//===--- 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"
+
+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)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MAGICNUMBERSCHECK_H
Index: clang-tidy/readability/MagicNumbersCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/readability/MagicNumbersCheck.cpp
@@ -0,0 +1,108 @@
+//===--- 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.
+//
+//===----------------------------------------------------------------------===//
+
+#include "MagicNumbersCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void MagicNumbersCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(integerLiteral().bind("x"), this);
+}
+
+void MagicNumbersCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *MatchedDecl = Result.Nodes.getNodeAs<IntegerLiteral>("x");
+ if (MatchedDecl) {
+
+ if (Result.SourceManager->isMacroBodyExpansion(
+ MatchedDecl->getLocation())) {
+ // this is not a magic value in the source file, it actually is defined
+ // via a macro - not as good as const, but still OK
+ return;
+ }
+
+ llvm::APInt Value = MatchedDecl->getValue();
+ if ((Value == 0) || (Value == 1)) {
+ // 0 and 1 are "given" in a binary computer; besides, every C-style loop
+ // will trigger on base and increment - which is too much noise
+ return;
+ }
+
+ const auto &Parents = Result.Context->getParents(*MatchedDecl);
+ for (const auto &Parent : Parents) {
+
+ // the definition of const values themselves, as global or local variables
+ {
+ const auto *AsVarDecl = Parent.get<clang::VarDecl>();
+ if (AsVarDecl) {
+ if (AsVarDecl->getType().isConstQualified()) {
+ // good const definition
+ return;
+ }
+ }
+ }
+
+ // const class/struct members
+ {
+ const auto *AsFieldDecl = Parent.get<clang::FieldDecl>();
+ if (AsFieldDecl) {
+ if (AsFieldDecl->getType().isConstQualified()) {
+ // good const definition
+ return;
+ }
+ }
+ }
+
+ // template-argument (at the definition location)
+ {
+ const auto *AsTemplateArgument =
+ Parent.get<clang::SubstNonTypeTemplateParmExpr>();
+ if (AsTemplateArgument) {
+ // no need to report here, it will be also reported at the
+ // instantiation location
+ return;
+ }
+ }
+
+ // local/global non-const array
+ {
+ const auto *AsInitListExpr = Parent.get<clang::InitListExpr>();
+ if (AsInitListExpr) {
+ const auto &InitListParents =
+ Result.Context->getParents(*AsInitListExpr);
+ for (const auto &InitListParent : InitListParents) {
+ const auto *AsVarDecl = InitListParent.get<clang::VarDecl>();
+ if (AsVarDecl) {
+ if (AsVarDecl->getType().isConstQualified()) {
+ // good const definition
+ return;
+ }
+ }
+ }
+ }
+ }
+ }
+
+ SmallVector<char, 32> Str(32, '\0');
+ Str.resize(0);
+
+ Value.toString(Str, 10, true);
+
+ diag(MatchedDecl->getLocation(), "magic number integer literal %0") << Str.data();
+ }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tidy/readability/CMakeLists.txt
+++ clang-tidy/readability/CMakeLists.txt
@@ -11,6 +11,7 @@
IdentifierNamingCheck.cpp
ImplicitBoolConversionCheck.cpp
InconsistentDeclarationParameterNameCheck.cpp
+ MagicNumbersCheck.cpp
MisleadingIndentationCheck.cpp
MisplacedArrayIndexCheck.cpp
NamedParameterCheck.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
