Updated patch:
- Added fixits
- Tweaked code comments
- Avoid FP when argument is surrounded with semicolon ;X;
http://reviews.llvm.org/D9528
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MacroParenthesesCheck.cpp
clang-tidy/misc/MacroParenthesesCheck.h
clang-tidy/misc/MiscTidyModule.cpp
test/clang-tidy/misc-macro-parentheses.cpp
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: test/clang-tidy/misc-macro-parentheses.cpp
===================================================================
--- test/clang-tidy/misc-macro-parentheses.cpp
+++ test/clang-tidy/misc-macro-parentheses.cpp
@@ -0,0 +1,34 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s misc-macro-parentheses %t
+// REQUIRES: shell
+
+#define BAD1 -1
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: macro replacement list should be enclosed in parentheses [misc-macro-parentheses]
+#define BAD2 1+2
+// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: macro replacement list should be enclosed in parentheses [misc-macro-parentheses]
+#define BAD3(A) (A+1)
+// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: macro argument should be enclosed in parentheses [misc-macro-parentheses]
+#define BAD4(x) ((unsigned char)(x & 0xff))
+// CHECK-MESSAGES: :[[@LINE-1]]:44: warning: macro argument should be enclosed in parentheses [misc-macro-parentheses]
+
+#define GOOD1 1
+#define GOOD2 (1+2)
+#define GOOD3(A) #A
+#define GOOD4(A,B) A ## B
+#define GOOD5(T) ((T*)0)
+#define GOOD6(B) "A" B "C"
+#define GOOD7(b) A b
+#define GOOD8(a) a B
+#define GOOD9(type) (type(123))
+#define GOOD10(car, ...) car
+#define GOOD11 a[b+c]
+#define GOOD12(x) a[x]
+#define GOOD13(x) a.x
+#define GOOD14(x) a->x
+#define GOOD14(x) ({ int a = x; a+4; })
+#define GOOD15(x) a_ ## x, b_ ## x = c_ ## x - 1,
+#define GOOD16 case 123: x=4+5; break;
+#define GOOD17(x) ;x;
+
+// These are allowed for now..
+#define MAYBE1 *12.34
+#define MAYBE2 <<3
Index: clang-tidy/misc/MacroParenthesesCheck.h
===================================================================
--- clang-tidy/misc/MacroParenthesesCheck.h
+++ clang-tidy/misc/MacroParenthesesCheck.h
@@ -0,0 +1,43 @@
+//===--- MacroParenthesesCheck.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_MISC_MACRO_PARENTHESES_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MACRO_PARENTHESES_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief Finds macros that can have unexpected behaviour due to missing
+/// parentheses.
+///
+/// Macros are expanded by the preprocessor as-is. As a result, there can be
+/// unexpected behaviour; operators may be evaluated in unexpected order and
+/// unary operators may become binary operators, etc.
+///
+/// When the replacement list has an expression it is recommended to surround it
+/// with parentheses. This ensures that the macro result is evaluated
+/// completely before it is used.
+///
+/// It is also recommended to surround macro arguments in the replacement list
+/// with parentheses. This ensures that the argument value is calculated
+/// properly.
+
+class MacroParenthesesCheck : public ClangTidyCheck {
+public:
+ MacroParenthesesCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerPPCallbacks(CompilerInstance &Compiler) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MACRO_PARENTHESES_H
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -16,6 +16,7 @@
#include "BoolPointerImplicitConversionCheck.h"
#include "InaccurateEraseCheck.h"
#include "InefficientAlgorithmCheck.h"
+#include "MacroParenthesesCheck.h"
#include "NoexceptMoveCtorsCheck.h"
#include "StaticAssertCheck.h"
#include "SwappedArgumentsCheck.h"
@@ -42,6 +43,8 @@
"misc-inaccurate-erase");
CheckFactories.registerCheck<InefficientAlgorithmCheck>(
"misc-inefficient-algorithm");
+ CheckFactories.registerCheck<MacroParenthesesCheck>(
+ "misc-macro-parentheses");
CheckFactories.registerCheck<NoexceptMoveCtorsCheck>(
"misc-noexcept-move-ctors");
CheckFactories.registerCheck<StaticAssertCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -7,6 +7,7 @@
BoolPointerImplicitConversionCheck.cpp
InaccurateEraseCheck.cpp
InefficientAlgorithmCheck.cpp
+ MacroParenthesesCheck.cpp
MiscTidyModule.cpp
NoexceptMoveCtorsCheck.cpp
StaticAssertCheck.cpp
Index: clang-tidy/misc/MacroParenthesesCheck.cpp
===================================================================
--- clang-tidy/misc/MacroParenthesesCheck.cpp
+++ clang-tidy/misc/MacroParenthesesCheck.cpp
@@ -0,0 +1,191 @@
+//===--- MacroParenthesesCheck.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 "MacroParenthesesCheck.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+
+namespace clang {
+namespace tidy {
+
+namespace {
+class MacroParenthesesPPCallbacks : public PPCallbacks {
+public:
+ explicit MacroParenthesesPPCallbacks(Preprocessor *PP,
+ MacroParenthesesCheck *Check)
+ : PP(PP), Check(Check) {}
+
+ void MacroDefined(const Token &MacroNameTok,
+ const MacroDirective *MD) override {
+ replacementList(MacroNameTok, MD->getMacroInfo());
+ argument(MacroNameTok, MD->getMacroInfo());
+ }
+
+private:
+ /// Replacement list with calculations should be enclosed in parentheses
+ void replacementList(const Token &MacroNameTok, const MacroInfo *MI);
+
+ /// arguments should be enclosed in parentheses
+ void argument(const Token &MacroNameTok, const MacroInfo *MI);
+
+ Preprocessor *PP;
+ MacroParenthesesCheck *Check;
+};
+
+/// Is argument surrounded properly with parentheses/braces/squares/commas?
+bool isSurroundedLeft(tok::TokenKind K) {
+ return K == tok::l_paren || K == tok::l_brace || K == tok::l_square ||
+ K == tok::comma || K == tok::semi;
+}
+
+/// Is argument surrounded properly with parentheses/braces/squares/commas?
+bool isSurroundedRight(tok::TokenKind K) {
+ return K == tok::r_paren || K == tok::r_brace || K == tok::r_square ||
+ K == tok::comma || K == tok::semi;
+}
+
+/// Is given TokenKind a keyword?
+bool isKeyword(tok::TokenKind K) {
+ /// \TODO better matching of keywords
+ return K == tok::kw_case || K == tok::kw_const || K == tok::kw_struct;
+}
+
+/// Is given TokenKind a calculation operator?
+bool isCalcOp(tok::TokenKind K) {
+ return K == tok::plus || K == tok::minus || K == tok::star ||
+ K == tok::slash || K == tok::percent || K == tok::amp ||
+ K == tok::pipe || K == tok::caret;
+}
+}
+
+void MacroParenthesesPPCallbacks::replacementList(const Token &MacroNameTok,
+ const MacroInfo *MI) {
+ // Count how deep we are in parentheses/braces/squares.
+ int Count = 0;
+
+ // Is there a previous comma?
+ bool Comma = false;
+
+ // Have we seen some calculation outside parentheses/braces/squares.
+ SourceLocation Loc;
+
+ for (auto TI = MI->tokens_begin(), TE = MI->tokens_end(); TI != TE; ++TI) {
+ const Token &Tok = *TI;
+ // Replacement list contains keywords, don't warn about it.
+ if (isKeyword(Tok.getKind()))
+ break;
+ if (Tok.is(tok::l_paren) || Tok.is(tok::l_brace) || Tok.is(tok::l_square)) {
+ ++Count;
+ } else if (Tok.is(tok::r_paren) || Tok.is(tok::r_brace) ||
+ Tok.is(tok::r_square)) {
+ --Count;
+ } else if (Count <= 0 && Tok.is(tok::comma)) {
+ if (Comma)
+ Loc = SourceLocation();
+ Comma = true;
+ } else if (Count <= 0 && isCalcOp(Tok.getKind())) {
+ // Heuristic for macros that are clearly not intended to be enclosed in
+ // parentheses, macro starts with operator. For example:
+ // #define X *10
+ if (TI == MI->tokens_begin() && (TI + 1) != TE && !Tok.is(tok::plus) &&
+ !Tok.is(tok::minus)) {
+ return;
+ }
+ Loc = Tok.getLocation();
+ if (!Comma)
+ break;
+ }
+ }
+ if (Loc.isValid()) {
+ const Token &Last = *(MI->tokens_end() - 1);
+ Check->diag(Loc, "macro replacement list should be enclosed in parentheses")
+ << FixItHint::CreateInsertion(MI->tokens_begin()->getLocation(), "(")
+ << FixItHint::CreateInsertion(Last.getLocation().getLocWithOffset(
+ PP->getSpelling(Last).length()),
+ ")");
+ }
+}
+
+void MacroParenthesesPPCallbacks::argument(const Token &MacroNameTok,
+ const MacroInfo *MI) {
+
+ for (auto TI = MI->tokens_begin(), TE = MI->tokens_end(); TI != TE; ++TI) {
+ const Token &Tok = *TI;
+
+ // Only interested in identifiers.
+ if (!Tok.is(tok::identifier) && !Tok.is(tok::raw_identifier))
+ continue;
+
+ // Only interested in macro arguments.
+ if (MI->getArgumentNum(Tok.getIdentifierInfo()) < 0)
+ continue;
+
+ // First token.
+ if (TI == MI->tokens_begin())
+ continue;
+
+ // Last token.
+ if ((TI + 1) == MI->tokens_end())
+ continue;
+
+ const tok::TokenKind Prev = (TI - 1)->getKind();
+ const tok::TokenKind Next = (TI + 1)->getKind();
+
+ // Argument is surrounded with parentheses/squares/braces/commas.
+ if (isSurroundedLeft(Prev) && isSurroundedRight(Next))
+ continue;
+
+ // Don't warn after hash/hashhash or before hashhash.
+ if (Prev == tok::hash || Prev == tok::hashhash || Next == tok::hashhash)
+ continue;
+
+ // Argument is a struct member.
+ if (Prev == tok::period || Prev == tok::arrow)
+ continue;
+
+ // String concatenation.
+ if (isStringLiteral(Prev) || isStringLiteral(Next))
+ continue;
+
+ // Type/Var.
+ if (isAnyIdentifier(Prev) || isKeyword(Prev) || isAnyIdentifier(Next) ||
+ isKeyword(Next))
+ continue;
+
+ // Initialisation.
+ if (Next == tok::l_paren)
+ continue;
+
+ // Cast.
+ if (Prev == tok::l_paren && Next == tok::star &&
+ TI + 2 != MI->tokens_end() && (TI + 2)->is(tok::r_paren))
+ continue;
+
+ // Assignment.
+ if (Prev == tok::equal && Next == tok::semi)
+ continue;
+
+ Check->diag(Tok.getLocation(), "macro argument should be enclosed in "
+ "parentheses")
+ << FixItHint::CreateInsertion(Tok.getLocation(), "(")
+ << FixItHint::CreateInsertion(Tok.getLocation().getLocWithOffset(
+ PP->getSpelling(Tok).length()),
+ ")");
+ }
+}
+
+void MacroParenthesesCheck::registerPPCallbacks(CompilerInstance &Compiler) {
+ Compiler.getPreprocessor().addPPCallbacks(
+ llvm::make_unique<MacroParenthesesPPCallbacks>(
+ &Compiler.getPreprocessor(), this));
+}
+
+} // namespace tidy
+} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
