Skip to content

Conversation

aaronpuchert
Copy link
Member

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.

@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2025

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Aaron Puchert (aaronpuchert)

Changes

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 --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 {

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.
@aaronpuchert aaronpuchert force-pushed the bugprone-throw-keyword-missing-base-note branch from 63acbbc to d501310 Compare September 25, 2025 19:48
Copy link
Collaborator

@Xazax-hun Xazax-hun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG, thanks!

@aaronpuchert aaronpuchert merged commit 4bdf545 into llvm:main Sep 26, 2025
11 checks passed
@aaronpuchert aaronpuchert deleted the bugprone-throw-keyword-missing-base-note branch September 26, 2025 17:16
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants