[clang-tidy] Fix bugprone-exception-escape warn on noexcept calls

[PiotrZSL]

Summary

If there is a function call that calls a non-throwing function,
this check should not report a warning, even if there is a throw
statement within that function.

The dependent_throw() test case was misleading and suggested that this
warning is not triggered when a throw exception is in unreachable code,
but in reality that warning did not trigger because it could not
determine whether the function should be noexcept without specializing
the T template argument. That test case is fixed to show non-specialized
and specialized cases.

Fixes https://github.com/llvm/llvm-project/issues/54668 and
https://github.com/llvm/llvm-project/issues/54956.

[Eugene.Zelenko]

It'll be good idea to mention change in Release Notes.

[Eugene.Zelenko]

Will be good idea to mention changes in Release Notes.

[lebedev.ri]

This review may be stuck/dead, consider abandoning if no longer relevant.
Removing myself as reviewer in attempt to clean dashboard.

[PiotrZSL]

Already fixed.

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

 }
 
 void ExceptionEscapeCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("thrower");
 
   if (!MatchedDecl)
     return;
 
-  if (Tracer.analyze(MatchedDecl).getBehaviour() ==
+  if (Tracer.analyze(MatchedDecl, *Result.Context).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-tools-extra/clang-tidy/openmp/ExceptionEscapeCheck.cpp

 void ExceptionEscapeCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Directive =
       Result.Nodes.getNodeAs<OMPExecutableDirective>("directive");
   assert(Directive && "Expected to match some OpenMP Executable directive.");
   const auto *StructuredBlock =
       Result.Nodes.getNodeAs<Stmt>("structured-block");
   assert(StructuredBlock && "Expected to get some OpenMP Structured Block.");
 
-  if (Tracer.analyze(StructuredBlock).getBehaviour() !=
+  if (Tracer.analyze(StructuredBlock, *Result.Context).getBehaviour() !=
       utils::ExceptionAnalyzer::State::Throwing)
     return; // No exceptions have been proven to escape out of the struc. block.
 
   // FIXME: We should provide more information about the exact location where
   // the exception is thrown, maybe the full path the exception escapes.
 
   diag(StructuredBlock->getBeginLoc(),
        "an exception thrown inside of the OpenMP '%0' region is not caught in "
        "that same region")
       << getOpenMPDirectiveName(Directive->getDirectiveKind());
 }
 
 } // namespace openmp
 } // namespace tidy
 } // namespace clang

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

public:
 
   ExceptionAnalyzer() = default;
 
   void ignoreBadAlloc(bool ShallIgnore) { IgnoreBadAlloc = ShallIgnore; }
   void ignoreExceptions(llvm::StringSet<> ExceptionNames) {
     IgnoredExceptions = std::move(ExceptionNames);
   }
 
-  ExceptionInfo analyze(const FunctionDecl *Func);
-  ExceptionInfo analyze(const Stmt *Stmt);
+  ExceptionInfo analyze(const FunctionDecl *Func, ASTContext &Context);
+  ExceptionInfo analyze(const Stmt *Stmt, ASTContext &Context);
 
 private:
   ExceptionInfo
   throwsException(const FunctionDecl *Func,
-                  llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
+                  llvm::SmallSet<const FunctionDecl *, 32> &CallStack,
+                  ASTContext &Context);
   ExceptionInfo
   throwsException(const Stmt *St, const ExceptionInfo::Throwables &Caught,
-                  llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
+                  llvm::SmallSet<const FunctionDecl *, 32> &CallStack,
+                  ASTContext &Context);
 
-  ExceptionInfo analyzeImpl(const FunctionDecl *Func);
-  ExceptionInfo analyzeImpl(const Stmt *Stmt);
+  ExceptionInfo analyzeImpl(const FunctionDecl *Func, ASTContext &Context);
+  ExceptionInfo analyzeImpl(const Stmt *Stmt, ASTContext &Context);
 
-  template <typename T> ExceptionInfo analyzeDispatch(const T *Node);
+  template <typename T>
+  ExceptionInfo analyzeDispatch(const T *Node, ASTContext &Context);
 
   bool IgnoreBadAlloc = true;
   llvm::StringSet<> IgnoredExceptions;
   std::map<const FunctionDecl *, ExceptionInfo> FunctionCache;
 };
 
 } // namespace utils
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_ANALYZER_H

