[clang-tidy] add checker for hicpp to ensure all exceptions are derived from std::exception

[JonasToth]

Summary

This check enforces rule 15.1.
It checks, that all thrown exceptions are of type std::exception (or derived).

[JonasToth]

Some comment slipped through. Fixed now

[Eugene.Zelenko]

docs/ReleaseNotes.rst

60

Please move it to new check list in alphabetical order.

63

This check should be omitted.

docs/clang-tidy/checks/hicpp-exception-baseclass.rst

8 ↗

(On Diff #110597)

Probably empty line should separate statements.

[JonasToth]

[Doc] fix first review comments

[JonasToth]

[Fix] list problem, some interference with other branch

[aaron.ballman]

clang-tidy/hicpp/ExceptionBaseclassCheck.cpp

35

How about: "throwing an exception whose type is not derived from 'std::exception'" and an accompanying note that points to the declaration of the exception object, if any?

36

Can just call BadThrow->getSourceRange() instead.

[JonasToth]

clang-tidy/hicpp/ExceptionBaseclassCheck.cpp

35

yes. that would be more telling.

[JonasToth]

adjust diagnostic message, emit note for defined type

[aaron.ballman]

LGTM!

[JonasToth]

Still got no commit access :(, but i asked already ;)

Could you please apply this patch?

[aaron.ballman]

In D36583#839044, @JonasToth wrote:

Still got no commit access :(, but i asked already ;)

Could you please apply this patch?

Can you rebase? It does not apply cleanly. Alternatively, you can wait for @lattner to give you your credentials and commit yourself (he's usually pretty quick at getting those out to folks).

[JonasToth]

rebased

[aaron.ballman]

I've commit in r310727.

[lebedev.ri]

Ok, i missed this somehow, and i have some concerns...
https://reviews.llvm.org/rL310727#108850

[JonasToth]

i will reopen this one, until the issues are solved.

[JonasToth]

• master remerged into this branch
• improve testcases for generic code

[JonasToth]

i did work on the generic code with macros and templates.
i could not reproduce all issues, but in general template exception cause problems with diagnostics and with catching these cases.

the inheritance tree is catched correctly, as far as i can see, maybe you could me with reproducing ur issue @lebedev.ri ?

[JonasToth]

plain macros seem to be handled correctly. Only troublemaker are templates.

[JonasToth]

added more generic cases, and improved the matcher, thanks to @lebedev.ri

[JonasToth]

the new code to fix the issues is in D37060, so i will close this revision.

clang-tidy/hicpp/ExceptionBaseclassCheck.cpp

 //===--- ExceptionBaseclassCheck.cpp - clang-tidy--------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "ExceptionBaseclassCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
-#include <iostream>
-
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace hicpp {
 
 void ExceptionBaseclassCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
     return;
 
   Finder->addMatcher(
       cxxThrowExpr(
           allOf(
+              unless(has(cxxUnresolvedConstructExpr())),
               has(expr(unless(hasType(cxxRecordDecl(
                   isSameOrDerivedFrom(hasName("std::exception"))))))),
               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");
+
   diag(BadThrow->getLocStart(),
        "throwing an exception whose type is not derived from 'std::exception'")
       << BadThrow->getSourceRange();
 
   const auto *TypeDecl = Result.Nodes.getNodeAs<NamedDecl>("decl");
   if (TypeDecl != nullptr)
     diag(TypeDecl->getLocStart(), "type defined here", DiagnosticIDs::Note);
 }
 
 } // namespace hicpp
 } // namespace tidy
 } // namespace clang

docs/ReleaseNotes.rst

 Improvements to clang-rename
 ----------------------------
 
 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
+- New `hicpp-exception-baseclass
+  <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-exception-baseclass.html>`_ check
+
+  This check ensures that all exception will be instances of ``std::exception`` and classes 
+  that are derived from it.
+
 - Renamed checks to use correct term "implicit conversion" instead of "implicit
   cast" and modified messages and option names accordingly:
 
     * **performance-implicit-cast-in-loop** was renamed to
       `performance-implicit-conversion-in-loop
       <http://clang.llvm.org/extra/clang-tidy/checks/performance-implicit-conversion-in-loop.html>`_
     * **readability-implicit-bool-cast** was renamed to
       `readability-implicit-bool-conversion

docs/clang-tidy/checks/list.rst

.. toctree::
    cert-err58-cpp
    cert-err60-cpp
    cert-err61-cpp (redirects to misc-throw-by-value-catch-by-reference) <cert-err61-cpp>
    cert-fio38-c (redirects to misc-non-copyable-objects) <cert-fio38-c>
    cert-flp30-c
    cert-msc30-c (redirects to cert-msc50-cpp) <cert-msc30-c>
    cert-msc50-cpp
    cert-oop11-cpp (redirects to misc-move-constructor-init) <cert-oop11-cpp>
-   cppcoreguidelines-c-copy-assignment-signature
+   cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) <cppcoreguidelines-c-copy-assignment-signature>
    cppcoreguidelines-interfaces-global-init
    cppcoreguidelines-no-malloc
    cppcoreguidelines-pro-bounds-array-to-pointer-decay
    cppcoreguidelines-pro-bounds-constant-array-index
    cppcoreguidelines-pro-bounds-pointer-arithmetic
    cppcoreguidelines-pro-type-const-cast
    cppcoreguidelines-pro-type-cstyle-cast
    cppcoreguidelines-pro-type-member-init

test/clang-tidy/hicpp-exception-baseclass.cpp

 // RUN: %check_clang_tidy %s hicpp-exception-baseclass %t -- -- -fcxx-exceptions
 
 namespace std {
 class exception {};
 } // namespace std
 
 class derived_exception : public std::exception {};
+class deep_hierarchy : public derived_exception {};
 class non_derived_exception {};
 
 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'
   } catch (int e) {
   }
   throw int(42); // Bad
   // CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is not derived from 'std::exception'
 
   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
+    // 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
+  // CHECK-MESSAGES: 9:1: note: type defined here
 }
 
 void allowed_throws() {
   try {
     throw std::exception(); // Ok
   } catch (std::exception &e) { // Ok
   }
   throw std::exception();
 
   try {
     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
+}
+
+// Templated function that throws exception based on template type
+template <typename T>
+void ThrowException() { throw T(); }
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: throwing an exception whose type is not derived from 'std::exception'
+  // CHECK-MESSAGES: [[@LINE-3]]:11: note: type defined here
+#define THROW_EXCEPTION(CLASS) ThrowException<CLASS>()
+#define THROW_BAD_EXCEPTION throw int(42);
+#define THROW_GOOD_EXCEPTION throw std::exception();
+#define THROW_DERIVED_EXCEPTION throw deep_hierarchy();
+
+template <typename T>
+class generic_exception : std::exception {};
+
+template <typename T>
+class bad_generic_exception {};
+
+template <typename T>
+class exotic_exception : public T {};
+
+void generic_exceptions() {
+  THROW_EXCEPTION(int);
+  THROW_EXCEPTION(std::exception);
+  THROW_EXCEPTION(derived_exception);
+  THROW_EXCEPTION(deep_hierarchy);
+  THROW_EXCEPTION(non_derived_exception);
+
+  THROW_BAD_EXCEPTION;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type is not derived from 'std::exception'
+  // CHECK-MESSAGES: [[@LINE-16]]:29: note: expanded from macro 'THROW_BAD_EXCEPTION'
+  THROW_GOOD_EXCEPTION;
+  THROW_DERIVED_EXCEPTION;
+
+  throw generic_exception<int>(); // Ok,
+  THROW_EXCEPTION(generic_exception<float>); // Ok
+
+  throw bad_generic_exception<int>(); // Bad, not derived
+  throw bad_generic_exception<std::exception>(); // Bad as well, since still not derived
+  THROW_EXCEPTION(bad_generic_exception<int>); // Bad
+  THROW_EXCEPTION(bad_generic_exception<std::exception>); // Bad
+
+  throw exotic_exception<non_derived_exception>(); // Bad
+  THROW_EXCEPTION(exotic_exception<non_derived_exception>); // Bad
+
+  throw exotic_exception<derived_exception>(); // Ok
+  THROW_EXCEPTION(exotic_exception<derived_exception>); // Ok
 }