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 - 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("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("decl"); if (TypeDecl != nullptr) 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,175 @@ } // namespace std class derived_exception : public std::exception {}; +class deep_hierarchy : public derived_exception {}; class non_derived_exception {}; +class terrible_idea : public non_derived_exception, public derived_exception {}; + +// 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 +class bad_inheritance : private std::exception {}; +class no_good_inheritance : protected std::exception {}; +class really_creative : public non_derived_exception, private std::exception {}; +#endif 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 12; + // 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(); // 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 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 + +// 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 + 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 + 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 + } 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 + 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 + 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 +#endif } 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_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 +void ThrowException() { throw T(); } +// CHECK-MESSAGES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception' +// CHECK-MESSAGES: 120:1: note: type defined here +// CHECK-MESSAGES: [[@LINE-3]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception' +// CHECK-MESSAGES: 120:1: note: type defined here +// CHECK-MESSAGES: [[@LINE-5]]:31: warning: throwing an exception whose type 'exotic_exception' is not derived from 'std::exception' +// CHECK-MESSAGES: 123: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() +#define THROW_BAD_EXCEPTION throw int(42); +#define THROW_GOOD_EXCEPTION throw std::exception(); +#define THROW_DERIVED_EXCEPTION throw deep_hierarchy(); + +template +class generic_exception : std::exception {}; + +template +class bad_generic_exception {}; + +template +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(); // Ok, + THROW_EXCEPTION(generic_exception); // Ok + + throw bad_generic_exception(); + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception' + throw bad_generic_exception(); + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception' + THROW_EXCEPTION(bad_generic_exception); + // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception' + THROW_EXCEPTION(bad_generic_exception); + // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception' + + throw exotic_exception(); + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'exotic_exception' is not derived from 'std::exception' + THROW_EXCEPTION(exotic_exception); + // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'exotic_exception' is not derived from 'std::exception' + + throw exotic_exception(); // Ok + THROW_EXCEPTION(exotic_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' + // CHECK-MESSAGES: 164:1: note: type defined here + 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' + // CHECK-MESSAGES: 166:1: note: type defined here + throw UsingGood(); // Ok }