clang-tools-extra/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"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 
+using namespace ast_matchers;
+
 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(
if (ThrownExceptions.size() == 0)
     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))
+    llvm::SmallSet<const FunctionDecl *, 32> &CallStack, ASTContext &Context) {
+  if (CallStack.count(Func) ||
+      (!CallStack.empty() &&
+       !match(functionDecl(isNoThrow()), *Func, Context).empty()))
     return ExceptionInfo::createNonThrowing();
 
   if (const Stmt *Body = Func->getBody()) {
     CallStack.insert(Func);
     ExceptionInfo Result =
-        throwsException(Body, ExceptionInfo::Throwables(), CallStack);
+        throwsException(Body, ExceptionInfo::Throwables(), CallStack, Context);
 
     // For a constructor, we also have to check the initializers.
     if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(Func)) {
       for (const CXXCtorInitializer *Init : Ctor->inits()) {
         ExceptionInfo Excs = throwsException(
-            Init->getInit(), ExceptionInfo::Throwables(), CallStack);
+            Init->getInit(), ExceptionInfo::Throwables(), CallStack, Context);
         Result.merge(Excs);
       }
     }
 
     CallStack.erase(Func);
     return Result;
   }
 
   auto Result = ExceptionInfo::createUnknown();
   if (const auto *FPT = Func->getType()->getAs<FunctionProtoType>()) {
     for (const QualType &Ex : FPT->exceptions())
       Result.registerException(Ex.getTypePtr());
   }
   return Result;
 }
 
 /// Analyzes a single statement 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) {
+    llvm::SmallSet<const FunctionDecl *, 32> &CallStack, ASTContext &Context) {
   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())
         ThrownType = ThrownType->castAs<ReferenceType>()
                          ->getPointeeType()
                          ->getUnqualifiedDesugaredType();
       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)) {
     ExceptionInfo Uncaught =
-        throwsException(Try->getTryBlock(), Caught, CallStack);
+        throwsException(Try->getTryBlock(), Caught, CallStack, Context);
     for (unsigned I = 0; I < Try->getNumHandlers(); ++I) {
       const CXXCatchStmt *Catch = Try->getHandler(I);
 
       // Everything is catched through 'catch(...)'.
       if (!Catch->getExceptionDecl()) {
-        ExceptionInfo Rethrown = throwsException(
-            Catch->getHandlerBlock(), Uncaught.getExceptionTypes(), CallStack);
+        ExceptionInfo Rethrown =
+            throwsException(Catch->getHandlerBlock(),
+                            Uncaught.getExceptionTypes(), CallStack, Context);
         Results.merge(Rethrown);
         Uncaught.clear();
       } else {
         const auto *CaughtType =
             Catch->getCaughtType()->getUnqualifiedDesugaredType();
         if (CaughtType->isReferenceType()) {
           CaughtType = CaughtType->castAs<ReferenceType>()
                            ->getPointeeType()
                            ->getUnqualifiedDesugaredType();
         }
 
         // 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);
+          ExceptionInfo Rethrown = throwsException(
+              Catch->getHandlerBlock(), CaughtExceptions, CallStack, Context);
           Results.merge(Rethrown);
         }
       }
     }
     Results.merge(Uncaught);
   } else if (const auto *Call = dyn_cast<CallExpr>(St)) {
     if (const FunctionDecl *Func = Call->getDirectCallee()) {
-      ExceptionInfo Excs = throwsException(Func, CallStack);
+      ExceptionInfo Excs = throwsException(Func, CallStack, Context);
       Results.merge(Excs);
     }
   } else if (const auto *Construct = dyn_cast<CXXConstructExpr>(St)) {
     ExceptionInfo Excs =
-        throwsException(Construct->getConstructor(), CallStack);
+        throwsException(Construct->getConstructor(), CallStack, Context);
     Results.merge(Excs);
   } else if (const auto *DefaultInit = dyn_cast<CXXDefaultInitExpr>(St)) {
     ExceptionInfo Excs =
-        throwsException(DefaultInit->getExpr(), Caught, CallStack);
+        throwsException(DefaultInit->getExpr(), Caught, CallStack, Context);
     Results.merge(Excs);
   } else {
     for (const Stmt *Child : St->children()) {
-      ExceptionInfo Excs = throwsException(Child, Caught, CallStack);
+      ExceptionInfo Excs = throwsException(Child, Caught, CallStack, Context);
       Results.merge(Excs);
     }
   }
   return Results;
 }
 
 ExceptionAnalyzer::ExceptionInfo
-ExceptionAnalyzer::analyzeImpl(const FunctionDecl *Func) {
+ExceptionAnalyzer::analyzeImpl(const FunctionDecl *Func, ASTContext &Context) {
   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;
-    ExceptionList = throwsException(Func, CallStack);
+    ExceptionList = throwsException(Func, CallStack, Context);
 
     // 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];
 
   return ExceptionList;
 }
 
 ExceptionAnalyzer::ExceptionInfo
-ExceptionAnalyzer::analyzeImpl(const Stmt *Stmt) {
+ExceptionAnalyzer::analyzeImpl(const Stmt *Stmt, ASTContext &Context) {
   llvm::SmallSet<const FunctionDecl *, 32> CallStack;
-  return throwsException(Stmt, ExceptionInfo::Throwables(), CallStack);
+  return throwsException(Stmt, ExceptionInfo::Throwables(), CallStack, Context);
 }
 
 template <typename T>
 ExceptionAnalyzer::ExceptionInfo
-ExceptionAnalyzer::analyzeDispatch(const T *Node) {
-  ExceptionInfo ExceptionList = analyzeImpl(Node);
+ExceptionAnalyzer::analyzeDispatch(const T *Node, ASTContext &Context) {
+  ExceptionInfo ExceptionList = analyzeImpl(Node, Context);
 
   if (ExceptionList.getBehaviour() == State::NotThrowing ||
       ExceptionList.getBehaviour() == State::Unknown)
     return ExceptionList;
 
   // Remove all ignored exceptions from the list of exceptions that can be
   // thrown.
   ExceptionList.filterIgnoredExceptions(IgnoredExceptions, IgnoreBadAlloc);
 
   return ExceptionList;
 }
 
 ExceptionAnalyzer::ExceptionInfo
-ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
-  return analyzeDispatch(Func);
+ExceptionAnalyzer::analyze(const FunctionDecl *Func, ASTContext &Context) {
+  return analyzeDispatch(Func, Context);
 }
 
 ExceptionAnalyzer::ExceptionInfo
-ExceptionAnalyzer::analyze(const Stmt *Stmt) {
-  return analyzeDispatch(Stmt);
+ExceptionAnalyzer::analyze(const Stmt *Stmt, ASTContext &Context) {
+  return analyzeDispatch(Stmt, Context);
 }
 
 } // namespace utils
 } // namespace tidy
 
 } // namespace clang

clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp

struct throwing_move_assignment {
   }
 };
 
 void throwing_noexcept() noexcept {
     // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throwing_noexcept' which should not throw exceptions
   throw 1;
 }
 
+void indirect_noexcept() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_noexcept' which should not throw exceptions
+  throwing_noexcept();
+}
+
 void throwing_throw_nothing() throw() {
     // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throwing_throw_nothing' which should not throw exceptions
   throw 1;
 }
 
+void indirect_throw_nothing() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_throw_nothing' which should not throw exceptions
+  throwing_throw_nothing();
+}
+
 void throw_and_catch() noexcept {
   // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch' which should not throw exceptions
   try {
     throw 1;
   } catch(int &) {
   }
 }
 
try {
     implicit_int_thrower();
   } catch(int&) {
   }
 }
 
 template<typename T>
 void dependent_throw() noexcept(sizeof(T)<4) {
   // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'dependent_throw' which should not throw exceptions
-  if (sizeof(T) > 4)
-    throw 1;
+  // CHECK-MESSAGES-NOT: :[[@LINE-2]]:6: warning: an exception may be thrown in function 'dependent_throw<int>' which should not throw exceptions
+  // CHECK-MESSAGES: :[[@LINE-3]]:6: warning: an exception may be thrown in function 'dependent_throw<char>' which should not throw exceptions
 }
 
+void indirect_dependent_noexcept_true() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_dependent_noexcept_true' which should not throw exceptions
+  dependent_throw<char>();
+}
+
+void indirect_dependent_noexcept_false() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_dependent_noexcept_false' which should not throw exceptions
+  dependent_throw<int>();
+}
+
 void swap(int&, int&) {
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'swap' which should not throw exceptions
   throw 1;
 }
 
 namespace std {
 class bad_alloc {};
 }