Fixed style issues. Added a conditional operator-based assert() to
misc-assert-side-effect.cpp.
StaticAssertCheck:
-added new FixIt cases: assert((...) && "AssertMsg") --> static_assert((...),
"AssertMsg")
-added test cases for each FixIt case
http://reviews.llvm.org/D7375
Files:
clang-tidy/misc/AssertSideEffectCheck.cpp
clang-tidy/misc/AssertSideEffectCheck.h
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/StaticAssertCheck.cpp
clang-tidy/misc/StaticAssertCheck.h
test/clang-tidy/misc-assert-side-effect.cpp
test/clang-tidy/misc-static-assert.cpp
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: clang-tidy/misc/AssertSideEffectCheck.cpp
===================================================================
--- clang-tidy/misc/AssertSideEffectCheck.cpp
+++ clang-tidy/misc/AssertSideEffectCheck.cpp
@@ -0,0 +1,114 @@
+//===--- 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 <algorithm>
+#include <string>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace ast_matchers {
+
+AST_MATCHER_P(Expr, hasSideEffect, bool, CheckFunctionCalls) {
+ const Expr *E = &Node;
+
+ if (const auto *Op = llvm::dyn_cast<UnaryOperator>(E)) {
+ UnaryOperator::Opcode OC = Op->getOpcode();
+ return OC == UO_PostInc || OC == UO_PostDec || OC == UO_PreInc ||
+ OC == UO_PreDec;
+ }
+
+ if (const auto *Op = llvm::dyn_cast<BinaryOperator>(E)) {
+ BinaryOperator::Opcode OC = Op->getOpcode();
+ return 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;
+ }
+
+ if (const auto *CExpr = llvm::dyn_cast<CallExpr>(E)) {
+ const auto *OpCallExpr = llvm::dyn_cast<CXXOperatorCallExpr>(E);
+ if (OpCallExpr) {
+ OverloadedOperatorKind OpKind = OpCallExpr->getOperator();
+ return 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;
+ }
+
+ bool RetVal = CheckFunctionCalls;
+ if (const auto *FuncDecl = CExpr->getDirectCallee())
+ if (const auto *MethDecl = llvm::dyn_cast<CXXMethodDecl>(FuncDecl))
+ RetVal &= !MethDecl->isConst();
+ return RetVal;
+ }
+
+ return llvm::isa<CXXNewExpr>(E) || llvm::isa<CXXDeleteExpr>(E) ||
+ llvm::isa<CXXThrowExpr>(E);
+}
+
+} // 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) {
+ const auto ConditionWithSideEffect =
+ hasCondition(hasDescendant(expr(hasSideEffect(CheckFunctionCalls))));
+ Finder->addMatcher(
+ stmt(anyOf(conditionalOperator(ConditionWithSideEffect),
+ ifStmt(ConditionWithSideEffect))).bind("condStmt"),
+ this);
+}
+
+void AssertSideEffectCheck::check(const MatchFinder::MatchResult &Result) {
+ ASTContext *const ASTCtx = Result.Context;
+ const auto *CondStmt = Result.Nodes.getNodeAs<Stmt>("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/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 <string>
+
+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<llvm::StringRef, 5> 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/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -2,11 +2,13 @@
add_clang_library(clangTidyMiscModule
ArgumentCommentCheck.cpp
+ AssertSideEffectCheck.cpp
AssignOperatorSignatureCheck.cpp
BoolPointerImplicitConversion.cpp
InaccurateEraseCheck.cpp
InefficientAlgorithmCheck.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,10 +11,12 @@
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "ArgumentCommentCheck.h"
+#include "AssertSideEffectCheck.h"
#include "AssignOperatorSignatureCheck.h"
#include "BoolPointerImplicitConversion.h"
#include "InaccurateEraseCheck.h"
#include "InefficientAlgorithmCheck.h"
+#include "StaticAssertCheck.h"
#include "SwappedArgumentsCheck.h"
#include "UndelegatedConstructor.h"
#include "UniqueptrResetRelease.h"
@@ -28,6 +30,8 @@
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<ArgumentCommentCheck>("misc-argument-comment");
+ CheckFactories.registerCheck<AssertSideEffectCheck>(
+ "misc-assert-side-effect");
CheckFactories.registerCheck<AssignOperatorSignatureCheck>(
"misc-assign-operator-signature");
CheckFactories.registerCheck<BoolPointerImplicitConversion>(
@@ -36,6 +40,8 @@
"misc-inaccurate-erase");
CheckFactories.registerCheck<InefficientAlgorithmCheck>(
"misc-inefficient-algorithm");
+ CheckFactories.registerCheck<StaticAssertCheck>(
+ "misc-static-assert");
CheckFactories.registerCheck<SwappedArgumentsCheck>(
"misc-swapped-arguments");
CheckFactories.registerCheck<UndelegatedConstructorCheck>(
Index: clang-tidy/misc/StaticAssertCheck.cpp
===================================================================
--- clang-tidy/misc/StaticAssertCheck.cpp
+++ clang-tidy/misc/StaticAssertCheck.cpp
@@ -0,0 +1,128 @@
+//===--- 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 <string>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+StaticAssertCheck::StaticAssertCheck(llvm::StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+
+void StaticAssertCheck::registerMatchers(MatchFinder *Finder) {
+ const auto IsAlwaysFalse = ignoringParenImpCasts(
+ anyOf(boolLiteral(equals(false)).bind("isAlwaysFalse"),
+ integerLiteral(equals(0)).bind("isAlwaysFalse")));
+ const auto AssertExprRoot = anyOf(
+ binaryOperator(
+ hasOperatorName("&&"),
+ hasEitherOperand(ignoringImpCasts(stringLiteral().bind("assertMSG"))),
+ anyOf(binaryOperator(hasEitherOperand(IsAlwaysFalse)), anything()))
+ .bind("assertExprRoot"),
+ IsAlwaysFalse);
+ const auto Condition = expr(anyOf(
+ expr(ignoringParenCasts(anyOf(
+ AssertExprRoot,
+ unaryOperator(hasUnaryOperand(ignoringParenCasts(AssertExprRoot)))))),
+ anything()));
+
+ Finder->addMatcher(
+ stmt(anyOf(conditionalOperator(hasCondition(Condition.bind("condition"))),
+ ifStmt(hasCondition(Condition.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<Stmt>("condStmt");
+ const auto *Condition = Result.Nodes.getNodeAs<Expr>("condition");
+ const auto *IsAlwaysFalse = Result.Nodes.getNodeAs<Expr>("isAlwaysFalse");
+ const auto *AssertMSG = Result.Nodes.getNodeAs<StringLiteral>("assertMSG");
+ const auto *AssertExprRoot =
+ Result.Nodes.getNodeAs<BinaryOperator>("assertExprRoot");
+ const SourceLocation AssertExpansionLoc = CondStmt->getLocStart();
+
+ if (!Opts.CPlusPlus11 || !AssertExpansionLoc.isValid() ||
+ !AssertExpansionLoc.isMacroID())
+ return;
+
+ llvm::StringRef MacroName = Lexer::getImmediateMacroName(
+ AssertExpansionLoc, ASTCtx->getSourceManager(), Opts);
+
+ if (MacroName != "assert" || !Condition->isEvaluatable(*ASTCtx))
+ return;
+
+ // False literal is not the result of macro expansion.
+ if (IsAlwaysFalse &&
+ !SM.getImmediateSpellingLoc(IsAlwaysFalse->getExprLoc()).isMacroID())
+ return;
+
+ const SourceLocation AssertLoc =
+ SM.getImmediateMacroCallerLoc(AssertExpansionLoc);
+
+ FixItHint NameHint, AssertExprRootHint, SLitHint, ArgHint;
+ if (AssertLoc.isValid() && !AssertLoc.isMacroID()) {
+ NameHint =
+ FixItHint::CreateReplacement(SourceRange(AssertLoc), "static_assert");
+
+ std::string StaticAssertMSG = ", \"\"";
+ if (AssertExprRoot) {
+ AssertExprRootHint = FixItHint::CreateRemoval(
+ SourceRange(AssertExprRoot->getOperatorLoc()));
+ SLitHint = FixItHint::CreateRemoval(
+ SourceRange(AssertMSG->getLocStart(), AssertMSG->getLocEnd()));
+ StaticAssertMSG =
+ (llvm::Twine(", \"") + AssertMSG->getString() + "\"").str();
+ }
+
+ llvm::MemoryBuffer *Buffer = SM.getBuffer(SM.getFileID(AssertLoc));
+ if (!Buffer)
+ return;
+
+ const char *BufferPos = SM.getCharacterData(AssertLoc);
+
+ Token Token;
+ Lexer Lexer(SM.getLocForStartOfFile(SM.getFileID(AssertLoc)), Opts,
+ Buffer->getBufferStart(), BufferPos, Buffer->getBufferEnd());
+ Lexer.SetCommentRetentionState(false);
+
+ while (!Lexer.LexFromRawLexer(Token) && Token.isNot(tok::l_paren))
+ ;
+
+ unsigned int ParenCount = 1;
+ while (ParenCount && !Lexer.LexFromRawLexer(Token)) {
+ if (Token.is(tok::l_paren))
+ ++ParenCount;
+ if (Token.is(tok::r_paren))
+ --ParenCount;
+ }
+
+ ArgHint = FixItHint::CreateInsertion(Token.getLocation(), StaticAssertMSG);
+ }
+
+ diag(AssertLoc, "found assert() that could be replaced by static_assert()")
+ << NameHint << AssertExprRootHint << SLitHint << ArgHint;
+}
+
+} // namespace tidy
+} // namespace clang
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 <string>
+
+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: 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 ------------------------------------------===//
+int abort() { return 0; }
+
+#ifdef NDEBUG
+#define assert(x) 1
+#else
+#define assert(x) \
+ if (!(x)) \
+ (void)abort()
+#endif
+
+#ifdef NDEBUG
+#define my_assert(x) 1
+#else
+#define my_assert(x) \
+ ((void)((x) ? 1 : abort()))
+#endif
+
+#ifdef NDEBUG
+#define not_my_assert(x) 1
+#else
+#define not_my_assert(x) \
+ if (!(x)) \
+ (void)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,80 @@
+// 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 ZERO_MACRO 0
+
+#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 <class T> 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>(a);
+ doSomething<B>(b);
+
+ assert(false);
+ // CHECK-FIXES: {{^ }}assert(false);
+
+ assert(ZERO_MACRO);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be
+ // CHECK-FIXES: {{^ }}static_assert(ZERO_MACRO, "");
+
+ assert(0 && "Don't report me!");
+ // CHECK-FIXES: {{^ }}assert(0 && "Don't report me!");
+
+ assert(false && "Don't report me!");
+ // CHECK-FIXES: {{^ }}assert(false && "Don't report me!");
+
+ assert(ZERO_MACRO && "Report me!");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be
+ // CHECK-FIXES: {{^ }}static_assert(ZERO_MACRO , "Report me!");
+
+ assert(10==5 && "Report me!");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be
+ // CHECK-FIXES: {{^ }}static_assert(10==5 , "Report me!");
+
+ return 0;
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
