Index: clang-tidy/modernize/CMakeLists.txt =================================================================== --- clang-tidy/modernize/CMakeLists.txt +++ clang-tidy/modernize/CMakeLists.txt @@ -16,6 +16,7 @@ ShrinkToFitCheck.cpp UseAutoCheck.cpp UseBoolLiteralsCheck.cpp + UseConstInsteadOfDefineCheck.cpp UseDefaultMemberInitCheck.cpp UseEmplaceCheck.cpp UseEqualsDefaultCheck.cpp Index: clang-tidy/modernize/ModernizeTidyModule.cpp =================================================================== --- clang-tidy/modernize/ModernizeTidyModule.cpp +++ clang-tidy/modernize/ModernizeTidyModule.cpp @@ -22,6 +22,7 @@ #include "ShrinkToFitCheck.h" #include "UseAutoCheck.h" #include "UseBoolLiteralsCheck.h" +#include "UseConstInsteadOfDefineCheck.h" #include "UseDefaultMemberInitCheck.h" #include "UseEmplaceCheck.h" #include "UseEqualsDefaultCheck.h" @@ -57,6 +58,8 @@ CheckFactories.registerCheck("modernize-use-auto"); CheckFactories.registerCheck( "modernize-use-bool-literals"); + CheckFactories.registerCheck( + "modernize-use-const-instead-of-define"); CheckFactories.registerCheck( "modernize-use-default-member-init"); CheckFactories.registerCheck("modernize-use-emplace"); Index: clang-tidy/modernize/UseConstInsteadOfDefineCheck.h =================================================================== --- /dev/null +++ clang-tidy/modernize/UseConstInsteadOfDefineCheck.h @@ -0,0 +1,40 @@ +//===--- UseConstInsteadOfDefineCheck.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_MODERNIZE_USE_CONST_INSTEAD_OF_DEFINE_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_CONST_INSTEAD_OF_DEFINE_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace modernize { + +/// C++ const variables should be preferred over #define statements +/// +/// const variables obey type checking and scopes +/// +/// Further documentation e.g. at +/// https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es31-dont-use-macros-for-constants-or-functions +/// http://www.stroustrup.com/JSF-AV-rules.pdf +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-const-instead-of-define.html +class UseConstInsteadOfDefineCheck : public ClangTidyCheck { +public: + UseConstInsteadOfDefineCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerPPCallbacks(CompilerInstance &Compiler) override; +}; + +} // namespace modernize +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_CONST_INSTEAD_OF_DEFINE_H Index: clang-tidy/modernize/UseConstInsteadOfDefineCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/modernize/UseConstInsteadOfDefineCheck.cpp @@ -0,0 +1,97 @@ +//===--- UseConstInsteadOfDefineCheck.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 "UseConstInsteadOfDefineCheck.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" + +namespace clang { +namespace tidy { +namespace modernize { + +namespace { +class UseConstInsteadOfDefineCheckPPCallbacks : public PPCallbacks { +public: + UseConstInsteadOfDefineCheckPPCallbacks(Preprocessor *PP, + UseConstInsteadOfDefineCheck *Check) + : PP(PP), Check(Check) {} + + void MacroDefined(const Token &MacroNameTok, + const MacroDirective *MD) override; + +private: + Preprocessor *PP; + UseConstInsteadOfDefineCheck *Check; +}; + +} // namespace + +/// Is T some sort of constant char? +static bool isAnyCharLiteral(const Token &T) { + return T.isOneOf(tok::char_constant, tok::wide_char_constant, + tok::utf8_char_constant, tok::utf16_char_constant, + tok::utf32_char_constant); +} + +void UseConstInsteadOfDefineCheckPPCallbacks::MacroDefined( + const Token &MacroNameTok, const MacroDirective *MD) { + + const MacroInfo *MI = MD->getMacroInfo(); + + bool IsConstantValue = false; + bool HasPrefix = false; + int ParanthesisLevel = 0; + + for (auto TI = MI->tokens_begin(), TE = MI->tokens_end(); TI != TE; ++TI) { + const Token &Tok = *TI; + + // Numeric values may have `+` or `-` signs in front of them + // others like `~` are not so obvious to convert and depend on usage. + // Although there might be cases where the sign is actually used, those + // should be rare enough. e.g. + // `#define A +5` + // `int a = 5 A`; + if (!IsConstantValue && Tok.isOneOf(tok::plus, tok::minus)) { + HasPrefix = true; + } else if (Tok.is(tok::l_paren)) { + ++ParanthesisLevel; + } else if (Tok.is(tok::r_paren) && (ParanthesisLevel > 0)) { + --ParanthesisLevel; + } + // Chars with a `+` or `-` sign are rather rare and may have some special + // significance as shown above. + else if (Tok.is(tok::numeric_constant) || + (!HasPrefix && isAnyCharLiteral(Tok))) { + IsConstantValue = true; + } else { + // Invalidate location as this `#define` is not a simple constant + // expression. + IsConstantValue = false; + break; + } + } + + if (IsConstantValue && (ParanthesisLevel == 0)) { + Check->diag(MI->getDefinitionLoc(), + "use const variables instead of #define statements for " + "constant values"); + } +} + +void UseConstInsteadOfDefineCheck::registerPPCallbacks( + CompilerInstance &Compiler) { + Compiler.getPreprocessor().addPPCallbacks( + llvm::make_unique( + &Compiler.getPreprocessor(), this)); +} + +} // namespace modernize +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,6 +57,11 @@ Improvements to clang-tidy -------------------------- +- New `modernize-use-const-instead-of-define + `_ check + + Finds uses of ``#define`` where a const value would be more appropriate. + - New `readability-misleading-indentation `_ check Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -112,6 +112,7 @@ modernize-shrink-to-fit modernize-use-auto modernize-use-bool-literals + modernize-use-const-instead-of-define modernize-use-default-member-init modernize-use-emplace modernize-use-equals-default Index: docs/clang-tidy/checks/modernize-use-const-instead-of-define.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/modernize-use-const-instead-of-define.rst @@ -0,0 +1,57 @@ +.. title:: clang-tidy - modernize-use-const-instead-of-define + +modernize-use-const-instead-of-define +===================================== + +C++ ``const`` variables should be preferred over ``#define`` directives as +``#define`` does not obey type checking and scope rules. + +Example +------- + +.. code-block:: c++ + + // calc.h + namespace RealCalculation { + #define PI 3.14159 + } + + // quick.h + namespace QuickCalculation { + #define PI 1 + } + + // calc.cpp + #include "calc.h" + #include "quick.h" + + double calc(const double r) { + return 2*PI*r*r; + } + +Strings +--------------- + +Strings are excluded from this rule as they might be used for string +concatenations. Example: + +.. code-block:: c++ + + #define A "A" + #define B "B" + char AB[3] = A B; + +Code guidelines +--------------- + +While many C++ code guidelines like to prohibit macros completely with the +exception of include guards, that's a rather strict limitation. +Disallowing simple numbers should not require any significant code change and +may even offer fix-it suggestions in the future. + +See also: + +- https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es31-dont-use-macros-for-constants-or-functions +- MISRA C++ 16-2-2 +- JSF AV Rule 30 +- HIC++ 16.1.1 Index: test/clang-tidy/modernize-use-const-instead-of-define.cpp =================================================================== --- /dev/null +++ test/clang-tidy/modernize-use-const-instead-of-define.cpp @@ -0,0 +1,67 @@ +// RUN: %check_clang_tidy %s modernize-use-const-instead-of-define %t + +// Although there might be cases where the - sign is actually used those +// should be rare enough. e.g. int a = 5 BAD1; +#define BAD1 -1 +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] +#define BAD2 2 +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] +#define BAD3(A) 0 +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] +#define BAD4(X) (7) +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] +#define BAD5(X) +4 +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] +#define BAD6 'x' +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] +#define BAD7 0xff +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] +#define BAD8 L'c' +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] +#define BAD9 U'c' +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] +#define BAD10 1U +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] +#define BAD11 1.5 +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] +#define BAD12 1.4f +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] +#define BAD13 1.3L +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] +#define BAD14 ((-3)) +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] +#define BAD15 -(-3) +// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define] + +#define GOOD1 - +#define GOOD2 (1+2) +#define GOOD3(A) #A +#define GOOD4(A,B) A+B +#define GOOD5(T) ((T*)0) +#define GOOD6(type) (type(123)) +#define GOOD7(car, ...) car +#define GOOD8 a[5] +#define GOOD9(x) ;x; +#define GOOD10 ;-2; +#define GOOD11 =2 +#define GOOD12 +'a' +#define GOOD13 -'z' +#define GOOD14 !3 +#define GOOD15 ~-1 +#define GOOD16 "str" +#define GOOD17 L"str" +#define GOOD18 U"str" +#define GOOD19 () +#define GOOD20 ++ +#define GOOD21 ++i +#define GOOD22 k-- +#define GOOD23 m +#define GOOD24 5- +#define GOOD25 ((3) +#define GOOD26 (3)) +#define GOOD27 )2( + +#define NOT_DETECTED_YET_1(x) ((unsigned char)(0xff)) +#define NOT_DETECTED_YET_2 (int)7 +#define NOT_DETECTED_YET_3 int(-5) +#define NOT_DETECTED_YET_4 (int)(0)