[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

9

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

36

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?

37

Can just call BadThrow->getSourceRange() instead.

[JonasToth]

clang-tidy/hicpp/ExceptionBaseclassCheck.cpp

36

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/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyHICPPModule
+  ExceptionBaseclassCheck.cpp
   NoAssemblerCheck.cpp
   HICPPTidyModule.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyCppCoreGuidelinesModule
   clangTidyGoogleModule
   clangTidyMiscModule
   clangTidyModernizeModule
   clangTidyReadabilityModule
   clangTidyUtils
   )

clang-tidy/hicpp/ExceptionBaseclassCheck.h

+//===--- ExceptionBaseclassCheck.h - clang-tidy------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_HICPP_EXCEPTION_BASECLASS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_HICPP_EXCEPTION_BASECLASS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace hicpp {
+
+/// Check for thrown exceptions and enforce they are all derived from std::exception.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/hicpp-exception-baseclass.html
+class ExceptionBaseclassCheck : public ClangTidyCheck {
+public:
+  ExceptionBaseclassCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace hicpp
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_HICPP_EXCEPTION_BASECLASS_H

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(
+              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

clang-tidy/hicpp/HICPPTidyModule.cpp

 #include "../misc/UndelegatedConstructor.h"
 #include "../misc/UseAfterMoveCheck.h"
 #include "../modernize/UseEqualsDefaultCheck.h"
 #include "../modernize/UseEqualsDeleteCheck.h"
 #include "../modernize/UseOverrideCheck.h"
 #include "../readability/BracesAroundStatementsCheck.h"
 #include "../readability/FunctionSizeCheck.h"
 #include "../readability/IdentifierNamingCheck.h"
+#include "ExceptionBaseclassCheck.h"
 #include "NoAssemblerCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace hicpp {
 
 class HICPPModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<readability::BracesAroundStatementsCheck>(
         "hicpp-braces-around-statements");
+    CheckFactories.registerCheck<ExceptionBaseclassCheck>(
+        "hicpp-exception-baseclass");
     CheckFactories.registerCheck<google::ExplicitConstructorCheck>(
         "hicpp-explicit-conversions");
     CheckFactories.registerCheck<readability::FunctionSizeCheck>(
         "hicpp-function-size");
     CheckFactories.registerCheck<readability::IdentifierNamingCheck>(
         "hicpp-named-parameter");
     CheckFactories.registerCheck<misc::UseAfterMoveCheck>(
         "hicpp-invalid-access-moved");

docs/ReleaseNotes.rst

 Improvements to clang-rename
 ----------------------------
 
 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
 - 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
       <http://clang.llvm.org/extra/clang-tidy/checks/readability-implicit-bool-conversion.html>`_;
       the check's options were renamed as follows:
       ``AllowConditionalIntegerCasts`` -> ``AllowIntegerConditions``,
       ``AllowConditionalPointerCasts`` -> ``AllowPointerConditions``.
 
 - New `android-cloexec-memfd_create
   <http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-memfd_create.html>`_ check
 
   Checks if the required file flag ``MFD_CLOEXEC`` is present in the argument
   of ``memfd_create()``.
 
 - New `bugprone-integer-division
   <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-integer-division.html>`_ check
 
   Finds cases where integer division in a floating point context is likely to
   cause unintended loss of precision.
 
+- New `hicpp-exception-baseclass
+  <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-exception-baseclass.html>`_ check
+
+  Ensures that all exception will be instances of ``std::exception`` and classes 
+  that are derived from it.
+
 - New `readability-static-accessed-through-instance
   <http://clang.llvm.org/extra/clang-tidy/checks/readability-static-accessed-through-instance.html>`_ check
 
   Finds member expressions that access static members through instances and
   replaces them with uses of the appropriate qualified-id.
 
 - Added `modernize-use-emplace.IgnoreImplicitConstructors
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html#cmdoption-arg-IgnoreImplicitConstructors>`_

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

+.. title:: clang-tidy - hicpp-exception-baseclass
+
+hicpp-exception-baseclass
+=========================
+
+Ensure that every value that in a ``throw`` expression is an instance of 
+``std::exception``.
+
+This enforces `rule 15.1 <http://www.codingstandard.com/section/15-1-throwing-an-exception/>`_
+of the High Integrity C++ Coding Standard.
+
+.. code-block:: c++
+
+  class custom_exception {};
+
+  void throwing() noexcept(false) {
+    // Problematic throw expressions.
+    throw int(42);
+    throw custom_exception();
+  }
+
+  class mathematical_error : public std::exception {};
+
+  void throwing2() noexcept(false) {
+    // These kind of throws are ok.
+    throw mathematical_error();
+    throw std::runtime_error();
+    throw std::exception();
+  }
+  

docs/clang-tidy/checks/list.rst

.. toctree::
    google-readability-namespace-comments (redirects to llvm-namespace-comment) <google-readability-namespace-comments>
    google-readability-redundant-smartptr-get (redirects to readability-redundant-smartptr-get) <google-readability-redundant-smartptr-get>
    google-readability-todo
    google-runtime-int
    google-runtime-member-string-references
    google-runtime-operator
    google-runtime-references
    hicpp-braces-around-statements (redirects to readability-braces-around-statements) <hicpp-braces-around-statements>
+   hicpp-exception-baseclass
    hicpp-explicit-conversions (redirects to google-explicit-constructor) <hicpp-explicit-conversions>
    hicpp-function-size (redirects to readability-function-size) <hicpp-function-size>
    hicpp-invalid-access-moved (redirects to misc-use-after-move) <hicpp-invalid-access-moved>
    hicpp-member-init (redirects to cppcoreguidelines-pro-type-member-init) <hicpp-member-init>
    hicpp-named-parameter (redirects to readability-named-parameter) <hicpp-named-parameter>
    hicpp-new-delete-operators (redirects to misc-new-delete-overloads) <hicpp-new-delete-operators>
    hicpp-no-assembler
    hicpp-noexcept-move (redirects to misc-noexcept-moveconstructor) <hicpp-noexcept-move>

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

+// RUN: %check_clang_tidy %s hicpp-exception-baseclass %t
+
+namespace std {
+class exception {};
+} // namespace std
+
+class derived_exception : public std::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
+  } 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
+}
+
+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
+}