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

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.

Reviewers: lebedev.ri, aaron.ballman, baloghadamsoftware, alexfh

Reviewed By: lebedev.ri, baloghadamsoftware

Subscribers: xazax.hun, rnkovacs, a.sidorin, Szelethus, donat.nagy, dkrupp, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D57883

llvm-svn: 354517

clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp

@@ -42,6 +42,7 @@ ExceptionEscapeCheck::ExceptionEscapeCheck(StringRef Name,
   IgnoredExceptions.insert(IgnoredExceptionsVec.begin(),
                            IgnoredExceptionsVec.end());
   Tracer.ignoreExceptions(std::move(IgnoredExceptions));
+  Tracer.ignoreBadAlloc(true);
 }
 
 void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
@@ -70,7 +71,8 @@ void ExceptionEscapeCheck::check(const MatchFinder::MatchResult &Result) {
   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(),

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

@@ -8,10 +8,44 @@
 
 #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();
@@ -22,36 +56,87 @@ static bool isBaseOf(const Type *DerivedType, const Type *BaseType) {
       [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);
+  }
 
-ExceptionAnalyzer::TypeVec ExceptionAnalyzer::throwsException(
+  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::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;
 
@@ -59,28 +144,28 @@ ExceptionAnalyzer::TypeVec ExceptionAnalyzer::throwsException(
     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 =
@@ -90,64 +175,62 @@ ExceptionAnalyzer::TypeVec ExceptionAnalyzer::throwsException(
                            ->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;
 
-  llvm::SmallSet<const FunctionDecl *, 32> CallStack;
-  TypeVec ExceptionList = throwsException(Func, CallStack);
+  // Check if the function has already been analyzed and reuse that result.
+  if (FunctionCache.count(Func) == 0) {
+    llvm::SmallSet<const FunctionDecl *, 32> 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());
+  ExceptionList.filterIgnoredExceptions(IgnoredExceptions, IgnoreBadAlloc);
 
-  // Cache the result of the analysis.
-  bool FunctionThrows = ExceptionList.size() > 0;
-  FunctionCache.insert(std::make_pair(Func, FunctionThrows));
-
-  return FunctionThrows;
+  return ExceptionList;
 }
-
-bool ExceptionAnalyzer::isIgnoredExceptionType(const Type *Exception) {
-  if (const auto *TD = Exception->getAsTagDecl()) {
-    if (TD->getDeclName().isIdentifier())
-      return IgnoredExceptions.count(TD->getName()) > 0;
-  }
-  return false;
-}
-
 } // namespace utils
 } // namespace tidy
 

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h

@@ -10,6 +10,7 @@
 #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"
 
@@ -17,29 +18,131 @@ 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);
   }
 
+  ExceptionInfo analyze(const FunctionDecl *Func);
+
 private:
-  using TypeVec = llvm::SmallVector<const Type *, 8>;
-
-  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);
+  ExceptionInfo
+  throwsException(const FunctionDecl *Func,
+                  llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
+  ExceptionInfo
+  throwsException(const Stmt *St, const ExceptionInfo::Throwables &Caught,
+                  llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
 
+  bool IgnoreBadAlloc = true;
   llvm::StringSet<> IgnoredExceptions;
-  llvm::DenseMap<const FunctionDecl *, bool> FunctionCache;
+  std::map<const FunctionDecl *, ExceptionInfo> FunctionCache;
 };
 } // namespace utils
 } // namespace tidy