Add logic to suppress noreturn falling for some basic control flow

Author: snarang181

clang/include/clang/Sema/AnalysisBasedWarnings.h

@@ -14,6 +14,7 @@
 #define LLVM_CLANG_SEMA_ANALYSISBASEDWARNINGS_H
 
 #include "clang/AST/Decl.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
 #include "llvm/ADT/DenseMap.h"
 #include <memory>
 
@@ -107,6 +108,8 @@ class AnalysisBasedWarnings {
   // Issue warnings that require whole-translation-unit analysis.
   void IssueWarnings(TranslationUnitDecl *D);
 
+  void InferNoReturnAttributes(Sema &S, TranslationUnitDecl *TU);
+
   // Gets the default policy which is in effect at the given source location.
   Policy getPolicyInEffectAt(SourceLocation Loc);
 
@@ -119,6 +122,17 @@ class AnalysisBasedWarnings {
 };
 
 } // namespace sema
+
+enum ControlFlowKind {
+  UnknownFallThrough,
+  NeverFallThrough,
+  MaybeFallThrough,
+  AlwaysFallThrough,
+  NeverFallThroughOrReturn
+};
+
+ControlFlowKind CheckFallThrough(AnalysisDeclContext &AC);
+
 } // namespace clang
 
 #endif

clang/include/clang/Sema/Sema.h

@@ -834,7 +834,7 @@ enum class CCEKind {
                            ///< message.
 };
 
-void inferNoReturnAttr(Sema &S, const Decl *D);
+bool inferNoReturnAttr(Sema &S, const Decl *D, bool FirstPass);
 
 /// Sema - This implements semantic analysis and AST building for C.
 /// \nosubgrouping

clang/lib/AST/Decl.cpp

