[clang-tools-extra] r239820 - clang-tidy: Add checker that warns about missing parentheses in macros
Daniel Marjamaki
daniel.marjamaki at evidente.se
Tue Jun 16 07:27:32 PDT 2015
Author: danielmarjamaki
Date: Tue Jun 16 09:27:31 2015
New Revision: 239820
URL: http://llvm.org/viewvc/llvm-project?rev=239820&view=rev
Log:
clang-tidy: Add checker that warns about missing parentheses in macros
* calculations in the replacement list should be inside parentheses
* macro arguments should be inside parentheses
Added:
clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.h
clang-tools-extra/trunk/test/clang-tidy/misc-macro-parentheses.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=239820&r1=239819&r2=239820&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Tue Jun 16 09:27:31 2015
@@ -7,6 +7,7 @@ add_clang_library(clangTidyMiscModule
BoolPointerImplicitConversionCheck.cpp
InaccurateEraseCheck.cpp
InefficientAlgorithmCheck.cpp
+ MacroParenthesesCheck.cpp
MiscTidyModule.cpp
NoexceptMoveConstructorCheck.cpp
StaticAssertCheck.cpp
Added: clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp?rev=239820&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp Tue Jun 16 09:27:31 2015
@@ -0,0 +1,192 @@
+//===--- 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(const Token &T) {
+ return T.isOneOf(tok::l_paren, tok::l_brace, tok::l_square, tok::comma,
+ tok::semi);
+}
+
+/// Is argument surrounded properly with parentheses/braces/squares/commas?
+bool isSurroundedRight(const Token &T) {
+ return T.isOneOf(tok::r_paren, tok::r_brace, tok::r_square, tok::comma,
+ tok::semi);
+}
+
+/// Is given TokenKind a keyword?
+bool isKeyword(const Token &T) {
+ /// \TODO better matching of keywords to avoid false positives
+ return T.isOneOf(tok::kw_case, tok::kw_const, tok::kw_struct);
+}
+
+/// Warning is written when one of these operators are not within parentheses.
+bool isWarnOp(const Token &T) {
+ /// \TODO This is an initial list of operators. It can be tweaked later to
+ /// get more positives or perhaps avoid some false positive.
+ return T.isOneOf(tok::plus, tok::minus, tok::star, tok::slash, tok::percent,
+ tok::amp, tok::pipe, tok::caret);
+}
+}
+
+void MacroParenthesesPPCallbacks::replacementList(const Token &MacroNameTok,
+ const MacroInfo *MI) {
+ // Count how deep we are in parentheses/braces/squares.
+ int Count = 0;
+
+ // SourceLocation for error
+ 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))
+ return;
+ // When replacement list contains comma/semi don't warn about it.
+ if (Count == 0 && Tok.isOneOf(tok::comma, tok::semi))
+ return;
+ if (Tok.isOneOf(tok::l_paren, tok::l_brace, tok::l_square)) {
+ ++Count;
+ } else if (Tok.isOneOf(tok::r_paren, tok::r_brace, tok::r_square)) {
+ --Count;
+ // If there are unbalanced parentheses don't write any warning
+ if (Count < 0)
+ return;
+ } else if (Count == 0 && isWarnOp(Tok)) {
+ // 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.isOneOf(tok::plus, tok::minus))
+ return;
+ // Don't warn about this macro if the last token is a star. For example:
+ // #define X void *
+ if ((TE - 1)->is(tok::star))
+ return;
+
+ Loc = Tok.getLocation();
+ }
+ }
+ 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) {
+ // First token.
+ if (TI == MI->tokens_begin())
+ continue;
+
+ // Last token.
+ if ((TI + 1) == MI->tokens_end())
+ continue;
+
+ const Token &Prev = *(TI - 1);
+ const Token &Next = *(TI + 1);
+
+ const Token &Tok = *TI;
+
+ // Only interested in identifiers.
+ if (!Tok.isOneOf(tok::identifier, tok::raw_identifier))
+ continue;
+
+ // Only interested in macro arguments.
+ if (MI->getArgumentNum(Tok.getIdentifierInfo()) < 0)
+ continue;
+
+ // 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.isOneOf(tok::hash, tok::hashhash) || Next.is(tok::hashhash))
+ continue;
+
+ // Argument is a struct member.
+ if (Prev.isOneOf(tok::period, tok::arrow))
+ continue;
+
+ // String concatenation.
+ if (isStringLiteral(Prev.getKind()) || isStringLiteral(Next.getKind()))
+ continue;
+
+ // Type/Var.
+ if (isAnyIdentifier(Prev.getKind()) || isKeyword(Prev) ||
+ isAnyIdentifier(Next.getKind()) || isKeyword(Next))
+ continue;
+
+ // Initialization.
+ if (Next.is(tok::l_paren))
+ continue;
+
+ // Cast.
+ if (Prev.is(tok::l_paren) && Next.is(tok::star) &&
+ TI + 2 != MI->tokens_end() && (TI + 2)->is(tok::r_paren))
+ continue;
+
+ // Assignment.
+ if (Prev.is(tok::equal) && Next.is(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
Added: clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.h?rev=239820&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.h Tue Jun 16 09:27:31 2015
@@ -0,0 +1,42 @@
+//===--- 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
Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=239820&r1=239819&r2=239820&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Tue Jun 16 09:27:31 2015
@@ -16,6 +16,7 @@
#include "BoolPointerImplicitConversionCheck.h"
#include "InaccurateEraseCheck.h"
#include "InefficientAlgorithmCheck.h"
+#include "MacroParenthesesCheck.h"
#include "NoexceptMoveConstructorCheck.h"
#include "StaticAssertCheck.h"
#include "SwappedArgumentsCheck.h"
@@ -42,6 +43,8 @@ public:
"misc-inaccurate-erase");
CheckFactories.registerCheck<InefficientAlgorithmCheck>(
"misc-inefficient-algorithm");
+ CheckFactories.registerCheck<MacroParenthesesCheck>(
+ "misc-macro-parentheses");
CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
"misc-noexcept-move-constructor");
CheckFactories.registerCheck<StaticAssertCheck>(
Added: clang-tools-extra/trunk/test/clang-tidy/misc-macro-parentheses.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-macro-parentheses.cpp?rev=239820&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-macro-parentheses.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-macro-parentheses.cpp Tue Jun 16 09:27:31 2015
@@ -0,0 +1,36 @@
+// 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 GOOD15(x) ({ int a = x; a+4; })
+#define GOOD16(x) a_ ## x, b_ ## x = c_ ## x - 1,
+#define GOOD17 case 123: x=4+5; break;
+#define GOOD18(x) ;x;
+#define GOOD19 ;-2;
+#define GOOD20 void*
+
+// These are allowed for now..
+#define MAYBE1 *12.34
+#define MAYBE2 <<3
More information about the cfe-commits
mailing list