[clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

[JonasToth]

Summary

The analsis on the throwing behvaiour on functions and statements gave only
a binary answer whether an exception could occur and if yes which types are
thrown.
This refactoring allows keeping track if there is a unknown factor, because the
code calls to some functions with unavailable source code with no noexcept
information.
This 'potential Unknown' information is propagated properly and can be queried
separately.

[lebedev.ri]

Exciting!

clang-tidy/utils/ExceptionAnalyzer.h

21–24

Would it be better to add them into ExceptionAnalyzer class?

145

Why can't llvm::DenseMap continue to be used?

[JonasToth]

clang-tidy/utils/ExceptionAnalyzer.h

21–24

ThrownExceptions? I think it fits here. Removing would shrink ExceptionInfo (and ContainsUnknown and State could be packed into one byte) which is a plus.

145

I would need to add traits for ExceptionInfo to work with DenseMap. So i went this way :)
From the docs DenseMap shall map small types (like ptr-ptr) but the ExceptionInfo has the set of types as SmallSet<Type*, 8>, so not sure if that is actually a good fit.

[lebedev.ri]

clang-tidy/utils/ExceptionAnalyzer.h

21–24

No, i meant all these classes/enums.

class ExceptionAnalyzer {
public:
enum class ExceptionState : std::int8_t {
....
}


class ExceptionInfo {
....
}

} // class ExceptionAnalyzer

[JonasToth]

• [Refactor] move support classes into the analyzer
• [Refactor] move bigger methods into implementation file

[JonasToth]

Good idea :)

[JonasToth]

ping :)

[baloghadamsoftware]

If I understand it correctly, this is more of an infrastructure improvement than check enhancement. It looks like a nice and clean code. Where do we expect to use this new behavior? In the current check or in the upcoming "modernize" check?

clang-tidy/utils/ExceptionAnalyzer.h

128

Why 4 bits, not just 2? Can we expect additional states?

[lebedev.ri]

Sorry for the reviews, i'm really stalling it seems..

This looks like a straight-forward change, but i'm not sure i'm familiar enough with ExceptionAnalyzer to fully properly review this..
As far as i can tell, this is ok. Perhaps @baloghadamsoftware will have other remarks.

In D57883#1402023, @baloghadamsoftware wrote:

If I understand it correctly, this is more of an infrastructure improvement than check enhancement. It looks like a nice and clean code. Where do we expect to use this new behavior? In the current check or in the upcoming "modernize" check?

It's for D57108, i'we guessed that such ternary answer will be required there.

clang-tidy/utils/ExceptionAnalyzer.h

26–31

Since this is later used in a bit field, it might be better to be explicit

enum class State : std::int8_t {
  Throwing = 0,    ///< The function can definitly throw given an AST.
  NotThrowing = 1, ///< This function can not throw, given an AST.
  Unknown = 2,     ///< This can happen for extern functions without available
               ///< definition.
};

and indeed, only 2 bits needed.

145

Good point.

[baloghadamsoftware]

In D57883#1402049, @lebedev.ri wrote:

It's for D57108, i'we guessed that such ternary answer will be required there.

Oh, I was not aware of that patch.

Anyway, it looks good to me.

[JonasToth]

• [Refactor] move support classes into the analyzer
• [Refactor] move bigger methods into implementation file
• minor adjustments

[JonasToth]

all comments resolved. I will land this now.

clang-tidy/utils/ExceptionAnalyzer.h

26–31

Done.

128

True, math not so strong in me :D

[JonasToth]

• be explicit about the State enumerator values

[bjope]

clang-tidy/utils/ExceptionAnalyzer.h

112

(post-commit comments)

I've seen that @dyung did some VS2015 fixes related to this in https://reviews.llvm.org/rCTE354545.

