diff options
author | Jonas Toth <jonas.toth@gmail.com> | 2018-09-17 13:55:10 +0000 |
---|---|---|
committer | Jonas Toth <jonas.toth@gmail.com> | 2018-09-17 13:55:10 +0000 |
commit | 00a59116251af48ae58c301873e4397b0261daeb (patch) | |
tree | e5fa36e840c96c9b1733383be980114cf9f2c86b /clang-tidy/hicpp | |
parent | 77ed1cd838a249d6134de9a6bdbe17ef46ecf946 (diff) |
[clang-tidy] fix PR37913, templated exception factory diagnosed correctly
Summary:
PR37913 documents wrong behaviour for a templated exception factory function.
The check does misidentify dependent types as not derived from std::exception.
The fix to this problem is to ignore dependent types, the analysis works correctly
on the instantiated function.
Reviewers: aaron.ballman, alexfh, hokein, ilya-biryukov
Reviewed By: alexfh
Subscribers: lebedev.ri, nemanjai, mgorny, kbarton, xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D48714
git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@342393 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'clang-tidy/hicpp')
-rw-r--r-- | clang-tidy/hicpp/ExceptionBaseclassCheck.cpp | 34 |
1 files changed, 26 insertions, 8 deletions
diff --git a/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp b/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp index 3ea56d2f..b299151c 100644 --- a/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp +++ b/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp @@ -22,26 +22,44 @@ void ExceptionBaseclassCheck::registerMatchers(MatchFinder *Finder) { 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( + unless(has(expr(anyOf(isTypeDependent(), isValueDependent())))), + // 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()), + // 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); } |