randomcppprogrammer added a reviewer: aaron.ballman.
randomcppprogrammer updated this revision to Diff 34111.
randomcppprogrammer added a comment.
Updated check according to comments given by Aaron Ballman. Most notable
change: added optional check for throwing anonmyous temporaries.
http://reviews.llvm.org/D11328
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
Index: test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
@@ -0,0 +1,138 @@
+// RUN: %python %S/check_clang_tidy.py %s misc-throw-by-value-catch-by-reference %t
+
+//#include <utility>
+
+class logic_error {
+public:
+ logic_error(const char *message) {}
+};
+
+typedef logic_error *logic_ptr;
+typedef logic_ptr logic_double_typedef;
+
+int lastException;
+
+logic_error CreateException() { return logic_error("created"); }
+
+void testThrowFunc() {
+ throw new logic_error("by pointer");
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: avoid throwing pointer
+ // [misc-throw-by-value-catch-by-reference]
+ logic_ptr tmp = new logic_error("by pointer");
+ throw tmp;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: avoid throwing pointer
+ // [misc-throw-by-value-catch-by-reference]
+ // CHECK-MESSAGES: :[[@LINE-3]]:9: warning: prefer throwing anonymous
+ // temporaries [misc-throw-by-value-catch-by-reference]
+ throw logic_error("by value");
+ auto *literal = "test";
+ throw logic_error(literal);
+ throw "test string literal";
+ throw L"throw wide string literal";
+ const char *characters = 0;
+ throw characters;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: avoid throwing pointer
+ // [misc-throw-by-value-catch-by-reference]
+ // CHECK-MESSAGES: :[[@LINE-3]]:9: warning: prefer throwing anonymous
+ // temporaries [misc-throw-by-value-catch-by-reference]
+ logic_error lvalue("lvalue");
+ throw lvalue;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer throwing anonymous
+ // temporaries [misc-throw-by-value-catch-by-reference]
+
+ // can be enabled once std::move can be included
+ // throw std::move(lvalue)
+ int &ex = lastException;
+ throw ex;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer throwing anonymous
+ // temporaries [misc-throw-by-value-catch-by-reference]
+ throw CreateException();
+}
+
+void throwReferenceFunc(logic_error &ref) {
+ throw ref;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer throwing anonymous
+ // temporaries [misc-throw-by-value-catch-by-reference]
+}
+
+void catchByPointer() {
+ try {
+ testThrowFunc();
+ } catch (logic_error *e) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch by reference
+ // [misc-throw-by-value-catch-by-reference]
+ }
+}
+
+void catchByValue() {
+ try {
+ testThrowFunc();
+ } catch (logic_error e) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch by reference
+ // [misc-throw-by-value-catch-by-reference]
+ }
+}
+
+void catchByReference() {
+ try {
+ testThrowFunc();
+ } catch (logic_error &e) {
+ }
+}
+
+void catchByConstReference() {
+ try {
+ testThrowFunc();
+ } catch (const logic_error &e) {
+ }
+}
+
+void catchTypedef() {
+ try {
+ testThrowFunc();
+ } catch (logic_ptr) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch by reference
+ // [misc-throw-by-value-catch-by-reference]
+ }
+}
+
+void catchAll() {
+ try {
+ testThrowFunc();
+ } catch (...) {
+ }
+}
+
+void catchLiteral() {
+ try {
+ testThrowFunc();
+ } catch (const char *) {
+ } catch (const wchar_t *) {
+ // disabled for now until it is clear
+ // how to enable them in the test
+ //} catch (const char16_t*) {
+ //} catch (const char32_t*) {
+ }
+}
+
+// catching fundamentals should not warn
+void catchFundamental() {
+ try {
+ testThrowFunc();
+ } catch (int) {
+ } catch (double) {
+ } catch (unsigned long) {
+ }
+}
+
+struct TrivialType {
+ double x;
+ double y;
+};
+
+void catchTrivial() {
+ try {
+ testThrowFunc();
+ } catch (TrivialType) {
+ }
+}
Index: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
@@ -0,0 +1,43 @@
+//===--- ThrowByValueCatchByReferenceCheck.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_THROW_BY_VALUE_CATCH_BY_REFERENCE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_THROW_BY_VALUE_CATCH_BY_REFERENCE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+///\brief checks for locations that do not throw by value
+// or catch by reference.
+// The check is C++ only. It checks that all throw locations
+// throw by value and not by pointer. Additionally it
+// contains an option ("CheckThrowTemporaries", default value "true") that
+// checks that thrown objects are anonymous temporaries. It is also
+// acceptable for this check to throw string literals.
+// This test checks that exceptions are caught by reference
+// and not by value or pointer. It will not warn when catching
+// pointer to char, wchar_t, char16_t or char32_t. This is
+// due to not warning on throwing string literals.
+class ThrowByValueCatchByReferenceCheck : public ClangTidyCheck {
+public:
+ ThrowByValueCatchByReferenceCheck(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;
+
+private:
+ bool checkAnonymousTemporaries;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_THROW_BY_VALUE_CATCH_BY_REFERENCE_H
Index: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
@@ -0,0 +1,137 @@
+//===--- ThrowByValueCatchByReferenceCheck.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 "ThrowByValueCatchByReferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/AST/OperationKinds.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+ThrowByValueCatchByReferenceCheck::ThrowByValueCatchByReferenceCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ checkAnonymousTemporaries(Options.get("CheckThrowTemporaries", "true") ==
+ "true") {}
+
+void ThrowByValueCatchByReferenceCheck::registerMatchers(MatchFinder *Finder) {
+ // Only register the matchers for C++
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ Finder->addMatcher(throwExpr().bind("throw"), this);
+ Finder->addMatcher(catchStmt().bind("catch"), this);
+}
+
+void ThrowByValueCatchByReferenceCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "CheckThrowTemporaries", "true");
+}
+
+void ThrowByValueCatchByReferenceCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *throwExpr = Result.Nodes.getNodeAs<CXXThrowExpr>("throw");
+ if (throwExpr != nullptr) {
+ auto *subExpr = throwExpr->getSubExpr();
+ auto qualType = subExpr->getType();
+ auto *type = qualType.getTypePtr();
+ if (type->isPointerType()) { // throwing a pointer
+ // check for strng literal - which is safe
+ // e.g. throw "simple exception"
+ if (isa<ImplicitCastExpr>(subExpr)) {
+ auto *inner = cast<ImplicitCastExpr>(subExpr)->getSubExpr();
+ if (isa<StringLiteral>(inner))
+ return;
+ }
+ diag(subExpr->getLocStart(), "avoid throwing pointer");
+ }
+ // if not thrown by pointer, then it has been thrown
+ // by value, which is OK.
+ // additional check if thrown value is a RValue
+ // according to guideline:
+ // https://www.securecoding.cert.org/confluence/display/cplusplus/ERR09-CPP.+Throw+anonymous+temporaries
+
+ // algorithm
+ // from CXXThrowExpr, move through all casts until you either encounter an
+ // LValueToRValue-Cast or a
+ // CXXConstructExpr.
+ // if it's a LValueToRValue-Cast, emit message
+ // if it's a CopyOrMoveConstructor, emit message if after casts, it
+ // contains a DeclRefExpr for the parameter.
+ // otherwise - do not emit a message.
+ if (checkAnonymousTemporaries) {
+ bool emit = false;
+ auto *currentSubExpr = subExpr;
+ while (isa<CastExpr>(currentSubExpr)) {
+ auto *currentCast = cast<CastExpr>(currentSubExpr);
+ if (currentCast->getCastKind() == clang::CK_LValueToRValue) {
+ emit = true;
+ break;
+ }
+ }
+ if (!emit) {
+ // Check for copy construction
+ // we're now through all potential casts. This should be a
+ // CXXConstructExpr in most cases
+ const CXXConstructExpr *constructorCall;
+ if (isa<CXXConstructExpr>(currentSubExpr))
+ constructorCall = cast<CXXConstructExpr>(currentSubExpr);
+ else
+ constructorCall = nullptr;
+ // if it's a copy constructor it could copy from a lvalue
+ if (constructorCall &&
+ constructorCall->getConstructor()->isCopyOrMoveConstructor()) {
+ // again skip all potential casts
+ auto argIter =
+ constructorCall
+ ->arg_begin(); // there's only one for copy constructors
+ auto *currentSubExpr = *argIter;
+ while (isa<CastExpr>(currentSubExpr)) {
+ auto *currentCast = cast<CastExpr>(currentSubExpr);
+ currentSubExpr = currentCast->getSubExpr();
+ }
+ // if the subexpr is now a DeclRefExpr, it's a real variable
+ if (isa<DeclRefExpr>(currentSubExpr))
+ emit = true;
+ }
+ }
+ if (emit)
+ diag(subExpr->getLocStart(), "prefer throwing anonymous temporaries");
+ }
+ }
+ const auto *catchStmt = Result.Nodes.getNodeAs<CXXCatchStmt>("catch");
+ if (catchStmt != nullptr) {
+ auto caughtType = catchStmt->getCaughtType();
+ if (caughtType.isNull())
+ return;
+ auto *type = caughtType.getTypePtr();
+ auto *varDecl = catchStmt->getExceptionDecl();
+ if (type->isPointerType()) {
+ auto canonical = type->getCanonicalTypeInternal().getTypePtr();
+ // check if pointed-to-type is char, wchar_t, char16_t, char32_t
+ auto *pointeeType =
+ cast<PointerType>(canonical)->getPointeeType().getTypePtr();
+ if (pointeeType->isAnyCharacterType() == false) {
+ diag(varDecl->getLocStart(), "catch by reference");
+ }
+ } else if (!type->isReferenceType()) {
+ // not pointer, not reference this means it must be "by value"
+ // do not advice on built-in types - catching them by value
+ // is ok
+ if (!caughtType.isTrivialType(*Result.Context))
+ diag(varDecl->getLocStart(), "catch by reference");
+ }
+ }
+}
+
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -22,6 +22,7 @@
#include "NoexceptMoveConstructorCheck.h"
#include "StaticAssertCheck.h"
#include "SwappedArgumentsCheck.h"
+#include "ThrowByValueCatchByReferenceCheck.h"
#include "UndelegatedConstructor.h"
#include "UniqueptrResetReleaseCheck.h"
#include "UnusedAliasDeclsCheck.h"
@@ -58,6 +59,8 @@
"misc-static-assert");
CheckFactories.registerCheck<SwappedArgumentsCheck>(
"misc-swapped-arguments");
+ CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(
+ "misc-throw-by-value-catch-by-reference");
CheckFactories.registerCheck<UndelegatedConstructorCheck>(
"misc-undelegated-constructor");
CheckFactories.registerCheck<UniqueptrResetReleaseCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -14,6 +14,7 @@
NoexceptMoveConstructorCheck.cpp
StaticAssertCheck.cpp
SwappedArgumentsCheck.cpp
+ ThrowByValueCatchByReferenceCheck.cpp
UndelegatedConstructor.cpp
UnusedAliasDeclsCheck.cpp
UnusedParametersCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