But I think there also is some history of problems with typed enums used in bitfields with GCC (I'm not sure about the details, but it might be discussed here https://reviews.llvm.org/D24289, and there have been workarounds applied before, for example here https://reviews.llvm.org/rCTE319608).

I do not know if we actually have a goal to workaround such problems (no warnings when using GCC), nor do I know if GCC produce correct code despite all the warnings we see with GCC (7.4.0) after this patch, such as

../tools/clang/tools/extra/clang-tidy/bugprone/../utils/ExceptionAnalyzer.h:112:23: warning: 'clang::tidy::utils::ExceptionAnalyzer::ExceptionInfo::Behaviour' is too small to hold all values of 'enum class clang::tidy::utils::ExceptionAnalyzer::State'
      State Behaviour : 2;
                        ^
../tools/clang/tools/extra/clang-tidy/bugprone/../utils/ExceptionAnalyzer.h:112:23: warning: 'clang::tidy::utils::ExceptionAnalyzer::ExceptionInfo::Behaviour' is too small to hold all values of 'enum class clang::tidy::utils::ExceptionAnalyzer::State'
     State Behaviour : 2;
                       ^
In file included from ../tools/clang/tools/extra/clang-tidy/utils/ExceptionAnalyzer.cpp:9:0:
../tools/clang/tools/extra/clang-tidy/utils/ExceptionAnalyzer.h:112:23: warning: 'clang::tidy::utils::ExceptionAnalyzer::ExceptionInfo::Behaviour' is too small to hold all values of 'enum class clang::tidy::utils::ExceptionAnalyzer::State'
     State Behaviour : 2;
                       ^
../tools/clang/tools/extra/clang-tidy/utils/ExceptionAnalyzer.cpp: In member function 'clang::tidy::utils::ExceptionAnalyzer::ExceptionInfo& clang::tidy::utils::ExceptionAnalyzer::ExceptionInfo::merge(const clang::tidy::utils::ExceptionAnalyzer::ExceptionInfo&)':
../tools/clang/tools/extra/clang-tidy/utils/ExceptionAnalyzer.cpp:40:28: warning: comparison is always false due to limited range of data type [-Wtype-limits]
   else if (Other.Behaviour == State::Unknown && Behaviour == State::NotThrowing)
            ~~~~~~~~~~~~~~~~^~~~~~~~
../tools/clang/tools/extra/clang-tidy/utils/ExceptionAnalyzer.cpp:41:24: warning: overflow in implicit constant conversion [-Woverflow]
     Behaviour = State::Unknown;
                        ^~~~~~~
../tools/clang/tools/extra/clang-tidy/utils/ExceptionAnalyzer.cpp: In member function 'void clang::tidy::utils::ExceptionAnalyzer::ExceptionInfo::reevaluateBehaviour()':
../tools/clang/tools/extra/clang-tidy/utils/ExceptionAnalyzer.cpp:105:26: warning: overflow in implicit constant conversion [-Woverflow]
       Behaviour = State::Unknown;
                          ^~~~~~~

To sum up:
The warning are a little bit annoying (but maybe something we have to live with if this is known bugs in gcc).
It seems like we have done other workarounds in the past (writing ugly code to satisfy MSVC and GCC). So should we do that here as well?
Might wanna check if GCC produce correct code for this.

[bjope]

Maybe @sammccall remembers why it was decided to rewrite the enum into constexpr:s in https://reviews.llvm.org/rCTE319608 ? Do we need a similar solution here?

[JonasToth]

clang-tidy/utils/ExceptionAnalyzer.h

112

I see. All i wanted to achieve was to make the object <= 64 bytes to fit in a cacheline. IMHO the bitfields can disapear. I dont want to sacrifice the enum class as I feel that gives more value then the (unmeasured) potential performance gain from shrinking the object sizes.

I will commit to remove the bitfields. Details can be figured out later.

[bjope]

clang-tidy/utils/ExceptionAnalyzer.h

112

( regarding https://reviews.llvm.org/rGc1e8cbd5c3f0ed33ab4fb8df98645ebea0018fe8 )

Pity if we have to sacrifice performance if these are false warnings from gcc. But I don't really have the details about that, so thanks for the fix!

clang-tidy/bugprone/ExceptionEscapeCheck.cpp

ExceptionEscapeCheck::ExceptionEscapeCheck(StringRef Name,
   FunctionsThatShouldNotThrow.insert(FunctionsThatShouldNotThrowVec.begin(),
                                      FunctionsThatShouldNotThrowVec.end());
 
   llvm::StringSet<> IgnoredExceptions;
   StringRef(RawIgnoredExceptions).split(IgnoredExceptionsVec, ",", -1, false);
   IgnoredExceptions.insert(IgnoredExceptionsVec.begin(),
                            IgnoredExceptionsVec.end());
   Tracer.ignoreExceptions(std::move(IgnoredExceptions));
+  Tracer.ignoreBadAlloc(true);
 }
 
 void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "FunctionsThatShouldNotThrow",
                 RawFunctionsThatShouldNotThrow);
   Options.store(Opts, "IgnoredExceptions", RawIgnoredExceptions);
 }
 

 }
 
 void ExceptionEscapeCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("thrower");
 
   if (!MatchedDecl)
     return;
 
