llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Aaron Puchert (aaronpuchert) <details> <summary>Changes</summary> It might not always be clear which base the check found that has "exception" in its name. So we add a note to point to that class. Ideally we'd point to the `CXXBaseSpecifier`, but it seems we can't bind to that. --- Full diff: https://github.com/llvm/llvm-project/pull/160751.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp (+7-1) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2-1) - (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/throw-keyword-missing.cpp (+6-3) ``````````diff diff --git a/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp index cafb4a3e5f0e5..9781f0a5ac9de 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp @@ -20,7 +20,8 @@ void ThrowKeywordMissingCheck::registerMatchers(MatchFinder *Finder) { hasType(cxxRecordDecl(anyOf( matchesName("[Ee]xception|EXCEPTION"), hasAnyBase(hasType(hasCanonicalType(recordType(hasDeclaration( - cxxRecordDecl(matchesName("[Ee]xception|EXCEPTION")))))))))), + cxxRecordDecl(matchesName("[Ee]xception|EXCEPTION")) + .bind("base"))))))))), unless(anyOf( hasAncestor( stmt(anyOf(cxxThrowExpr(), callExpr(), returnStmt()))), @@ -39,6 +40,11 @@ void ThrowKeywordMissingCheck::check(const MatchFinder::MatchResult &Result) { diag(TemporaryExpr->getBeginLoc(), "suspicious exception object created but " "not thrown; did you mean 'throw %0'?") << TemporaryExpr->getType().getBaseTypeIdentifier()->getName(); + + if (const auto *BaseDecl = Result.Nodes.getNodeAs<Decl>("base")) + diag(BaseDecl->getLocation(), + "object type inherits from base class declared here", + DiagnosticIDs::Note); } } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 8c7426c33d13b..d557006b31d25 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -267,7 +267,8 @@ Changes in existing checks - Improved :doc:`bugprone-throw-keyword-missing <clang-tidy/checks/bugprone/throw-keyword-missing>` check by only considering - the canonical types of base classes as written. + the canonical types of base classes as written and adding a note on the base + class that triggered the warning. - Improved :doc:`bugprone-unchecked-optional-access <clang-tidy/checks/bugprone/unchecked-optional-access>` check by supporting diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/throw-keyword-missing.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/throw-keyword-missing.cpp index 6ddaf246a354e..87a7beb3dfcd0 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/throw-keyword-missing.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/throw-keyword-missing.cpp @@ -20,6 +20,7 @@ typedef basic_string<char> string; typedef basic_string<wchar_t> wstring; // std::exception and std::runtime_error declaration. +// CHECK-MESSAGES-DAG: [[#EXCEPTION_LINE:@LINE + 1]]:8 struct exception { exception(); exception(const exception &other); @@ -50,12 +51,13 @@ struct RegularException { void stdExceptionNotTrownTest(int i) { if (i < 0) - // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception object created but not thrown; did you mean 'throw {{.*}}'? [bugprone-throw-keyword-missing] + // CHECK-MESSAGES-DAG: :[[@LINE+1]]:5: warning: suspicious exception object created but not thrown; did you mean 'throw {{.*}}'? [bugprone-throw-keyword-missing] std::exception(); if (i > 0) - // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception + // CHECK-MESSAGES-DAG: :[[@LINE+1]]:5: warning: suspicious exception std::runtime_error("Unexpected argument"); + // CHECK-MESSAGES: note: object type inherits from base class declared here } void stdExceptionThrownTest(int i) { @@ -132,7 +134,7 @@ class CtorInitializerListTest { CtorInitializerListTest(int) try : exc(RegularException()) { // Constructor body } catch (...) { - // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception + // CHECK-MESSAGES-DAG: :[[@LINE+1]]:5: warning: suspicious exception RegularException(); } @@ -181,6 +183,7 @@ class RegularError : public ERROR_BASE {}; void typedefTest() { // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: suspicious exception RegularError(); + // CHECK-MESSAGES: :[[#EXCEPTION_LINE]]:8: note: object type inherits from base class declared here } struct ExceptionRAII { `````````` </details> https://github.com/llvm/llvm-project/pull/160751 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
