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 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("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(); + if (AsVarDecl) { + if (AsVarDecl->getType().isConstQualified()) { + // good const definition + return; + } + } + } + + // const class/struct members + { + const auto *AsFieldDecl = Parent.get(); + if (AsFieldDecl) { + if (AsFieldDecl->getType().isConstQualified()) { + // good const definition + return; + } + } + } + + // template-argument (at the definition location) + { + const auto *AsTemplateArgument = + Parent.get(); + 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(); + if (AsInitListExpr) { + const auto &InitListParents = + Result.Context->getParents(*AsInitListExpr); + for (const auto &InitListParent : InitListParents) { + const auto *AsVarDecl = InitListParent.get(); + if (AsVarDecl) { + if (AsVarDecl->getType().isConstQualified()) { + // good const definition + return; + } + } + } + } + } + } + + SmallVector 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/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( "readability-inconsistent-declaration-parameter-name"); + CheckFactories.registerCheck( + "readability-magic-numbers"); CheckFactories.registerCheck( "readability-misleading-indentation"); CheckFactories.registerCheck( Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,6 +57,12 @@ Improvements to clang-tidy -------------------------- +- New `readability-magic-numbers + `_ 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: 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/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: 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 +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 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 Bucket; +} + +const int ConstValueArray[] = {7, 9};