-  if (Tracer.throwsException(MatchedDecl))
+  if (Tracer.analyze(MatchedDecl).getBehaviour() ==
+      utils::ExceptionAnalyzer::State::Throwing)
     // FIXME: We should provide more information about the exact location where
     // the exception is thrown, maybe the full path the exception escapes
     diag(MatchedDecl->getLocation(),
          "an exception may be thrown in function %0 "
 
          "which should not throw exceptions")
         << MatchedDecl;
 }
 
 } // namespace bugprone
 } // namespace tidy
 } // namespace clang

clang-tidy/utils/ExceptionAnalyzer.h

 //===--- ExceptionAnalyzer.h - clang-tidy -----------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_ANALYZER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_ANALYZER_H
 
 #include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringSet.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 
-/// This class analysis if a `FunctionDecl` can in principle throw an exception,
-/// either directly or indirectly.
-/// It can be configured to ignore custom exception types.
+/// This class analysis if a `FunctionDecl` can in principle throw an
+/// exception, either directly or indirectly. It can be configured to ignore
+/// custom exception types.
 class ExceptionAnalyzer {
 public:
+  enum class State : std::int8_t {
+    Throwing = 0,    ///< The function can definitly throw given an AST.
+    NotThrowing = 1, ///< This function can not throw, given an AST.
+    Unknown = 2,     ///< This can happen for extern functions without available
+                     ///< definition.
+  };
+
+  /// Bundle the gathered information about an entity like a function regarding
+  /// it's exception behaviour. The 'NonThrowing'-state can be considered as the
+  /// neutral element in terms of information propagation.
+  /// In the case of 'Throwing' state it is possible that 'getExceptionTypes'
+  /// does not include *ALL* possible types as there is the possibility that
+  /// an 'Unknown' function is called that might throw a previously unknown
+  /// exception at runtime.
+  class ExceptionInfo {
+  public:
+    using Throwables = llvm::SmallSet<const Type *, 2>;
+    static ExceptionInfo createUnknown() {
+      return ExceptionInfo(State::Unknown);
+    }
+    static ExceptionInfo createNonThrowing() {
+      return ExceptionInfo(State::Throwing);
+    }
+
+    /// By default the exception situation is unknown and must be
+    /// clarified step-wise.
+    ExceptionInfo() : Behaviour(State::NotThrowing), ContainsUnknown(false) {}
+    ExceptionInfo(State S)
+        : Behaviour(S), ContainsUnknown(S == State::Unknown) {}
+
+    ExceptionInfo(const ExceptionInfo &) = default;
+    ExceptionInfo &operator=(const ExceptionInfo &) = default;
+    ExceptionInfo(ExceptionInfo &&) = default;
+    ExceptionInfo &operator=(ExceptionInfo &&) = default;
+
+    State getBehaviour() const { return Behaviour; }
+
+    /// Register a single exception type as recognized potential exception to be
+    /// thrown.
+    void registerException(const Type *ExceptionType);
+
+    /// Registers a `SmallVector` of exception types as recognized potential
+    /// exceptions to be thrown.
+    void registerExceptions(const Throwables &Exceptions);
+
+    /// Updates the local state according to the other state. That means if
+    /// for example a function contains multiple statements the 'ExceptionInfo'
+    /// for the final function is the merged result of each statement.
+    /// If one of these statements throws the whole function throws and if one
+    /// part is unknown and the rest is non-throwing the result will be
+    /// unknown.
+    ExceptionInfo &merge(const ExceptionInfo &Other);
+
+    /// This method is useful in case 'catch' clauses are analyzed as it is
+    /// possible to catch multiple exception types by one 'catch' if they
+    /// are a subclass of the 'catch'ed exception type.
+    /// Returns 'true' if some exceptions were filtered, otherwise 'false'.
+    bool filterByCatch(const Type *BaseClass);
+
+    /// Filter the set of thrown exception type against a set of ignored
+    /// types that shall not be considered in the exception analysis.
+    /// This includes explicit `std::bad_alloc` ignoring as separate option.
+    ExceptionInfo &
+    filterIgnoredExceptions(const llvm::StringSet<> &IgnoredTypes,
+                            bool IgnoreBadAlloc);
+
+    /// Clear the state to 'NonThrowing' to make the corresponding entity
+    /// neutral.
+    void clear();
+
+    /// References the set of known exception types that can escape from the
+    /// corresponding entity.
+    const Throwables &getExceptionTypes() const { return ThrownExceptions; }
+
+    /// Signal if the there is any 'Unknown' element within the scope of
+    /// the related entity. This might be relevant if the entity is 'Throwing'
+    /// and to ensure that no other exception then 'getExceptionTypes' can
+    /// occur. If there is an 'Unknown' element this can not be guaranteed.
+    bool containsUnknownElements() const { return ContainsUnknown; }
+
+  private:
+    /// Recalculate the 'Behaviour' for example after filtering.
+    void reevaluateBehaviour();
+
+    /// Keep track if the entity related to this 'ExceptionInfo' can in princple
+    /// throw, if it's unknown or if it won't throw.
+    enum State Behaviour : 2;
+
+    /// Keep track if the entity contains any unknown elements to keep track
+    /// of the certainty of decisions and/or correct 'Behaviour' transition
+    /// after filtering.
+    bool ContainsUnknown : 1;
+
+    /// 'ThrownException' is empty if the 'Behaviour' is either 'NotThrowing' or
+    /// 'Unknown'.
+    Throwables ThrownExceptions;
+  };
+  static_assert(sizeof(ExceptionInfo) <= 64u,
+                "size of exceptioninfo shall be at max one cache line");
+
   ExceptionAnalyzer() = default;
 
-  bool throwsException(const FunctionDecl *Func);
+  void ignoreBadAlloc(bool ShallIgnore) { IgnoreBadAlloc = ShallIgnore; }
   void ignoreExceptions(llvm::StringSet<> ExceptionNames) {
     IgnoredExceptions = std::move(ExceptionNames);
   }
 
-private:
-  using TypeVec = llvm::SmallVector<const Type *, 8>;
+  ExceptionInfo analyze(const FunctionDecl *Func);
 
-  TypeVec throwsException(const FunctionDecl *Func,
-                          llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
-  TypeVec throwsException(const Stmt *St, const TypeVec &Caught,
-                          llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
-  bool isIgnoredExceptionType(const Type *Exception);
+private:
+  ExceptionInfo
+  throwsException(const FunctionDecl *Func,
+  ExceptionInfo
+  throwsException(const Stmt *St, const ExceptionInfo::Throwables &Caught,
 
+  bool IgnoreBadAlloc = true;
   llvm::StringSet<> IgnoredExceptions;
-  llvm::DenseMap<const FunctionDecl *, bool> FunctionCache;
+  std::map<const FunctionDecl *, ExceptionInfo> FunctionCache;
 };
 } // namespace utils
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_ANALYZER_H

clang-tidy/utils/ExceptionAnalyzer.cpp

 //===--- ExceptionAnalyzer.cpp - clang-tidy -------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "ExceptionAnalyzer.h"
 
-#include "clang/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-
 namespace clang {
+namespace tidy {
+namespace utils {
+
+void ExceptionAnalyzer::ExceptionInfo::registerException(
+    const Type *ExceptionType) {
+  assert(ExceptionType != nullptr && "Only valid types are accepted");
+  Behaviour = State::Throwing;
+  ThrownExceptions.insert(ExceptionType);
+}
+
+void ExceptionAnalyzer::ExceptionInfo::registerExceptions(
+    const Throwables &Exceptions) {
+  if (Exceptions.size() == 0)
+    return;
+  Behaviour = State::Throwing;
+  ThrownExceptions.insert(Exceptions.begin(), Exceptions.end());
+}
+
+ExceptionAnalyzer::ExceptionInfo &ExceptionAnalyzer::ExceptionInfo::merge(
+    const ExceptionAnalyzer::ExceptionInfo &Other) {
+  // Only the following two cases require an update to the local
+  // 'Behaviour'. If the local entity is already throwing there will be no
+  // change and if the other entity is throwing the merged entity will throw
+  // as well.
+  // If one of both entities is 'Unknown' and the other one does not throw
+  // the merged entity is 'Unknown' as well.
+  if (Other.Behaviour == State::Throwing)
+    Behaviour = State::Throwing;
+  else if (Other.Behaviour == State::Unknown && Behaviour == State::NotThrowing)
+    Behaviour = State::Unknown;
+
+  ContainsUnknown = ContainsUnknown || Other.ContainsUnknown;
+  ThrownExceptions.insert(Other.ThrownExceptions.begin(),
+                          Other.ThrownExceptions.end());
+  return *this;
+}
+
 static bool isBaseOf(const Type *DerivedType, const Type *BaseType) {
   const auto *DerivedClass = DerivedType->getAsCXXRecordDecl();
   const auto *BaseClass = BaseType->getAsCXXRecordDecl();
   if (!DerivedClass || !BaseClass)
     return false;
 
   return !DerivedClass->forallBases(
       [BaseClass](const CXXRecordDecl *Cur) { return Cur != BaseClass; });
 }
 
-namespace tidy {
-namespace utils {
+bool ExceptionAnalyzer::ExceptionInfo::filterByCatch(const Type *BaseClass) {
+  llvm::SmallVector<const Type *, 8> TypesToDelete;
+  for (const Type *T : ThrownExceptions) {
+    if (T == BaseClass || isBaseOf(T, BaseClass))
+      TypesToDelete.push_back(T);
+  }
+
+  for (const Type *T : TypesToDelete)
+    ThrownExceptions.erase(T);
+
+  reevaluateBehaviour();
+  return TypesToDelete.size() > 0;
+}
+
+ExceptionAnalyzer::ExceptionInfo &
+ExceptionAnalyzer::ExceptionInfo::filterIgnoredExceptions(
+    const llvm::StringSet<> &IgnoredTypes, bool IgnoreBadAlloc) {
+  llvm::SmallVector<const Type *, 8> TypesToDelete;
+  // Note: Using a 'SmallSet' with 'llvm::remove_if()' is not possible.
+  // Therefore this slightly hacky implementation is required.
+  for (const Type *T : ThrownExceptions) {
+    if (const auto *TD = T->getAsTagDecl()) {
+      if (TD->getDeclName().isIdentifier()) {
+        if ((IgnoreBadAlloc &&
+             (TD->getName() == "bad_alloc" && TD->isInStdNamespace())) ||
+            (IgnoredTypes.count(TD->getName()) > 0))
+          TypesToDelete.push_back(T);
+      }
+    }
+  }
+  for (const Type *T : TypesToDelete)
+    ThrownExceptions.erase(T);
+
+  reevaluateBehaviour();
+  return *this;
+}
+
+void ExceptionAnalyzer::ExceptionInfo::clear() {
+  Behaviour = State::NotThrowing;
+  ContainsUnknown = false;
+  ThrownExceptions.clear();
+}
+
+void ExceptionAnalyzer::ExceptionInfo::reevaluateBehaviour() {
+  if (ThrownExceptions.size() == 0)
+    if (ContainsUnknown)
+      Behaviour = State::Unknown;
+    else
+      Behaviour = State::NotThrowing;
+  else
+    Behaviour = State::Throwing;
+}
 
-ExceptionAnalyzer::TypeVec ExceptionAnalyzer::throwsException(
+ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException(
     const FunctionDecl *Func,
     llvm::SmallSet<const FunctionDecl *, 32> &CallStack) {
   if (CallStack.count(Func))
-    return TypeVec();
+    return ExceptionInfo::createNonThrowing();
 
   if (const Stmt *Body = Func->getBody()) {
     CallStack.insert(Func);
-    const TypeVec Result = throwsException(Body, TypeVec(), CallStack);
+    ExceptionInfo Result =
+        throwsException(Body, ExceptionInfo::Throwables(), CallStack);
     CallStack.erase(Func);
     return Result;
   }
 
-  TypeVec Result;
+  auto Result = ExceptionInfo::createUnknown();
   if (const auto *FPT = Func->getType()->getAs<FunctionProtoType>()) {
-    for (const QualType Ex : FPT->exceptions()) {
-      Result.push_back(Ex.getTypePtr());
-    }
+    for (const QualType Ex : FPT->exceptions())
+      Result.registerException(Ex.getTypePtr());
   }
   return Result;
 }
 
-ExceptionAnalyzer::TypeVec ExceptionAnalyzer::throwsException(
-    const Stmt *St, const TypeVec &Caught,
+/// Analyzes a single statment on it's throwing behaviour. This is in principle
+/// possible except some 'Unknown' functions are called.
+ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException(
+    const Stmt *St, const ExceptionInfo::Throwables &Caught,
     llvm::SmallSet<const FunctionDecl *, 32> &CallStack) {
-  TypeVec Results;
-
+  auto Results = ExceptionInfo::createNonThrowing();
   if (!St)
     return Results;
 
   if (const auto *Throw = dyn_cast<CXXThrowExpr>(St)) {
     if (const auto *ThrownExpr = Throw->getSubExpr()) {
       const auto *ThrownType =
           ThrownExpr->getType()->getUnqualifiedDesugaredType();
-      if (ThrownType->isReferenceType()) {
+      if (ThrownType->isReferenceType())
         ThrownType = ThrownType->castAs<ReferenceType>()
                          ->getPointeeType()
                          ->getUnqualifiedDesugaredType();
-      }
-      if (const auto *TD = ThrownType->getAsTagDecl()) {
-        if (TD->getDeclName().isIdentifier() && TD->getName() == "bad_alloc" &&
-            TD->isInStdNamespace())
-          return Results;
-      }
-      Results.push_back(ThrownExpr->getType()->getUnqualifiedDesugaredType());
-    } else {
-      Results.append(Caught.begin(), Caught.end());
-    }
+      Results.registerException(
+          ThrownExpr->getType()->getUnqualifiedDesugaredType());
+    } else
+      // A rethrow of a caught exception happens which makes it possible
+      // to throw all exception that are caught in the 'catch' clause of
+      // the parent try-catch block.
+      Results.registerExceptions(Caught);
   } else if (const auto *Try = dyn_cast<CXXTryStmt>(St)) {
-    TypeVec Uncaught = throwsException(Try->getTryBlock(), Caught, CallStack);
+    ExceptionInfo Uncaught =
+        throwsException(Try->getTryBlock(), Caught, CallStack);
     for (unsigned i = 0; i < Try->getNumHandlers(); ++i) {
       const CXXCatchStmt *Catch = Try->getHandler(i);
+
+      // Everything is catched through 'catch(...)'.
       if (!Catch->getExceptionDecl()) {
-        const TypeVec Rethrown =
-            throwsException(Catch->getHandlerBlock(), Uncaught, CallStack);
-        Results.append(Rethrown.begin(), Rethrown.end());
+        ExceptionInfo Rethrown = throwsException(
+            Catch->getHandlerBlock(), Uncaught.getExceptionTypes(), CallStack);
+        Results.merge(Rethrown);
         Uncaught.clear();
       } else {
         const auto *CaughtType =
             Catch->getCaughtType()->getUnqualifiedDesugaredType();
         if (CaughtType->isReferenceType()) {
           CaughtType = CaughtType->castAs<ReferenceType>()
                            ->getPointeeType()
                            ->getUnqualifiedDesugaredType();
         }
-        auto NewEnd =
-            llvm::remove_if(Uncaught, [&CaughtType](const Type *ThrownType) {
-              return ThrownType == CaughtType ||
-                     isBaseOf(ThrownType, CaughtType);
-            });
-        if (NewEnd != Uncaught.end()) {
-          Uncaught.erase(NewEnd, Uncaught.end());
-          const TypeVec Rethrown = throwsException(
-              Catch->getHandlerBlock(), TypeVec(1, CaughtType), CallStack);
-          Results.append(Rethrown.begin(), Rethrown.end());
+
+        // If the caught exception will catch multiple previously potential
+        // thrown types (because it's sensitive to inheritance) the throwing
+        // situation changes. First of all filter the exception types and
+        // analyze if the baseclass-exception is rethrown.
+        if (Uncaught.filterByCatch(CaughtType)) {
+          ExceptionInfo::Throwables CaughtExceptions;
+          CaughtExceptions.insert(CaughtType);
+          ExceptionInfo Rethrown = throwsException(Catch->getHandlerBlock(),
+                                                   CaughtExceptions, CallStack);
+          Results.merge(Rethrown);
         }
       }
     }
-    Results.append(Uncaught.begin(), Uncaught.end());
+    Results.merge(Uncaught);
   } else if (const auto *Call = dyn_cast<CallExpr>(St)) {
     if (const FunctionDecl *Func = Call->getDirectCallee()) {
-      TypeVec Excs = throwsException(Func, CallStack);
-      Results.append(Excs.begin(), Excs.end());
+      ExceptionInfo Excs = throwsException(Func, CallStack);
+      Results.merge(Excs);
     }
   } else {
     for (const Stmt *Child : St->children()) {
-      TypeVec Excs = throwsException(Child, Caught, CallStack);
-      Results.append(Excs.begin(), Excs.end());
+      ExceptionInfo Excs = throwsException(Child, Caught, CallStack);
+      Results.merge(Excs);
     }
   }
   return Results;
 }
 
-bool ExceptionAnalyzer::throwsException(const FunctionDecl *Func) {
-  // Check if the function has already been analyzed and reuse that result.
-  if (FunctionCache.count(Func) > 0)
-    return FunctionCache[Func];
+ExceptionAnalyzer::ExceptionInfo
+ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
+  ExceptionInfo ExceptionList;
 
+  // Check if the function has already been analyzed and reuse that result.
+  if (FunctionCache.count(Func) == 0) {
     llvm::SmallSet<const FunctionDecl *, 32> CallStack;
-  TypeVec ExceptionList = throwsException(Func, CallStack);
+    ExceptionList = throwsException(Func, CallStack);
+
+    // Cache the result of the analysis. This is done prior to filtering
+    // because it is best to keep as much information as possible.
+    // The results here might be relevant to different analysis passes
+    // with different needs as well.
+    FunctionCache.insert(std::make_pair(Func, ExceptionList));
+  } else
+    ExceptionList = FunctionCache[Func];
+
+  if (ExceptionList.getBehaviour() == State::NotThrowing ||
+      ExceptionList.getBehaviour() == State::Unknown)
+    return ExceptionList;
 
   // Remove all ignored exceptions from the list of exceptions that can be
   // thrown.
-  auto NewEnd = llvm::remove_if(ExceptionList, [this](const Type *Exception) {
-    return isIgnoredExceptionType(Exception);
-  });
-  ExceptionList.erase(NewEnd, ExceptionList.end());
-
-  // Cache the result of the analysis.
-  bool FunctionThrows = ExceptionList.size() > 0;
-  FunctionCache.insert(std::make_pair(Func, FunctionThrows));
-
-  return FunctionThrows;
-}
-
-bool ExceptionAnalyzer::isIgnoredExceptionType(const Type *Exception) {
-  if (const auto *TD = Exception->getAsTagDecl()) {
-    if (TD->getDeclName().isIdentifier())
-      return IgnoredExceptions.count(TD->getName()) > 0;
-  }
-  return false;
-}
+  ExceptionList.filterIgnoredExceptions(IgnoredExceptions, IgnoreBadAlloc);
 
+  return ExceptionList;
+}
 } // namespace utils
 } // namespace tidy
 
 } // namespace clang