AlexanderLanin updated this revision to Diff 88103.
AlexanderLanin marked 2 inline comments as done.
AlexanderLanin added a comment.
Fixes as reported in review
https://reviews.llvm.org/D29692
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/modernize/UseConstInsteadOfDefineCheck.cpp
clang-tidy/modernize/UseConstInsteadOfDefineCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-use-const-instead-of-define.rst
test/clang-tidy/modernize-use-const-instead-of-define.cpp
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,40 @@
+// 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 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 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)
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,41 @@
+.. 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;
+ }
+
+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
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/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,12 @@
Improvements to clang-tidy
--------------------------
+
+- New `modernize-use-const-instead-of-define
+ <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-const-instead-of-define.html>`_ check
+
+ Finds uses of ``#define`` where a const value would be more appropriate.
+
- New `safety-no-assembler
<http://clang.llvm.org/extra/clang-tidy/checks/safety-no-assembler.html>`_ check
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,119 @@
+//===--- 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 {
+ checkForConstantValues(MacroNameTok, MD->getMacroInfo());
+ }
+
+private:
+ /// performs the actual check
+ void checkForConstantValues(const Token &MacroNameTok, const MacroInfo *MI);
+
+ Preprocessor *PP;
+ UseConstInsteadOfDefineCheck *Check;
+};
+
+} // namespace
+
+/// numeric values may have + or - signs in front of them
+/// others like ~ are not so obvious and depend on usage
+static bool isReasonableNumberPrefix(const Token &T) {
+ return T.isOneOf(tok::plus, tok::minus);
+}
+
+/// Is T some sort of constant numeric value?
+static bool isAnyNumericLiteral(const Token &T) { return T.is(tok::numeric_constant); }
+
+/// 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);
+}
+
+/// Is T some sort of constant value?
+static bool isConstantValue(const Token &T) {
+ return isAnyNumericLiteral(T) || isAnyCharLiteral(T);
+}
+
+/// values may be in (parenthesis)
+static bool isAnyParenthesis(const Token &T) {
+ return T.isOneOf(tok::l_paren, tok::r_paren);
+}
+
+void UseConstInsteadOfDefineCheckPPCallbacks::checkForConstantValues(
+ const Token &MacroNameTok, const MacroInfo *MI) {
+ SourceLocation Loc;
+
+ bool hasPrefix = false;
+ bool hasValue = false;
+
+ for (auto TI = MI->tokens_begin(), TE = MI->tokens_end(); TI != TE; ++TI) {
+ const Token &Tok = *TI;
+
+ if (!hasPrefix && isReasonableNumberPrefix(Tok)) {
+ hasPrefix = true;
+
+ // start at number prefix like the + in +7
+ Loc = Tok.getLocation();
+ }
+ // numbers may have a prefix, however chars with a prefix are rather
+ // strange... let's not touch them
+ else if (isAnyParenthesis(Tok) || isAnyNumericLiteral(Tok) ||
+ (!hasPrefix && isAnyCharLiteral(Tok))) {
+ // prefix shall not be accepted anymore after any number
+ hasPrefix = true;
+ hasValue = true;
+
+ // Store location in case this is the first appearance
+ if (Loc.isInvalid()) {
+ Loc = Tok.getLocation();
+ }
+ } else {
+ // invalidate location, this #define is not a simple constant expression
+ Loc = SourceLocation();
+ break;
+ }
+ }
+
+ // loc is valid with a prefix only, so also check for an actual value/number
+ if (Loc.isValid() && hasValue) {
+ Check->diag(Loc, "use const variables instead of #define statements for "
+ "constant values");
+ }
+}
+
+
+void UseConstInsteadOfDefineCheck::registerPPCallbacks(
+ CompilerInstance &Compiler) {
+ Compiler.getPreprocessor().addPPCallbacks(
+ llvm::make_unique<UseConstInsteadOfDefineCheckPPCallbacks>(
+ &Compiler.getPreprocessor(), this));
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
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<UseAutoCheck>("modernize-use-auto");
CheckFactories.registerCheck<UseBoolLiteralsCheck>(
"modernize-use-bool-literals");
+ CheckFactories.registerCheck<UseConstInsteadOfDefineCheck>(
+ "modernize-use-const-instead-of-define");
CheckFactories.registerCheck<UseDefaultMemberInitCheck>(
"modernize-use-default-member-init");
CheckFactories.registerCheck<UseEmplaceCheck>("modernize-use-emplace");
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