@@ -3587,7 +3587,7 @@ bool FunctionDecl::isGlobal() const {
 
 bool FunctionDecl::isNoReturn() const {
   if (hasAttr<NoReturnAttr>() || hasAttr<CXX11NoReturnAttr>() ||
-      hasAttr<C11NoReturnAttr>())
+      hasAttr<C11NoReturnAttr>() || hasAttr<InferredNoReturnAttr>())
     return true;
 
   if (auto *FnTy = getType()->getAs<FunctionType>())

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -403,14 +403,6 @@ static bool isNoexcept(const FunctionDecl *FD) {
 // Check for missing return value.
 //===----------------------------------------------------------------------===//
 
-enum ControlFlowKind {
-  UnknownFallThrough,
-  NeverFallThrough,
-  MaybeFallThrough,
-  AlwaysFallThrough,
-  NeverFallThroughOrReturn
-};
-
 /// CheckFallThrough - Check that we don't fall off the end of a
 /// Statement that should return a value.
 ///
@@ -420,7 +412,7 @@ enum ControlFlowKind {
 /// return.  We assume NeverFallThrough iff we never fall off the end of the
 /// statement but we may return.  We assume that functions not marked noreturn
 /// will return.
-static ControlFlowKind CheckFallThrough(AnalysisDeclContext &AC) {
+clang::ControlFlowKind clang::CheckFallThrough(AnalysisDeclContext &AC) {
   CFG *cfg = AC.getCFG();
   if (!cfg) return UnknownFallThrough;
 
@@ -2678,6 +2670,35 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
   }
 }
 
+void clang::sema::AnalysisBasedWarnings::InferNoReturnAttributes(
+    Sema &S, TranslationUnitDecl *TU) {
+  llvm::SmallVector<Decl *, 32> AllFunctions;
+
+  for (Decl *D : TU->decls()) {
+    // Collect ordinary (non-template) functions and methods:
+    if (auto *FD = dyn_cast<FunctionDecl>(D)) {
+      if (!FD->getDescribedFunctionTemplate() && FD->hasBody())
+        AllFunctions.push_back(FD);
+    }
+
+    // For each function template, include all its instantiations:
+    if (auto *FT = dyn_cast<FunctionTemplateDecl>(D)) {
+      for (auto *Spec : FT->specializations()) {
+        if (auto *FDs = dyn_cast<FunctionDecl>(Spec))
+          if (FDs->hasBody())
+            AllFunctions.push_back(FDs);
+      }
+    }
+  }
+
+  // Pass 1: Mark all trivial always-throwing functions.
+  for (Decl *D : AllFunctions)
+    inferNoReturnAttr(S, D, /*FirstPass*/ true);
+  // Pass 2:  Run until no changes.
+  for (Decl *D : AllFunctions)
+    inferNoReturnAttr(S, D, /*FirstPass*/ false);
+}
+
 void clang::sema::AnalysisBasedWarnings::IssueWarnings(
     sema::AnalysisBasedWarnings::Policy P, sema::FunctionScopeInfo *fscope,
     const Decl *D, QualType BlockType) {

clang/lib/Sema/Sema.cpp

@@ -2453,7 +2453,8 @@ Sema::PopFunctionScopeInfo(const AnalysisBasedWarnings::Policy *WP,
 
   // Issue any analysis-based warnings.
   if (WP && D) {
-    inferNoReturnAttr(*this, D);
+    AnalysisWarnings.InferNoReturnAttributes(
+        *this, getASTContext().getTranslationUnitDecl());
     AnalysisWarnings.IssueWarnings(*WP, Scope.get(), D, BlockType);
   } else
     for (const auto &PUD : Scope->PossiblyUnreachableDiags)

clang/lib/Sema/SemaDeclAttr.cpp

@@ -23,6 +23,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/Mangle.h"
 #include "clang/AST/Type.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Cuda.h"
 #include "clang/Basic/DarwinSDKInfo.h"
@@ -32,6 +33,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Sema/AnalysisBasedWarnings.h"
 #include "clang/Sema/Attr.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/DelayedDiagnostic.h"
@@ -1939,11 +1941,7 @@ static void handleNakedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   D->addAttr(::new (S.Context) NakedAttr(S.Context, AL));
 }
 
-// FIXME: This is a best-effort heuristic.
-// Currently only handles single throw expressions (optionally with
-// ExprWithCleanups). We could expand this to perform control-flow analysis for
-// more complex patterns.
-static bool isKnownToAlwaysThrow(const FunctionDecl *FD) {
+static bool alwaysThrowsDirect(const FunctionDecl *FD) {
   if (!FD->hasBody())
     return false;
   const Stmt *Body = FD->getBody();
@@ -1965,25 +1963,52 @@ static bool isKnownToAlwaysThrow(const FunctionDecl *FD) {
   return isa<CXXThrowExpr>(OnlyStmt);
 }
 
-void clang::inferNoReturnAttr(Sema &S, const Decl *D) {
+static bool isKnownToAlwaysThrowCFG(const FunctionDecl *FD, Sema &S) {
+  if (!FD->hasBody())
+    return false;
+
+  // Drop the const qualifier to do an analysis.
+  FunctionDecl *NonConstFD = const_cast<FunctionDecl *>(FD);
+  AnalysisDeclContext AC(nullptr, NonConstFD);
+
+  switch (CheckFallThrough(AC)) {
+  case NeverFallThrough:
+  case NeverFallThroughOrReturn:
+    return true;
+  case AlwaysFallThrough:
+  case UnknownFallThrough:
+  case MaybeFallThrough:
+    return false;
+  }
+}
+
+bool clang::inferNoReturnAttr(Sema &S, const Decl *D, bool FirstPass) {
   auto *FD = dyn_cast<FunctionDecl>(D);
   if (!FD)
-    return;
+    return false;
+  if (FD->isInvalidDecl())
+    return false;
+  if (FD->hasAttr<NoReturnAttr>() || FD->hasAttr<InferredNoReturnAttr>())
+    return false;
 
   auto *NonConstFD = const_cast<FunctionDecl *>(FD);
-  DiagnosticsEngine &Diags = S.getDiagnostics();
-  if (Diags.isIgnored(diag::warn_falloff_nonvoid, FD->getLocation()) &&
-      Diags.isIgnored(diag::warn_suggest_noreturn_function, FD->getLocation()))
-    return;
 
-  if (!FD->hasAttr<NoReturnAttr>() && !FD->hasAttr<InferredNoReturnAttr>() &&
-      isKnownToAlwaysThrow(FD)) {
-    NonConstFD->addAttr(InferredNoReturnAttr::CreateImplicit(S.Context));
+  auto tryAddInferredNoReturn = [&](bool Condition) {
+    if (Condition) {
+      NonConstFD->addAttr(InferredNoReturnAttr::CreateImplicit(S.Context));
+      if (FD->getReturnType()->isVoidType())
+        S.Diag(FD->getLocation(), diag::warn_suggest_noreturn_function)
+            << /*isFunction=*/0 << FD;
+      return true;
+    }
+    return false;
+  };
 
-    // Emit a diagnostic suggesting the function being marked [[noreturn]].
-    S.Diag(FD->getLocation(), diag::warn_suggest_noreturn_function)
-        << /*isFunction=*/0 << FD;
+  if (FirstPass) {
+    // Look for trivial function bodies that always throw.
+    return tryAddInferredNoReturn(alwaysThrowsDirect(FD));
   }
+  return tryAddInferredNoReturn(isKnownToAlwaysThrowCFG(FD, S));
 }
 
 static void handleNoReturnAttr(Sema &S, Decl *D, const ParsedAttr &Attrs) {

clang/test/SemaCXX/wreturn-always-throws-control-flow.cpp

@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -fexceptions -Wreturn-type -verify %s
+// expected-no-diagnostics
+
+namespace std {
+  class string {
+  public:
+    string(const char*); // constructor for runtime_error
+  };
+  class runtime_error {
+  public:
+    runtime_error(const string &); 
+  };
+}
+
+void throwA() {
+  throw std::runtime_error("ERROR A");
+}
+
+void throwB(int n) {
+    if (n)
+        throw std::runtime_error("ERROR B");
+    else
+        throw std::runtime_error("ERROR B with n=0");
+}
+
+int test(int x) {
+    if (x > 0) 
+        throwA(); 
+    else 
+        throwB(x); 
+}