[clang-tools-extra] 57330c8 - Don't look into base class aliases in bugprone-throw-keyword-missing (#160725)

[via cfe-commits]

Author: Aaron Puchert
Date: 2025-09-25T20:12:47+02:00
New Revision: 57330c8514c78e7032975961badf5fa59091f059

URL: https://github.com/llvm/llvm-project/commit/57330c8514c78e7032975961badf5fa59091f059
DIFF: https://github.com/llvm/llvm-project/commit/57330c8514c78e7032975961badf5fa59091f059.diff

LOG: Don't look into base class aliases in bugprone-throw-keyword-missing (#160725)

The check confusingly fires on non-exception classes if any base class
has an alias in an exception class. In our case, the exception had an
alias for an allocator interface, so every allocator inheriting from
that interface was treated as an exception type. (But only when the
header for the exception was included.)

The reason behind this is the odd (but documented) behavior of
isDerivedFrom and similar matchers: it does not only iterate through the
bases as written, but through all relevant nodes to check them for being
a base. This makes the matcher also finds aliases of the base classes.

Only going through the bases as written can be done with `hasAnyBase`.
However, that doesn't cover the class itself, and we have to check it
separately. Since we're no longer looking through aliases via the
matcher, and because we're apparently interested in the canonical type,
we check that (see the test with "typedef std::exception ERROR_BASE;").

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/bugprone/throw-keyword-missing.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
index 89eafb15f2652..cafb4a3e5f0e5 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
@@ -17,8 +17,10 @@ namespace clang::tidy::bugprone {
 void ThrowKeywordMissingCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       cxxConstructExpr(
-          hasType(cxxRecordDecl(
-              isSameOrDerivedFrom(matchesName("[Ee]xception|EXCEPTION")))),
+          hasType(cxxRecordDecl(anyOf(
+              matchesName("[Ee]xception|EXCEPTION"),
+              hasAnyBase(hasType(hasCanonicalType(recordType(hasDeclaration(
+                  cxxRecordDecl(matchesName("[Ee]xception|EXCEPTION")))))))))),
           unless(anyOf(
               hasAncestor(
                   stmt(anyOf(cxxThrowExpr(), callExpr(), returnStmt()))),

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index f5563389d3106..8c7426c33d13b 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -265,6 +265,10 @@ Changes in existing checks
   namespace are treated as the tag or the data part of a user-defined
   tagged union respectively.
 
+- 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.
+
 - Improved :doc:`bugprone-unchecked-optional-access
   <clang-tidy/checks/bugprone/unchecked-optional-access>` check by supporting
   ``NullableValue::makeValue`` and ``NullableValue::makeValueInplace`` to

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 bafd3d19b5a31..6ddaf246a354e 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
@@ -32,8 +32,9 @@ struct runtime_error : public exception {
 
 } // namespace std
 
-// The usage of this class should never emit a warning.
+// The usage of these classes should never emit a warning.
 struct RegularClass {};
+struct RegularDerived : public RegularClass {};
 
 // Class name contains the substring "exception", in certain cases using this class should emit a warning.
 struct RegularException {
@@ -41,6 +42,8 @@ struct RegularException {
 
   // Constructors with a single argument are treated 
diff erently (cxxFunctionalCastExpr).
   RegularException(int) {}
+
+  typedef RegularClass RegularAlias;
 };
 
 // --------------
@@ -68,6 +71,10 @@ void regularClassNotThrownTest(int i) {
     RegularClass();
 }
 
+void regularClassWithAliasNotThrownTest(int i) {
+  RegularDerived();
+}
+
 void regularClassThrownTest(int i) {
   if (i < 0)
     throw RegularClass();