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 StaticAssertCheck.cpp SwappedArgumentsCheck.cpp Index: clang-tidy/misc/MacroParenthesesCheck.h =================================================================== --- clang-tidy/misc/MacroParenthesesCheck.h +++ clang-tidy/misc/MacroParenthesesCheck.h @@ -0,0 +1,29 @@ +//===--- 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 { + +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/MacroParenthesesCheck.cpp =================================================================== --- clang-tidy/misc/MacroParenthesesCheck.cpp +++ clang-tidy/misc/MacroParenthesesCheck.cpp @@ -0,0 +1,166 @@ +//===--- 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); + argument(MacroNameTok, MD); + } + +private: + /** replacement list with calculations should be enclosed in parentheses */ + void replacementList(const Token &MacroNameTok, const MacroDirective *MD); + + /** arguments should be enclosed in parentheses */ + void argument(const Token &MacroNameTok, const MacroDirective *MD); + + Preprocessor *PP; + MacroParenthesesCheck *Check; +}; +} + +void MacroParenthesesPPCallbacks::replacementList(const Token &MacroNameTok, + const MacroDirective *MD) { + const MacroInfo *MI = MD->getMacroInfo(); + + // bigger than 0 => we are inside parentheses / braces / squares + int Indent = 0; + + // Is there a previous comma? + bool Comma = false; + + // Have we seen some calculation outside parentheses/braces/squares + bool Err = false; + + for (auto TI = MI->tokens_begin(), TE = MI->tokens_end(); TI != TE; ++TI) { + const Token &Tok = *TI; + if (Tok.is(tok::l_paren) || Tok.is(tok::l_brace) || Tok.is(tok::l_square)) { + ++Indent; + } else if (Tok.is(tok::r_paren) || Tok.is(tok::r_brace) || + Tok.is(tok::r_square)) { + --Indent; + } else if (Indent <= 0 && Tok.is(tok::comma)) { + if (Comma) + Err = false; + Comma = true; + } else if (Indent <= 0 && + (Tok.is(tok::plus) || Tok.is(tok::minus) || Tok.is(tok::star) || + Tok.is(tok::slash) || Tok.is(tok::percent) || + Tok.is(tok::amp) || Tok.is(tok::pipe) || Tok.is(tok::caret) || + Tok.is(tok::question) || Tok.is(tok::colon))) { + // heuristic for macros that are clearly not intended to be enclosed in + // parentheses, macro contains just an operator and a number + // #define X *10 + if (TI == MI->tokens_begin() && !Tok.is(tok::plus) && + !Tok.is(tok::minus) && (TI + 1) != TE && + (TI + 1)->is(tok::numeric_constant) && (TI + 2) == TE) { + return; + } + Err = true; + if (!Comma) + break; + } + } + if (Err) { + SourceLocation Loc = MI->tokens_begin()->getLocation(); + Check->diag(Loc, + "macro replacement list should be enclosed in parentheses"); + } +} + +void MacroParenthesesPPCallbacks::argument(const Token &MacroNameTok, + const MacroDirective *MD) { + const MacroInfo *MI = MD->getMacroInfo(); + enum { START, AFTER_LPAR, EXPECT_RPAR, AFTER_EQ, EXPECT_SEMI } State = START; + for (auto TI = MI->tokens_begin(), TE = MI->tokens_end(); TI != TE; ++TI) { + const Token &Tok = *TI; + if (Tok.is(tok::hash) || Tok.is(tok::hashhash)) { + // Skip next token + ++TI; + if (TI == TE) + break; + continue; + } else if (Tok.is(tok::l_paren) || Tok.is(tok::l_square)) { + State = AFTER_LPAR; + } else if (Tok.is(tok::r_paren) || Tok.is(tok::r_square)) { + State = START; + } else if (State == EXPECT_SEMI && Tok.is(tok::semi)) { + State = START; + } else if ((Tok.is(tok::identifier) || Tok.is(tok::raw_identifier)) && + MI->getArgumentNum(Tok.getIdentifierInfo()) >= 0) { + // Ignore this if it's the first token + if (TI == MI->tokens_begin()) + continue; + // Ignore this if previous token is a string literal, identifier, or a + // period/arrow + if (TI != MI->tokens_begin() && + ((TI - 1)->is(tok::string_literal) || (TI - 1)->is(tok::arrow) || + (TI - 1)->is(tok::period) || (TI - 1)->is(tok::identifier) || + (TI - 1)->is(tok::raw_identifier))) + continue; + // Ignore this if next token is ##, a string literal, or a identifier + if ((TI + 1) != TE && + ((TI + 1)->is(tok::hashhash) || (TI + 1)->is(tok::string_literal) || + (TI + 1)->is(tok::identifier) || (TI + 1)->is(tok::raw_identifier))) + continue; + if (State == AFTER_LPAR) { + State = EXPECT_RPAR; + } else if (State == AFTER_EQ) { + State = EXPECT_SEMI; + } else { + Check->diag(Tok.getLocation(), "macro argument should be enclosed in " + "parentheses"); + return; + } + } else if (Tok.is(tok::comma)) { + State = AFTER_LPAR; + } else if (Tok.is(tok::period)) { + // Skip next token + ++TI; + if (TI == TE) + break; + continue; + } else if (Tok.is(tok::equal) && State == START) { + State = AFTER_EQ; + } else if (Tok.is(tok::star) && State == EXPECT_RPAR) { + /* cast - stay in AFTER_ID */ + } else if (State == AFTER_LPAR) { + State = START; + } else if (State == EXPECT_RPAR || State == EXPECT_SEMI) { + auto Prev = TI - 1; + Check->diag(Prev->getLocation(), "macro argument should be enclosed in " + "parentheses"); + return; + } + } +} + +void MacroParenthesesCheck::registerPPCallbacks(CompilerInstance &Compiler) { + Compiler.getPreprocessor().addPPCallbacks( + llvm::make_unique( + &Compiler.getPreprocessor(), this)); +} + +} // namespace tidy +} // namespace clang 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 "StaticAssertCheck.h" #include "SwappedArgumentsCheck.h" #include "UndelegatedConstructor.h" @@ -41,6 +42,8 @@ "misc-inaccurate-erase"); CheckFactories.registerCheck( "misc-inefficient-algorithm"); + CheckFactories.registerCheck( + "misc-macro-parentheses"); CheckFactories.registerCheck( "misc-static-assert"); CheckFactories.registerCheck( 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,32 @@ +// 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]]:27: 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, + +// These are allowed for now.. +#define MAYBE1 *12.34 +#define MAYBE2 <<3