Index: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt @@ -17,6 +17,7 @@ MisplacedWideningCastCheck.cpp MoveConstantArgumentCheck.cpp MoveConstructorInitCheck.cpp + MultipleStatementMacroCheck.cpp NewDeleteOverloadsCheck.cpp NoexceptMoveConstructorCheck.cpp NonCopyableObjects.cpp Index: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp @@ -25,6 +25,7 @@ #include "MisplacedWideningCastCheck.h" #include "MoveConstantArgumentCheck.h" #include "MoveConstructorInitCheck.h" +#include "MultipleStatementMacroCheck.h" #include "NewDeleteOverloadsCheck.h" #include "NoexceptMoveConstructorCheck.h" #include "NonCopyableObjects.h" @@ -79,6 +80,8 @@ "misc-move-const-arg"); CheckFactories.registerCheck( "misc-move-constructor-init"); + CheckFactories.registerCheck( + "misc-multiple-statement-macro"); CheckFactories.registerCheck( "misc-new-delete-overloads"); CheckFactories.registerCheck( Index: clang-tools-extra/trunk/clang-tidy/misc/MultipleStatementMacroCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/MultipleStatementMacroCheck.h +++ clang-tools-extra/trunk/clang-tidy/misc/MultipleStatementMacroCheck.h @@ -0,0 +1,37 @@ +//===--- MultipleStatementMacroCheck.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_MULTIPLE_STATEMENT_MACRO_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MULTIPLE_STATEMENT_MACRO_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace misc { + +/// Detect multiple statement macros that are used in unbraced conditionals. +/// Only the first statement of the macro will be inside the conditional and the +/// other ones will be executed unconditionally. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/misc-multiple-statement-macro.html +class MultipleStatementMacroCheck : public ClangTidyCheck { +public: + MultipleStatementMacroCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MULTIPLE_STATEMENT_MACRO_H Index: clang-tools-extra/trunk/clang-tidy/misc/MultipleStatementMacroCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/MultipleStatementMacroCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/misc/MultipleStatementMacroCheck.cpp @@ -0,0 +1,106 @@ +//===--- MultipleStatementMacroCheck.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 "MultipleStatementMacroCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace misc { + +namespace { + +AST_MATCHER(Expr, isInMacro) { return Node.getLocStart().isMacroID(); } + +/// \brief Find the next statement after `S`. +const Stmt *nextStmt(const MatchFinder::MatchResult &Result, const Stmt *S) { + auto Parents = Result.Context->getParents(*S); + if (Parents.empty()) + return nullptr; + const Stmt *Parent = Parents[0].get(); + if (!Parent) + return nullptr; + const Stmt* Prev = nullptr; + for (const Stmt *Child : Parent->children()) { + if (Prev == S) + return Child; + Prev = Child; + } + return nextStmt(Result, Parent); +} + +using ExpansionRanges = std::vector>; + +/// \bried Get all the macro expansion ranges related to `Loc`. +/// +/// The result is ordered from most inner to most outer. +ExpansionRanges getExpansionRanges(SourceLocation Loc, + const MatchFinder::MatchResult &Result) { + ExpansionRanges Locs; + while (Loc.isMacroID()) { + Locs.push_back(Result.SourceManager->getImmediateExpansionRange(Loc)); + Loc = Locs.back().first; + } + return Locs; +} + +} // namespace + +void MultipleStatementMacroCheck::registerMatchers(MatchFinder *Finder) { + const auto Inner = expr(isInMacro(), unless(compoundStmt())).bind("inner"); + Finder->addMatcher( + stmt(anyOf(ifStmt(hasThen(Inner)), ifStmt(hasElse(Inner)).bind("else"), + whileStmt(hasBody(Inner)), forStmt(hasBody(Inner)))) + .bind("outer"), + this); +} + +void MultipleStatementMacroCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *Inner = Result.Nodes.getNodeAs("inner"); + const auto *Outer = Result.Nodes.getNodeAs("outer"); + const auto *Next = nextStmt(Result, Outer); + if (!Next) + return; + + SourceLocation OuterLoc = Outer->getLocStart(); + if (Result.Nodes.getNodeAs("else")) + OuterLoc = cast(Outer)->getElseLoc(); + + auto InnerRanges = getExpansionRanges(Inner->getLocStart(), Result); + auto OuterRanges = getExpansionRanges(OuterLoc, Result); + auto NextRanges = getExpansionRanges(Next->getLocStart(), Result); + + // Remove all the common ranges, starting from the top (the last ones in the + // list). + while (!InnerRanges.empty() && !OuterRanges.empty() && !NextRanges.empty() && + InnerRanges.back() == OuterRanges.back() && + InnerRanges.back() == NextRanges.back()) { + InnerRanges.pop_back(); + OuterRanges.pop_back(); + NextRanges.pop_back(); + } + + // Inner and Next must have at least one more macro that Outer doesn't have, + // and that range must be common to both. + if (InnerRanges.empty() || NextRanges.empty() || + InnerRanges.back() != NextRanges.back()) + return; + + diag(InnerRanges.back().first, "multiple statement macro used without " + "braces; some statements will be " + "unconditionally executed"); +} + +} // namespace misc +} // namespace tidy +} // namespace clang Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst @@ -61,6 +61,7 @@ misc-macro-repeated-side-effects misc-misplaced-widening-cast misc-move-constructor-init + misc-multiple-statement-macro misc-new-delete-overloads misc-noexcept-move-constructor misc-non-copyable-objects Index: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-multiple-statement-macro.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-multiple-statement-macro.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-multiple-statement-macro.rst @@ -0,0 +1,16 @@ +.. title:: clang-tidy - misc-multiple-statement-macro + +misc-multiple-statement-macro +============================= + +Detect multiple statement macros that are used in unbraced conditionals. +Only the first statement of the macro will be inside the conditional and the other ones will be executed unconditionally. + +Example: + +.. code:: c++ + + #define INCREMENT_TWO(x, y) (x)++; (y)++ + if (do_increment) + INCREMENT_TWO(a, b); // `(b)++;` will be executed unconditionally. + Index: clang-tools-extra/trunk/test/clang-tidy/misc-multiple-statement-macro.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/misc-multiple-statement-macro.cpp +++ clang-tools-extra/trunk/test/clang-tidy/misc-multiple-statement-macro.cpp @@ -0,0 +1,85 @@ +// RUN: %check_clang_tidy %s misc-multiple-statement-macro %t + +void F(); + +#define BAD_MACRO(x) \ + F(); \ + F() + +#define GOOD_MACRO(x) \ + do { \ + F(); \ + F(); \ + } while (0) + +#define GOOD_MACRO2(x) F() + +#define GOOD_MACRO3(x) F(); + +#define MACRO_ARG_MACRO(X) \ + if (54) \ + X(2) + +#define ALL_IN_MACRO(X) \ + if (43) \ + F(); \ + F() + +#define GOOD_NESTED(x) \ + if (x) \ + GOOD_MACRO3(x); \ + F(); + +#define IF(x) if(x) + +void positives() { + if (1) + BAD_MACRO(1); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple statement macro used without braces; some statements will be unconditionally executed [misc-multiple-statement-macro] + if (1) { + } else + BAD_MACRO(1); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple statement macro used + while (1) + BAD_MACRO(1); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple statement macro used + for (;;) + BAD_MACRO(1); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple statement macro used + + MACRO_ARG_MACRO(BAD_MACRO); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: multiple statement macro used + MACRO_ARG_MACRO(F(); int); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: multiple statement macro used + IF(1) BAD_MACRO(1); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: multiple statement macro used +} + +void negatives() { + if (1) { + BAD_MACRO(1); + } else { + BAD_MACRO(1); + } + while (1) { + BAD_MACRO(1); + } + for (;;) { + BAD_MACRO(1); + } + + if (1) + GOOD_MACRO(1); + if (1) { + GOOD_MACRO(1); + } + if (1) + GOOD_MACRO2(1); + if (1) + GOOD_MACRO3(1); + + MACRO_ARG_MACRO(GOOD_MACRO); + ALL_IN_MACRO(1); + + IF(1) GOOD_MACRO(1); +}