JonasToth updated this revision to Diff 162945.
JonasToth added a comment.
- [Misc] comment the matcher to better understand it
- [Fix] adjust the test cases to properly function now
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48714
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
@@ -2,6 +2,7 @@
namespace std {
class exception {};
+class invalid_argument : public exception {};
} // namespace std
class derived_exception : public std::exception {};
@@ -20,54 +21,54 @@
void problematic() {
try {
throw int(42);
- // CHECK-NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+ // CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
} catch (int e) {
}
throw int(42);
- // CHECK-NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
try {
throw 12;
- // CHECK-NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+ // CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
} catch (...) {
throw; // Ok, even if the type is not known, conforming code can never rethrow a non-std::exception object.
}
try {
throw non_derived_exception();
- // CHECK-NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
- // CHECK-NOTES: 9:1: note: type defined here
+ // CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+ // CHECK-MESSAGES: 10:1: note: type defined here
} catch (non_derived_exception &e) {
}
throw non_derived_exception();
- // CHECK-NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
- // CHECK-NOTES: 9:1: note: type defined here
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+ // CHECK-MESSAGES: 10:1: note: type defined here
// FIXME: More complicated kinds of inheritance should be checked later, but there is
// currently no way use ASTMatchers for this kind of task.
#if 0
// Handle private inheritance cases correctly.
try {
throw bad_inheritance();
// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
- // CHECK MESSAGES: 10:1: note: type defined here
+ // CHECK MESSAGES: 11:1: note: type defined here
throw no_good_inheritance();
// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
- // CHECK MESSAGES: 11:1: note: type defined here
+ // CHECK MESSAGES: 12:1: note: type defined here
throw really_creative();
// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
- // CHECK MESSAGES: 12:1: note: type defined here
+ // CHECK MESSAGES: 13:1: note: type defined here
} catch (...) {
}
throw bad_inheritance();
// CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
- // CHECK MESSAGES: 10:1: note: type defined here
+ // CHECK MESSAGES: 11:1: note: type defined here
throw no_good_inheritance();
// CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
- // CHECK MESSAGES: 11:1: note: type defined here
+ // CHECK MESSAGES: 12:1: note: type defined here
throw really_creative();
// CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
- // CHECK MESSAGES: 12:1: note: type defined here
+ // CHECK MESSAGES: 13:1: note: type defined here
#endif
}
@@ -91,24 +92,32 @@
throw deep_hierarchy(); // Ok
try {
- throw terrible_idea(); // Ok, but multiple inheritance isn't clean
+ throw terrible_idea(); // Ok, but multiple inheritance isn't clean
} catch (std::exception &e) { // Can be caught as std::exception, even with multiple inheritance
}
throw terrible_idea(); // Ok, but multiple inheritance
}
// Templated function that throws exception based on template type
template <typename T>
void ThrowException() { throw T(); }
-// CHECK-NOTES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception<int>' is not derived from 'std::exception'
-// CHECK-NOTES: 120:1: note: type defined here
-// CHECK-NOTES: [[@LINE-3]]:31: warning: throwing an exception whose type 'bad_generic_exception<std::exception>' is not derived from 'std::exception'
-// CHECK-NOTES: 120:1: note: type defined here
-// CHECK-NOTES: [[@LINE-5]]:31: warning: throwing an exception whose type 'exotic_exception<non_derived_exception>' is not derived from 'std::exception'
-// CHECK-NOTES: 123:1: note: type defined here
-// CHECK-NOTES: [[@LINE-7]]:31: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
-// CHECK-NOTES: [[@LINE-8]]:31: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
-// CHECK-NOTES: 9:1: note: type defined here
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception<int>' is not derived from 'std::exception'
+// CHECK-MESSAGES: [[@LINE-2]]:31: note: type 'bad_generic_exception<int>' is a template instantiation of 'T'
+
+// CHECK-MESSAGES: [[@LINE-4]]:31: warning: throwing an exception whose type 'bad_generic_exception<std::exception>' is not derived from 'std::exception'
+// CHECK-MESSAGES: [[@LINE-5]]:31: note: type 'bad_generic_exception<std::exception>' is a template instantiation of 'T'
+// CHECK-MESSAGES: 129:1: note: type defined here
+
+// CHECK-MESSAGES: [[@LINE-8]]:31: warning: throwing an exception whose type 'exotic_exception<non_derived_exception>' is not derived from 'std::exception'
+// CHECK-MESSAGES: [[@LINE-9]]:31: note: type 'exotic_exception<non_derived_exception>' is a template instantiation of 'T'
+// CHECK-MESSAGES: 132:1: note: type defined here
+
+// CHECK-MESSAGES: [[@LINE-12]]:31: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+// CHECK-MESSAGES: [[@LINE-13]]:31: note: type 'int' is a template instantiation of 'T'
+
+// CHECK-MESSAGES: [[@LINE-15]]:31: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
+// CHECK-MESSAGES: [[@LINE-16]]:31: note: type 'non_derived_exception' is a template instantiation of 'T'
+
#define THROW_EXCEPTION(CLASS) ThrowException<CLASS>()
#define THROW_BAD_EXCEPTION throw int(42);
#define THROW_GOOD_EXCEPTION throw std::exception();
@@ -125,39 +134,33 @@
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-NOTES: [[@LINE-1]]:3: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
- // CHECK-NOTES: [[@LINE-25]]:35: note: expanded from macro 'THROW_BAD_EXCEPTION'
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+ // CHECK-MESSAGES: [[@LINE-22]]: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-NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_generic_exception<int>' is not derived from 'std::exception'
- // CHECK-NOTES: 120:1: note: type defined here
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_generic_exception<int>' is not derived from 'std::exception'
+ // CHECK-MESSAGES: 129:1: note: type defined here
throw bad_generic_exception<std::exception>();
- // CHECK-NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_generic_exception<std::exception>' is not derived from 'std::exception'
- // CHECK-NOTES: 120:1: note: type defined here
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_generic_exception<std::exception>' is not derived from 'std::exception'
+ // CHECK-MESSAGES: 129:1: note: type defined here
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-NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'exotic_exception<non_derived_exception>' is not derived from 'std::exception'
- // CHECK-NOTES: 123:1: note: type defined here
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'exotic_exception<non_derived_exception>' is not derived from 'std::exception'
+ // CHECK-MESSAGES: 132:1: note: type defined here
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
@@ -171,12 +174,30 @@
void typedefed() {
throw TypedefedBad();
- // CHECK-NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'TypedefedBad' (aka 'int') is not derived from 'std::exception'
- // CHECK-NOTES: 167:1: note: type defined here
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'TypedefedBad' (aka 'int') is not derived from 'std::exception'
+ // CHECK-MESSAGES: 170:1: note: type defined here
throw TypedefedGood(); // Ok
throw UsingBad();
- // CHECK-NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'UsingBad' (aka 'int') is not derived from 'std::exception'
- // CHECK-NOTES: 169:1: note: type defined here
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'UsingBad' (aka 'int') is not derived from 'std::exception'
+ // CHECK-MESSAGES: 172:1: note: type defined here
throw UsingGood(); // Ok
}
+
+// Fix PR37913
+struct invalid_argument_maker {
+ ::std::invalid_argument operator()() const;
+};
+struct int_maker {
+ int operator()() const;
+};
+template <typename T>
+void templated_thrower() { throw T{}(); }
+// CHECK-MESSAGES: [[@LINE-1]]:34: warning: throwing an exception whose type 'int' is not derived from 'std::exception'
+
+void exception_created_with_function() {
+ templated_thrower<invalid_argument_maker>();
+ templated_thrower<int_maker>();
+
+ throw invalid_argument_maker{}();
+}
Index: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
===================================================================
--- clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
+++ clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
@@ -22,26 +22,46 @@
return;
Finder->addMatcher(
- 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())))
+ cxxThrowExpr(
+ allOf(
+ // The thrown value is not derived from 'std::exception'.
+ has(expr(unless(hasType(
+ qualType(hasCanonicalType(hasDeclaration(cxxRecordDecl(
+ isSameOrDerivedFrom(hasName("::std::exception")))))))))),
+ // This condition is always true, but will bind to the template
+ // value if the thrown type is templated.
+ anyOf(has(expr(hasType(
+ substTemplateTypeParmType().bind("templ_type")))),
+ anything()),
+ // Ignore uninstantiated templates to avoid false positives.
+ has(expr(allOf(
+ unless(cxxUnresolvedConstructExpr()),
+ unless(callExpr(callee(cxxUnresolvedConstructExpr())))))),
+ // Bind to the declaration of the type of the value that is
+ // thrown. 'anything()' is necessary to always suceed in the
+ // 'eachOf' because builtin types are not 'namedDecl'.
+ 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");
+ assert(BadThrow && "Did not match the throw expression");
diag(BadThrow->getSubExpr()->getBeginLoc(), "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)
+ if (const auto *Template =
+ Result.Nodes.getNodeAs<SubstTemplateTypeParmType>("templ_type"))
+ diag(BadThrow->getSubExpr()->getBeginLoc(),
+ "type %0 is a template instantiation of %1", DiagnosticIDs::Note)
+ << BadThrow->getSubExpr()->getType()
+ << Template->getReplacedParameter()->getDecl();
+
+ if (const auto *TypeDecl = Result.Nodes.getNodeAs<NamedDecl>("decl"))
diag(TypeDecl->getBeginLoc(), "type defined here", DiagnosticIDs::Note);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
