JonasToth updated this revision to Diff 113252.
JonasToth added a comment.
- added inheritance cases, adjusted matcher is required
https://reviews.llvm.org/D37060
Files:
clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
test/clang-tidy/hicpp-exception-baseclass.cpp
Index: test/clang-tidy/hicpp-exception-baseclass.cpp
===================================================================
--- test/clang-tidy/hicpp-exception-baseclass.cpp
+++ test/clang-tidy/hicpp-exception-baseclass.cpp
@@ -5,38 +5,152 @@
} // namespace std
class derived_exception : public std::exception {};
+class deep_hierarchy : public derived_exception {};
class non_derived_exception {};
+class bad_inheritance : private std::exception {};
+class no_good_inheritance : protected std::exception {};
+class terrible_idead : public non_derived_exception, public derived_exception {};
+class really_creative : public non_derived_exception, private std::exception {};
void problematic() {
try {
- throw int(42); // Built in is not allowed
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: throwing an exception whose type is not derived from 'std::exception'
+ throw int(42);
+ // CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
} catch (int e) {
}
- throw int(42); // Bad
-// CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is not derived from 'std::exception'
+ throw int(42);
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
try {
- throw non_derived_exception(); // Some class is not allowed
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: throwing an exception whose type is not derived from 'std::exception'
-// CHECK-MESSAGES: 8:1: note: type defined here
+ throw 12;
+ // CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+ } catch (...) {
+ throw; // FIXME: This throw is not catched, but known to be of type 'int'
+ }
+
+ try {
+ throw non_derived_exception();
+ // CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+ // CHECK-MESSAGES: 9:1: note: type defined here
} catch (non_derived_exception &e) {
}
- throw non_derived_exception(); // Bad
-// CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is not derived from 'std::exception'
-// CHECK-MESSAGES: 8:1: note: type defined here
+ throw non_derived_exception();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+ // CHECK-MESSAGES: 9:1: note: type defined here
+
+ // Handle private inheritance cases correctly.
+ try {
+ throw bad_inheritance();
+ throw no_good_inheritance();
+ throw really_creative();
+ } catch (...) {
+ }
+ throw bad_inheritance();
+ throw no_good_inheritance();
+ throw really_creative();
}
void allowed_throws() {
try {
- throw std::exception(); // Ok
+ throw std::exception(); // Ok
} catch (std::exception &e) { // Ok
}
throw std::exception();
try {
- throw derived_exception(); // Ok
+ throw derived_exception(); // Ok
} catch (derived_exception &e) { // Ok
}
throw derived_exception(); // Ok
+
+ try {
+ throw deep_hierarchy(); // Ok, multiple levels of inheritance
+ } catch (deep_hierarchy &e) { // Ok
+ }
+ throw deep_hierarchy(); // Ok
+
+ try {
+ throw terrible_idead(); // Ok, but multiple inheritance isn't clean
+ } catch (std::exception &e) { // Can be caught as std::exception, even with multiple inheritance
+ }
+ throw terrible_idead(); // Ok, but multiple inheritance
+}
+
+// Templated function that throws exception based on template type
+template <typename T>
+void ThrowException() { throw T(); }
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception<int>' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-3]]:31: warning: throwing an exception whose type 'bad_generic_exception<std::exception>' is not derived from 'std::exception'
+// CHECK-MESSAGES: 71:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-5]]:31: warning: throwing an exception whose type 'exotic_exception<non_derived_exception>' is not derived from 'std::exception'
+// CHECK-MESSAGES: 74:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-7]]:31: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+// CHECK-MESSAGES: [[@LINE-8]]:31: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: 9:1: note: type defined here
+#define THROW_EXCEPTION(CLASS) ThrowException<CLASS>()
+#define THROW_BAD_EXCEPTION throw int(42);
+#define THROW_GOOD_EXCEPTION throw std::exception();
+#define THROW_DERIVED_EXCEPTION throw deep_hierarchy();
+
+template <typename T>
+class generic_exception : std::exception {};
+
+template <typename T>
+class bad_generic_exception {};
+
+template <typename T>
+class exotic_exception : public T {};
+
+void generic_exceptions() {
+ THROW_EXCEPTION(int);
+ // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+ THROW_EXCEPTION(non_derived_exception);
+ // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+ // CHECK MESSAGES: 9:1: note: type defined here
+ THROW_EXCEPTION(std::exception); // Ok
+ THROW_EXCEPTION(derived_exception); // Ok
+ THROW_EXCEPTION(deep_hierarchy); // Ok
+
+ THROW_BAD_EXCEPTION;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+ // CHECK-MESSAGES: [[@LINE-25]]:35: note: expanded from macro 'THROW_BAD_EXCEPTION'
+ THROW_GOOD_EXCEPTION;
+ THROW_DERIVED_EXCEPTION;
+
+ throw generic_exception<int>(); // Ok,
+ THROW_EXCEPTION(generic_exception<float>); // Ok
+
+ throw bad_generic_exception<int>();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_generic_exception<int>' is not derived from 'std::exception'
+ throw bad_generic_exception<std::exception>();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_generic_exception<std::exception>' is not derived from 'std::exception'
+ THROW_EXCEPTION(bad_generic_exception<int>);
+ // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'bad_generic_exception<int>' is not derived from 'std::exception'
+ THROW_EXCEPTION(bad_generic_exception<std::exception>);
+ // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'bad_generic_exception<std::exception>' is not derived from 'std::exception'
+
+ throw exotic_exception<non_derived_exception>();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'exotic_exception<non_derived_exception>' is not derived from 'std::exception'
+ THROW_EXCEPTION(exotic_exception<non_derived_exception>);
+ // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'exotic_exception<non_derived_exception>' is not derived from 'std::exception'
+
+ throw exotic_exception<derived_exception>(); // Ok
+ THROW_EXCEPTION(exotic_exception<derived_exception>); // Ok
+}
+
+// Test for typedefed exception types
+typedef int TypedefedBad;
+typedef derived_exception TypedefedGood;
+using UsingBad = int;
+using UsingGood = deep_hierarchy;
+
+void typedefed() {
+ throw TypedefedBad();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'TypedefedBad' (aka 'int') is not derived from 'std::exception'
+ throw TypedefedGood(); // Ok
+
+ throw UsingBad();
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'UsingBad' (aka 'int') is not derived from 'std::exception'
+ throw UsingGood(); // Ok
}
Index: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
===================================================================
--- clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
+++ clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
@@ -11,8 +11,6 @@
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include <iostream>
-
using namespace clang::ast_matchers;
namespace clang {
@@ -24,20 +22,23 @@
return;
Finder->addMatcher(
- cxxThrowExpr(
- allOf(
- has(expr(unless(hasType(cxxRecordDecl(
- isSameOrDerivedFrom(hasName("std::exception"))))))),
- eachOf(has(expr(hasType(namedDecl().bind("decl")))), anything())))
+ cxxThrowExpr(allOf(has(expr(unless(hasType(qualType(hasCanonicalType(
+ hasDeclaration(cxxRecordDecl(isSameOrDerivedFrom(
+ hasName("std::exception")))))))))),
+ has(expr(unless(cxxUnresolvedConstructExpr()))),
+ eachOf(has(expr(hasType(namedDecl().bind("decl")))),
+ anything())))
.bind("bad_throw"),
this);
}
void ExceptionBaseclassCheck::check(const MatchFinder::MatchResult &Result) {
const auto *BadThrow = Result.Nodes.getNodeAs<CXXThrowExpr>("bad_throw");
- diag(BadThrow->getLocStart(),
- "throwing an exception whose type is not derived from 'std::exception'")
- << BadThrow->getSourceRange();
+
+ diag(BadThrow->getSubExpr()->getLocStart(), "throwing an exception whose "
+ "type %0 is not derived from "
+ "'std::exception'")
+ << BadThrow->getSubExpr()->getType() << BadThrow->getSourceRange();
const auto *TypeDecl = Result.Nodes.getNodeAs<NamedDecl>("decl");
if (TypeDecl != nullptr)
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits