[clang-tidy] refactor bugprone-exception-escape analysis into class
authorJonas Toth <jonas.toth@gmail.com>
Thu, 31 Jan 2019 10:46:37 +0000 (10:46 +0000)
committerJonas Toth <jonas.toth@gmail.com>
Thu, 31 Jan 2019 10:46:37 +0000 (10:46 +0000)
Summary:
The check `bugprone-exception-escape` does an AST-based analysis to determine
if a function might throw an exception and warns based on that information.
The analysis part is refactored into a standalone class similiar to
`ExprMutAnalyzer` that is generally useful.
I intent to use that class in a new check to automatically introduce `noexcept`
if possible.

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

Reviewed By: baloghadamsoftware, lebedev.ri

Subscribers: lebedev.ri, mgorny, xazax.hun, rnkovacs, cfe-commits

Tags: #clang-tools-extra

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

llvm-svn: 352741

clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.h
clang-tools-extra/clang-tidy/utils/CMakeLists.txt
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp [new file with mode: 0644]
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h [new file with mode: 0644]

index bd57da9..0849e2e 100644 (file)
@@ -1,4 +1,4 @@
-//===--- ExceptionEscapeCheck.cpp - clang-tidy-----------------------------===//
+//===--- ExceptionEscapeCheck.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.
 
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
-
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringSet.h"
 
 using namespace clang::ast_matchers;
 
-namespace {
-typedef llvm::SmallVector<const clang::Type *, 8> TypeVec;
-} // namespace
-
 namespace clang {
-
-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; });
-}
-
-static const TypeVec
-throwsException(const Stmt *St, const TypeVec &Caught,
-                llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
-
-static const TypeVec
-throwsException(const FunctionDecl *Func,
-                llvm::SmallSet<const FunctionDecl *, 32> &CallStack) {
-  if (CallStack.count(Func))
-    return TypeVec();
-
-  if (const Stmt *Body = Func->getBody()) {
-    CallStack.insert(Func);
-    const TypeVec Result = throwsException(Body, TypeVec(), CallStack);
-    CallStack.erase(Func);
-    return Result;
-  }
-
-  TypeVec Result;
-  if (const auto *FPT = Func->getType()->getAs<FunctionProtoType>()) {
-    for (const QualType Ex : FPT->exceptions()) {
-      Result.push_back(Ex.getTypePtr());
-    }
-  }
-  return Result;
-}
-
-static const TypeVec
-throwsException(const Stmt *St, const TypeVec &Caught,
-                llvm::SmallSet<const FunctionDecl *, 32> &CallStack) {
-  TypeVec Results;
-
-  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()) {
-        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());
-    }
-  } else if (const auto *Try = dyn_cast<CXXTryStmt>(St)) {
-    TypeVec Uncaught = throwsException(Try->getTryBlock(), Caught, CallStack);
-    for (unsigned i = 0; i < Try->getNumHandlers(); ++i) {
-      const CXXCatchStmt *Catch = Try->getHandler(i);
-      if (!Catch->getExceptionDecl()) {
-        const TypeVec Rethrown =
-            throwsException(Catch->getHandlerBlock(), Uncaught, CallStack);
-        Results.append(Rethrown.begin(), Rethrown.end());
-        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());
-        }
-      }
-    }
-    Results.append(Uncaught.begin(), Uncaught.end());
-  } 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());
-    }
-  } else {
-    for (const Stmt *Child : St->children()) {
-      TypeVec Excs = throwsException(Child, Caught, CallStack);
-      Results.append(Excs.begin(), Excs.end());
-    }
-  }
-  return Results;
-}
-
-static const TypeVec throwsException(const FunctionDecl *Func) {
-  llvm::SmallSet<const FunctionDecl *, 32> CallStack;
-  return throwsException(Func, CallStack);
-}
-
-namespace ast_matchers {
-AST_MATCHER_P(FunctionDecl, throws, internal::Matcher<Type>, InnerMatcher) {
-  TypeVec ExceptionList = throwsException(&Node);
-  auto NewEnd = llvm::remove_if(
-      ExceptionList, [this, Finder, Builder](const Type *Exception) {
-        return !InnerMatcher.matches(*Exception, Finder, Builder);
-      });
-  ExceptionList.erase(NewEnd, ExceptionList.end());
-  return ExceptionList.size();
-}
-
-AST_MATCHER_P(Type, isIgnored, llvm::StringSet<>, IgnoredExceptions) {
-  if (const auto *TD = Node.getAsTagDecl()) {
-    if (TD->getDeclName().isIdentifier())
-      return IgnoredExceptions.count(TD->getName()) > 0;
-  }
-  return false;
-}
-
+namespace {
 AST_MATCHER_P(FunctionDecl, isEnabled, llvm::StringSet<>,
               FunctionsThatShouldNotThrow) {
   return FunctionsThatShouldNotThrow.count(Node.getNameAsString()) > 0;
 }
-} // namespace ast_matchers
+} // namespace
 
 namespace tidy {
 namespace bugprone {
-
 ExceptionEscapeCheck::ExceptionEscapeCheck(StringRef Name,
                                            ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context), RawFunctionsThatShouldNotThrow(Options.get(
@@ -173,9 +36,12 @@ ExceptionEscapeCheck::ExceptionEscapeCheck(StringRef Name,
       .split(FunctionsThatShouldNotThrowVec, ",", -1, false);
   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));
 }
 
 void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
@@ -193,22 +59,25 @@ void ExceptionEscapeCheck::registerMatchers(MatchFinder *Finder) {
                          cxxConstructorDecl(isMoveConstructor()),
                          cxxMethodDecl(isMoveAssignmentOperator()),
                          hasName("main"), hasName("swap"),
-                         isEnabled(FunctionsThatShouldNotThrow)),
-                   throws(unless(isIgnored(IgnoredExceptions))))
+                         isEnabled(FunctionsThatShouldNotThrow)))
           .bind("thrower"),
       this);
 }
 
 void ExceptionEscapeCheck::check(const MatchFinder::MatchResult &Result) {
-  const FunctionDecl *MatchedDecl =
-      Result.Nodes.getNodeAs<FunctionDecl>("thrower");
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("thrower");
+
   if (!MatchedDecl)
     return;
 
-  // 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;
+  if (Tracer.throwsException(MatchedDecl))
+    // 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
index 5f4ddf2..4626e42 100644 (file)
@@ -1,4 +1,4 @@
-//===--- ExceptionEscapeCheck.h - clang-tidy---------------------*- C++ -*-===//
+//===--- ExceptionEscapeCheck.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.
@@ -10,7 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EXCEPTION_ESCAPE_H
 
 #include "../ClangTidy.h"
-
+#include "../utils/ExceptionAnalyzer.h"
 #include "llvm/ADT/StringSet.h"
 
 namespace clang {
@@ -36,7 +36,7 @@ private:
   std::string RawIgnoredExceptions;
 
   llvm::StringSet<> FunctionsThatShouldNotThrow;
-  llvm::StringSet<> IgnoredExceptions;
+  utils::ExceptionAnalyzer Tracer;
 };
 
 } // namespace bugprone
index 9162bce..68ab7a1 100644 (file)
@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support)
 add_clang_library(clangTidyUtils
   ASTUtils.cpp
   DeclRefExprUtils.cpp
+  ExceptionAnalyzer.cpp
   ExprSequence.cpp
   FixItHintUtils.cpp
   HeaderFileExtensionsUtils.cpp
diff --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
new file mode 100644 (file)
index 0000000..7f1ca52
--- /dev/null
@@ -0,0 +1,154 @@
+//===--- 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 {
+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 {
+
+ExceptionAnalyzer::TypeVec ExceptionAnalyzer::throwsException(
+    const FunctionDecl *Func,
+    llvm::SmallSet<const FunctionDecl *, 32> &CallStack) {
+  if (CallStack.count(Func))
+    return TypeVec();
+
+  if (const Stmt *Body = Func->getBody()) {
+    CallStack.insert(Func);
+    const TypeVec Result = throwsException(Body, TypeVec(), CallStack);
+    CallStack.erase(Func);
+    return Result;
+  }
+
+  TypeVec Result;
+  if (const auto *FPT = Func->getType()->getAs<FunctionProtoType>()) {
+    for (const QualType Ex : FPT->exceptions()) {
+      Result.push_back(Ex.getTypePtr());
+    }
+  }
+  return Result;
+}
+
+ExceptionAnalyzer::TypeVec ExceptionAnalyzer::throwsException(
+    const Stmt *St, const TypeVec &Caught,
+    llvm::SmallSet<const FunctionDecl *, 32> &CallStack) {
+  TypeVec Results;
+
+  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()) {
+        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());
+    }
+  } else if (const auto *Try = dyn_cast<CXXTryStmt>(St)) {
+    TypeVec Uncaught = throwsException(Try->getTryBlock(), Caught, CallStack);
+    for (unsigned i = 0; i < Try->getNumHandlers(); ++i) {
+      const CXXCatchStmt *Catch = Try->getHandler(i);
+      if (!Catch->getExceptionDecl()) {
+        const TypeVec Rethrown =
+            throwsException(Catch->getHandlerBlock(), Uncaught, CallStack);
+        Results.append(Rethrown.begin(), Rethrown.end());
+        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());
+        }
+      }
+    }
+    Results.append(Uncaught.begin(), Uncaught.end());
+  } 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());
+    }
+  } else {
+    for (const Stmt *Child : St->children()) {
+      TypeVec Excs = throwsException(Child, Caught, CallStack);
+      Results.append(Excs.begin(), Excs.end());
+    }
+  }
+  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];
+
+  llvm::SmallSet<const FunctionDecl *, 32> CallStack;
+  TypeVec ExceptionList = throwsException(Func, CallStack);
+
+  // 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;
+}
+
+} // namespace utils
+} // namespace tidy
+
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
new file mode 100644 (file)
index 0000000..327da30
--- /dev/null
@@ -0,0 +1,48 @@
+//===--- 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 "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.
+class ExceptionAnalyzer {
+public:
+  ExceptionAnalyzer() = default;
+
+  bool throwsException(const FunctionDecl *Func);
+  void ignoreExceptions(llvm::StringSet<> ExceptionNames) {
+    IgnoredExceptions = std::move(ExceptionNames);
+  }
+
+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);
+
+  llvm::StringSet<> IgnoredExceptions;
+  llvm::DenseMap<const FunctionDecl *, bool> FunctionCache;
+};
+} // namespace utils
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_ANALYZER_H