Index: clang-tidy/misc/AssertSideEffectCheck.h =================================================================== --- clang-tidy/misc/AssertSideEffectCheck.h +++ clang-tidy/misc/AssertSideEffectCheck.h @@ -0,0 +1,40 @@ +//===--- AssertSideEffectCheck.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_ASSERT_SIDE_EFFECT_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ASSERT_SIDE_EFFECT_CHECK_H + +#include "../ClangTidy.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" +#include + +namespace clang { +namespace tidy { + +/// \brief Finds \c assert() with side effect. +/// +/// The conition of \c assert() is evaluated only in debug builds so a condition +/// with side effect can cause different behaviour in debug / relesase builds. +class AssertSideEffectCheck : public ClangTidyCheck { + const bool CheckFunctionCalls; + const std::string RawAssertList; + llvm::SmallVector AssertList; + +public: + AssertSideEffectCheck(llvm::StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ASSERT_SIDE_EFFECT_CHECK_H Index: clang-tidy/misc/AssertSideEffectCheck.cpp =================================================================== --- clang-tidy/misc/AssertSideEffectCheck.cpp +++ clang-tidy/misc/AssertSideEffectCheck.cpp @@ -0,0 +1,118 @@ +//===--- AssertSideEffectCheck.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 "AssertSideEffectCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/Lexer.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Casting.h" +#include +#include + +using namespace clang; +using namespace ast_matchers; + +namespace clang { +namespace ast_matchers { + +AST_MATCHER_P(Expr, hasSideEffect, bool, CheckFunctionCalls) { + const Expr *E = &Node; + bool RetVal = false; + + if (const auto *Op = llvm::dyn_cast(E)) { + UnaryOperator::Opcode OC = Op->getOpcode(); + RetVal = OC == UO_PostInc || OC == UO_PostDec || OC == UO_PreInc || + OC == UO_PreDec; + } + + if (const auto *Op = llvm::dyn_cast(E)) { + BinaryOperator::Opcode OC = Op->getOpcode(); + RetVal = OC == BO_Assign || OC == BO_MulAssign || OC == BO_DivAssign || + OC == BO_RemAssign || OC == BO_AddAssign || OC == BO_SubAssign || + OC == BO_ShlAssign || OC == BO_ShrAssign || OC == BO_AndAssign || + OC == BO_XorAssign || OC == BO_OrAssign; + } + + RetVal |= llvm::isa(E) || llvm::isa(E) || + llvm::isa(E); + + if (const auto *CExpr = llvm::dyn_cast(E)) { + RetVal = CheckFunctionCalls; + if (const auto *FuncDecl = CExpr->getDirectCallee()) + if (const auto *MethDecl = llvm::dyn_cast(FuncDecl)) + RetVal &= !MethDecl->isConst(); + + const auto *OpCallExpr = llvm::dyn_cast(E); + if (OpCallExpr) { + RetVal = false; + OverloadedOperatorKind OpKind = OpCallExpr->getOperator(); + RetVal |= OpKind == OO_Equal || OpKind == OO_PlusEqual || + OpKind == OO_MinusEqual || OpKind == OO_StarEqual || + OpKind == OO_SlashEqual || OpKind == OO_SlashEqual || + OpKind == OO_PipeEqual || OpKind == OO_CaretEqual || + OpKind == OO_LessLessEqual || + OpKind == OO_GreaterGreaterEqual || OpKind == OO_PlusPlus || + OpKind == OO_MinusMinus || OpKind == OO_PercentEqual; + } + } + + return RetVal; +} + +} // namespace ast_matchers + +namespace tidy { + +AssertSideEffectCheck::AssertSideEffectCheck(llvm::StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + CheckFunctionCalls(Options.get("CheckFunctionCalls", false)), + RawAssertList(Options.get("AssertList", "assert")) { + llvm::StringRef SR = RawAssertList; + SR.split(AssertList, " ", -1, false); +} + +void AssertSideEffectCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "CheckFunctionCalls", CheckFunctionCalls); + Options.store(Opts, "AssertList", RawAssertList); +} + +void AssertSideEffectCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + stmt(anyOf(conditionalOperator(hasCondition( + hasDescendant(expr(hasSideEffect(CheckFunctionCalls))))), + ifStmt(hasCondition(hasDescendant(expr( + hasSideEffect(CheckFunctionCalls))))))).bind("condStmt"), + this); +} + +void AssertSideEffectCheck::check(const MatchFinder::MatchResult &Result) { + ASTContext *const ASTCtx = Result.Context; + const auto *CondStmt = Result.Nodes.getNodeAs("condStmt"); + const SourceLocation Loc = CondStmt->getLocStart(); + + if (!Loc.isValid() || !Loc.isMacroID()) + return; + + llvm::StringRef MacroName = Lexer::getImmediateMacroName( + Loc, ASTCtx->getSourceManager(), ASTCtx->getLangOpts()); + + // Check if this macro is an assert + if (std::find(AssertList.begin(), AssertList.end(), MacroName) == + AssertList.end()) + return; + + diag(Loc, "found " + MacroName.str() + "() with side effect"); +} + +} // namespace tidy +} // namespace clang Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -2,8 +2,10 @@ add_clang_library(clangTidyMiscModule ArgumentCommentCheck.cpp + AssertSideEffectCheck.cpp BoolPointerImplicitConversion.cpp MiscTidyModule.cpp + StaticAssertCheck.cpp SwappedArgumentsCheck.cpp UndelegatedConstructor.cpp UnusedRAII.cpp Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ clang-tidy/misc/MiscTidyModule.cpp @@ -11,7 +11,9 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "ArgumentCommentCheck.h" +#include "AssertSideEffectCheck.h" #include "BoolPointerImplicitConversion.h" +#include "StaticAssertCheck.h" #include "SwappedArgumentsCheck.h" #include "UndelegatedConstructor.h" #include "UniqueptrResetRelease.h" @@ -25,8 +27,12 @@ public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck("misc-argument-comment"); + CheckFactories.registerCheck( + "misc-assert-side-effect"); CheckFactories.registerCheck( "misc-bool-pointer-implicit-conversion"); + CheckFactories.registerCheck( + "misc-static-assert"); CheckFactories.registerCheck( "misc-swapped-arguments"); CheckFactories.registerCheck( Index: clang-tidy/misc/StaticAssertCheck.h =================================================================== --- clang-tidy/misc/StaticAssertCheck.h +++ clang-tidy/misc/StaticAssertCheck.h @@ -0,0 +1,35 @@ +//===--- StaticAssertCheck.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_STATIC_ASSERT_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STATIC_ASSERT_CHECK_H + +#include "../ClangTidy.h" +#include "llvm/ADT/StringRef.h" +#include + +namespace clang { +namespace tidy { + +/// \brief Replaces \c assert() with \c static_assert() if the condition is +/// evaluatable at compile time. +/// +/// The condition of \c static_assert() is evaluated at compile time which is +/// safer and more efficient. +class StaticAssertCheck : public ClangTidyCheck { +public: + StaticAssertCheck(llvm::StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_STATIC_ASSERT_CHECK_H Index: clang-tidy/misc/StaticAssertCheck.cpp =================================================================== --- clang-tidy/misc/StaticAssertCheck.cpp +++ clang-tidy/misc/StaticAssertCheck.cpp @@ -0,0 +1,69 @@ +//===--- StaticAssertCheck.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 "StaticAssertCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Expr.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/Lexer.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Casting.h" +#include +#include + +using namespace clang; +using namespace ast_matchers; + +namespace clang { +namespace tidy { + +StaticAssertCheck::StaticAssertCheck(llvm::StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + +void StaticAssertCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + stmt(anyOf(conditionalOperator(hasCondition(expr().bind("condition"))), + ifStmt(hasCondition(expr().bind("condition")))), + unless(isInTemplateInstantiation())).bind("condStmt"), + this); +} + +void StaticAssertCheck::check(const MatchFinder::MatchResult &Result) { + ASTContext *const ASTCtx = Result.Context; + const LangOptions &Opts = ASTCtx->getLangOpts(); + const SourceManager &SM = ASTCtx->getSourceManager(); + const auto *CondStmt = Result.Nodes.getNodeAs("condStmt"); + const auto *Condition = Result.Nodes.getNodeAs("condition"); + const SourceLocation Loc = CondStmt->getLocStart(); + + if (!Opts.CPlusPlus11 || !Loc.isValid() || !Loc.isMacroID()) + return; + + llvm::StringRef MacroName = + Lexer::getImmediateMacroName(Loc, ASTCtx->getSourceManager(), Opts); + + if (MacroName != "assert" || !Condition->isEvaluatable(*ASTCtx)) + return; + + const SourceLocation CLoc = SM.getImmediateMacroCallerLoc(Loc); + const std::pair RPair = + SM.getExpansionRange(CLoc); + const SourceRange SRange(RPair.first, RPair.second); + + FixItHint Hint; + if (CLoc.isValid() && !CLoc.isMacroID()) + Hint = FixItHint::CreateReplacement(SRange, "static_assert"); + + diag(Loc, "found assert() that could be replaced by static_assert()") << Hint; +} + +} // namespace tidy +} // namespace clang Index: test/clang-tidy/misc-assert-side-effect.cpp =================================================================== --- test/clang-tidy/misc-assert-side-effect.cpp +++ test/clang-tidy/misc-assert-side-effect.cpp @@ -0,0 +1,63 @@ +// RUN: $(dirname %s)/check_clang_tidy.sh %s misc-assert-side-effect %t -config="{CheckOptions: [{key: misc-assert-side-effect.CheckFunctionCalls, value: 1}, {key: misc-assert-side-effect.AssertList, value: 'assert my_assert'}]}" -- +// REQUIRES: shell + +//===--- assert definition block ------------------------------------------===// +void abort() {} + +#ifdef NDEBUG +#define assert(x) 1 +#else +#define assert(x) \ + if (!(x)) \ + abort() +#endif + +#ifdef NDEBUG +#define my_assert(x) 1 +#else +#define my_assert(x) \ + if (!(x)) \ + abort() +#endif + +#ifdef NDEBUG +#define not_my_assert(x) 1 +#else +#define not_my_assert(x) \ + if (!(x)) \ + abort() +#endif +//===----------------------------------------------------------------------===// + +class MyClass { +public: + bool badFunc(int a, int b) { return a * b > 0; } + bool goodFunc(int a, int b) const { return a * b > 0; } + + MyClass &operator=(const MyClass &rhs) { return *this; } + + operator bool() const { return true; } +}; + +int main() { + + int X = 0; + assert(X == 1); + + assert(X = 1); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() with side effect [misc-assert-side-effect] + my_assert(X = 1); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found my_assert() with side effect + not_my_assert(X = 1); + + MyClass mc; + assert(mc.badFunc(0, 1)); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() with side effect + assert(mc.goodFunc(0, 1)); + + MyClass mc2; + assert(mc2 = mc); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() with side effect + + return 0; +} Index: test/clang-tidy/misc-static-assert.cpp =================================================================== --- test/clang-tidy/misc-static-assert.cpp +++ test/clang-tidy/misc-static-assert.cpp @@ -0,0 +1,56 @@ +// RUN: $(dirname %s)/check_clang_tidy.sh %s misc-static-assert %t +// REQUIRES: shell + +void abort() {} +#ifdef NDEBUG +#define assert(x) 1 +#else +#define assert(x) \ + if (!(x)) \ + abort() +#endif + +#define my_macro() assert(0 == 1) +// CHECK-FIXES: #define my_macro() assert(0 == 1) + +constexpr bool myfunc(int a, int b) { return a * b == 0; } + +class A { +public: + bool method() { return true; } +}; + +class B { +public: + constexpr bool method() { return true; } +}; + +template void doSomething(T t) { + assert(myfunc(1, 2)); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be replaced by static_assert() [misc-static-assert] + // CHECK-FIXES: {{^ }}static_assert(myfunc(1, 2)); + assert(t.method()); + // CHECK-FIXES: {{^ }}assert(t.method()); +} + +int main() { + my_macro(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be + // CHECK-FIXES: {{^ }}my_macro(); + + assert(myfunc(1, 2) && (3 == 4)); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be + // CHECK-FIXES: {{^ }}static_assert(myfunc(1, 2) && (3 == 4)); + + int x = 1; + assert(x == 0); + // CHECK-FIXES: {{^ }}assert(x == 0); + + A a; + B b; + + doSomething(a); + doSomething(b); + + return 0; +}