https://github.com/snarang181 created https://github.com/llvm/llvm-project/pull/147447
This patch adds a new AnalysisBasedWarnings pass that runs right before IssueWarnings: 1. Pass 1 seeds trivial always-throwing functions. 2. Pass 2 performs a fixed-point CFG-based analysis (CheckFallThrough) to propagate noreturn inference to any caller whose all paths end in calls to already-inferred noreturn functions. To support templates, we also walk each FunctionTemplateDecl’s specializations so that instantiations (e.g. ensureZeroTemplate<int>) get analyzed. >From 21f0d364933cd59acaa1fb323c1018dbce7c3452 Mon Sep 17 00:00:00 2001 From: Samarth Narang <snar...@umass.edu> Date: Mon, 7 Jul 2025 19:02:38 -0700 Subject: [PATCH] Add logic to suppress noreturn falling for some basic control flow --- .../clang/Sema/AnalysisBasedWarnings.h | 14 +++++ clang/include/clang/Sema/Sema.h | 2 +- clang/lib/AST/Decl.cpp | 2 +- clang/lib/Sema/AnalysisBasedWarnings.cpp | 39 +++++++++--- clang/lib/Sema/Sema.cpp | 3 +- clang/lib/Sema/SemaDeclAttr.cpp | 59 +++++++++++++------ .../wreturn-always-throws-control-flow.cpp | 31 ++++++++++ 7 files changed, 121 insertions(+), 29 deletions(-) create mode 100644 clang/test/SemaCXX/wreturn-always-throws-control-flow.cpp diff --git a/clang/include/clang/Sema/AnalysisBasedWarnings.h b/clang/include/clang/Sema/AnalysisBasedWarnings.h index 4103c3f006a8f..21627e6729743 100644 --- a/clang/include/clang/Sema/AnalysisBasedWarnings.h +++ b/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 diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index b281b1cfef96a..6769d48d5f7ee 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/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 diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 8855d0107daca..52d0c48f93457 100644 --- a/clang/lib/AST/Decl.cpp +++ b/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>()) diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 7420ba2d461c6..0f6c025afa9ec 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/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) { diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index 56608e990fd50..7b33eaa966807 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/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) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index c5d5d03cc99c7..546adaf85f215 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/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) { diff --git a/clang/test/SemaCXX/wreturn-always-throws-control-flow.cpp b/clang/test/SemaCXX/wreturn-always-throws-control-flow.cpp new file mode 100644 index 0000000000000..1b61bec76f833 --- /dev/null +++ b/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); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits