https://github.com/snarang181 updated https://github.com/llvm/llvm-project/pull/145166
>From 602b472aadebe365cb9f499ce85687a5e31605b8 Mon Sep 17 00:00:00 2001 From: Samarth Narang <snar...@umass.edu> Date: Sat, 21 Jun 2025 08:42:00 -0400 Subject: [PATCH 1/7] Suppress noreturn warning if last statement in a function is a throw --- clang/lib/Sema/AnalysisBasedWarnings.cpp | 45 ++++++++++++++++++++ clang/test/SemaCXX/wreturn-always-throws.cpp | 22 ++++++++++ 2 files changed, 67 insertions(+) create mode 100644 clang/test/SemaCXX/wreturn-always-throws.cpp diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 2a107a36e24b4..85a5d99c710d5 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -626,6 +626,31 @@ struct CheckFallThroughDiagnostics { } // anonymous namespace +static bool isKnownToAlwaysThrow(const FunctionDecl *FD) { + if (!FD->hasBody()) + return false; + const Stmt *Body = FD->getBody(); + const Stmt *OnlyStmt = nullptr; + + if (const auto *Compound = dyn_cast<CompoundStmt>(Body)) { + if (Compound->size() != 1) + return false; // More than one statement, can't be known to always throw. + OnlyStmt = *Compound->body_begin(); + } else { + OnlyStmt = Body; + } + + // Unwrap ExprWithCleanups if necessary. + if (const auto *EWC = dyn_cast<ExprWithCleanups>(OnlyStmt)) { + OnlyStmt = EWC->getSubExpr(); + } + // Check if the only statement is a throw expression. + if (isa<CXXThrowExpr>(OnlyStmt)) { + return true; // Known to always throw. + } + return false; // Not known to always throw. +} + /// CheckFallThroughForBody - Check that we don't fall off the end of a /// function that should return a value. Check that we don't fall off the end /// of a noreturn function. We assume that functions and blocks not marked @@ -681,6 +706,26 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, if (CD.diag_FallThrough_HasNoReturn) S.Diag(RBrace, CD.diag_FallThrough_HasNoReturn) << CD.FunKind; } else if (!ReturnsVoid && CD.diag_FallThrough_ReturnsNonVoid) { + // If the final statement is a call to an always-throwing function, + // don't warn about the fall-through. + if (const auto *FD = dyn_cast<FunctionDecl>(D)) { + if (const auto *CS = dyn_cast<CompoundStmt>(Body)) { + if (!CS->body_empty()) { + const Stmt *LastStmt = CS->body_back(); + // Unwrap ExprWithCleanups if necessary. + if (const auto *EWC = dyn_cast<ExprWithCleanups>(LastStmt)) { + LastStmt = EWC->getSubExpr(); + } + if (const auto *CE = dyn_cast<CallExpr>(LastStmt)) { + if (const FunctionDecl *Callee = CE->getDirectCallee()) { + if (isKnownToAlwaysThrow(Callee)) { + return; // Don't warn about fall-through. + } + } + } + } + } + } bool NotInAllControlPaths = FallThroughType == MaybeFallThrough; S.Diag(RBrace, CD.diag_FallThrough_ReturnsNonVoid) << CD.FunKind << NotInAllControlPaths; diff --git a/clang/test/SemaCXX/wreturn-always-throws.cpp b/clang/test/SemaCXX/wreturn-always-throws.cpp new file mode 100644 index 0000000000000..0bbce0f5c1871 --- /dev/null +++ b/clang/test/SemaCXX/wreturn-always-throws.cpp @@ -0,0 +1,22 @@ +// 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 throwError(const std::string& msg) { + throw std::runtime_error(msg); +} + +int ensureZero(const int i) { + if (i == 0) return 0; + throwError("ERROR"); // no-warning +} >From 60c5a28a2e6caee24dc689948857c27f4a77fe89 Mon Sep 17 00:00:00 2001 From: Samarth Narang <70980689+snarang...@users.noreply.github.com> Date: Sat, 21 Jun 2025 11:05:17 -0400 Subject: [PATCH 2/7] Update clang/lib/Sema/AnalysisBasedWarnings.cpp Co-authored-by: Copilot <175728472+copi...@users.noreply.github.com> --- clang/lib/Sema/AnalysisBasedWarnings.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 85a5d99c710d5..67e379fdd7d38 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -624,9 +624,7 @@ struct CheckFallThroughDiagnostics { } }; -} // anonymous namespace - -static bool isKnownToAlwaysThrow(const FunctionDecl *FD) { +bool isKnownToAlwaysThrow(const FunctionDecl *FD) { if (!FD->hasBody()) return false; const Stmt *Body = FD->getBody(); @@ -651,6 +649,7 @@ static bool isKnownToAlwaysThrow(const FunctionDecl *FD) { return false; // Not known to always throw. } +} // anonymous namespace /// CheckFallThroughForBody - Check that we don't fall off the end of a /// function that should return a value. Check that we don't fall off the end /// of a noreturn function. We assume that functions and blocks not marked >From 65126bc67c48e14b2efe2530db9516e1db16aa32 Mon Sep 17 00:00:00 2001 From: Samarth Narang <snar...@umass.edu> Date: Sat, 21 Jun 2025 11:29:58 -0400 Subject: [PATCH 3/7] Handle direct throws in functions. --- clang/lib/Sema/AnalysisBasedWarnings.cpp | 4 ++++ clang/test/SemaCXX/wreturn-always-throws.cpp | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 67e379fdd7d38..9502669612f74 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -722,6 +722,10 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, } } } + // Direct throw. + if (isa<CXXThrowExpr>(LastStmt)) { + return; // Don't warn about fall-through. + } } } } diff --git a/clang/test/SemaCXX/wreturn-always-throws.cpp b/clang/test/SemaCXX/wreturn-always-throws.cpp index 0bbce0f5c1871..5d2d2f0dffefe 100644 --- a/clang/test/SemaCXX/wreturn-always-throws.cpp +++ b/clang/test/SemaCXX/wreturn-always-throws.cpp @@ -20,3 +20,7 @@ int ensureZero(const int i) { if (i == 0) return 0; throwError("ERROR"); // no-warning } + +int alwaysThrows() { + throw std::runtime_error("This function always throws"); // no-warning +} >From c9a6d21785db5be0e7e7654a4e5982e4829e7db6 Mon Sep 17 00:00:00 2001 From: Samarth Narang <snar...@umass.edu> Date: Sat, 21 Jun 2025 12:34:41 -0400 Subject: [PATCH 4/7] Address review comments Add a note in the release notes about the fix for the no-return function warning --- clang/docs/ReleaseNotes.rst | 6 ++++ clang/lib/Sema/AnalysisBasedWarnings.cpp | 35 ++++++++++-------------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 86216f1503457..60d991831534b 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -648,6 +648,12 @@ Improvements to Clang's diagnostics #GH69470, #GH59391, #GH58172, #GH46215, #GH45915, #GH45891, #GH44490, #GH36703, #GH32903, #GH23312, #GH69874. +- Clang no longer warns about missing return statements (-Wreturn-type) + if the final statement of a non-void function is a `throw` expression + or a call to a function that is known to always throw. This avoids + false positives in code patterns where control flow is intentionally + terminated via exceptions. + Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 9502669612f74..ac9ba762adeb3 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -643,10 +643,7 @@ bool isKnownToAlwaysThrow(const FunctionDecl *FD) { OnlyStmt = EWC->getSubExpr(); } // Check if the only statement is a throw expression. - if (isa<CXXThrowExpr>(OnlyStmt)) { - return true; // Known to always throw. - } - return false; // Not known to always throw. + return isa<CXXThrowExpr>(OnlyStmt); } } // anonymous namespace @@ -708,25 +705,23 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, // If the final statement is a call to an always-throwing function, // don't warn about the fall-through. if (const auto *FD = dyn_cast<FunctionDecl>(D)) { - if (const auto *CS = dyn_cast<CompoundStmt>(Body)) { - if (!CS->body_empty()) { - const Stmt *LastStmt = CS->body_back(); - // Unwrap ExprWithCleanups if necessary. - if (const auto *EWC = dyn_cast<ExprWithCleanups>(LastStmt)) { - LastStmt = EWC->getSubExpr(); - } - if (const auto *CE = dyn_cast<CallExpr>(LastStmt)) { - if (const FunctionDecl *Callee = CE->getDirectCallee()) { - if (isKnownToAlwaysThrow(Callee)) { - return; // Don't warn about fall-through. - } - } - } - // Direct throw. - if (isa<CXXThrowExpr>(LastStmt)) { + if (const auto *CS = dyn_cast<CompoundStmt>(Body); + CS && !CS->body_empty()) { + const Stmt *LastStmt = CS->body_back(); + // Unwrap ExprWithCleanups if necessary. + if (const auto *EWC = dyn_cast<ExprWithCleanups>(LastStmt)) { + LastStmt = EWC->getSubExpr(); + } + if (const auto *CE = dyn_cast<CallExpr>(LastStmt)) { + if (const FunctionDecl *Callee = CE->getDirectCallee(); + Callee && isKnownToAlwaysThrow(Callee)) { return; // Don't warn about fall-through. } } + // Direct throw. + if (isa<CXXThrowExpr>(LastStmt)) { + return; // Don't warn about fall-through. + } } } bool NotInAllControlPaths = FallThroughType == MaybeFallThrough; >From d9b3920e63cc78605ea3bd2a1bcb5a225709343f Mon Sep 17 00:00:00 2001 From: Samarth Narang <snar...@umass.edu> Date: Tue, 24 Jun 2025 19:14:45 -0400 Subject: [PATCH 5/7] Make clear in the release notes and inline comments that this is a conservative fix and not a complete solution to the no-return function analysis. --- clang/docs/ReleaseNotes.rst | 11 ++++++----- clang/lib/Sema/AnalysisBasedWarnings.cpp | 7 ++++++- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 60d991831534b..c035d53c0d31a 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -648,11 +648,12 @@ Improvements to Clang's diagnostics #GH69470, #GH59391, #GH58172, #GH46215, #GH45915, #GH45891, #GH44490, #GH36703, #GH32903, #GH23312, #GH69874. -- Clang no longer warns about missing return statements (-Wreturn-type) - if the final statement of a non-void function is a `throw` expression - or a call to a function that is known to always throw. This avoids - false positives in code patterns where control flow is intentionally - terminated via exceptions. +- Clang now avoids issuing `-Wreturn-type` warnings in some cases where + the final statement of a non-void function is a `throw` expression, or + a call to a function that is trivially known to always throw (i.e., its + body consists solely of a `throw` statement). This avoids certain + false positives in exception-heavy code, though only simple patterns + are currently recognized. Improvements to Clang's time-trace diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index ac9ba762adeb3..e1b0b56a4635a 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -624,6 +624,11 @@ struct CheckFallThroughDiagnostics { } }; +// FIXME: This is a shallow best-effort check. Currently, we only handle +// cases where the function body consists of a single `throw` expression, +// possibly wrapped in an `ExprWithCleanups`. We do not perform general +// control-flow analysis or handle more complex throwing patterns. +// Consider expanding this to handle more cases in the future. bool isKnownToAlwaysThrow(const FunctionDecl *FD) { if (!FD->hasBody()) return false; @@ -704,7 +709,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, } else if (!ReturnsVoid && CD.diag_FallThrough_ReturnsNonVoid) { // If the final statement is a call to an always-throwing function, // don't warn about the fall-through. - if (const auto *FD = dyn_cast<FunctionDecl>(D)) { + if (const auto *FD = D->getAsFunction()) { if (const auto *CS = dyn_cast<CompoundStmt>(Body); CS && !CS->body_empty()) { const Stmt *LastStmt = CS->body_back(); >From 8cca1b0b1041e0830c750df79d3ee20ecbff2094 Mon Sep 17 00:00:00 2001 From: Samarth Narang <snar...@umass.edu> Date: Thu, 26 Jun 2025 23:32:51 -0400 Subject: [PATCH 6/7] Clang now suppresses -Wreturn-type diagnostics for non-void functions that always throw, by inferring an implicit InferredNoReturnAttr on simple cases (single throw statements). --- clang/include/clang/Basic/Attr.td | 7 +++++ clang/include/clang/Sema/Sema.h | 2 ++ clang/lib/Sema/AnalysisBasedWarnings.cpp | 32 ++----------------- clang/lib/Sema/Sema.cpp | 7 +++-- clang/lib/Sema/SemaDeclAttr.cpp | 39 ++++++++++++++++++++++++ 5 files changed, 56 insertions(+), 31 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 5d8ce6b650d9c..5bf7e5400173a 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -965,6 +965,13 @@ def AnalyzerNoReturn : InheritableAttr { let Documentation = [Undocumented]; } +def InferredNoReturn : InheritableAttr { + let Spellings = []; + let SemaHandler = 0; + let Documentation = [Undocumented]; + let Subjects = SubjectList<[Function], ErrorDiag>; +} + def Annotate : InheritableParamOrStmtAttr { let Spellings = [Clang<"annotate">]; let Args = [StringArgument<"Annotation">, VariadicExprArgument<"Args">]; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 9397546c8fc5d..4ded9ffae7d71 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -834,6 +834,8 @@ enum class CCEKind { ///< message. }; +void inferNoReturnAttr(Sema &S, FunctionDecl *FD); + /// Sema - This implements semantic analysis and AST building for C. /// \nosubgrouping class Sema final : public SemaBase { diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index e1b0b56a4635a..f3798bb6a7450 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -624,34 +624,8 @@ struct CheckFallThroughDiagnostics { } }; -// FIXME: This is a shallow best-effort check. Currently, we only handle -// cases where the function body consists of a single `throw` expression, -// possibly wrapped in an `ExprWithCleanups`. We do not perform general -// control-flow analysis or handle more complex throwing patterns. -// Consider expanding this to handle more cases in the future. -bool isKnownToAlwaysThrow(const FunctionDecl *FD) { - if (!FD->hasBody()) - return false; - const Stmt *Body = FD->getBody(); - const Stmt *OnlyStmt = nullptr; - - if (const auto *Compound = dyn_cast<CompoundStmt>(Body)) { - if (Compound->size() != 1) - return false; // More than one statement, can't be known to always throw. - OnlyStmt = *Compound->body_begin(); - } else { - OnlyStmt = Body; - } - - // Unwrap ExprWithCleanups if necessary. - if (const auto *EWC = dyn_cast<ExprWithCleanups>(OnlyStmt)) { - OnlyStmt = EWC->getSubExpr(); - } - // Check if the only statement is a throw expression. - return isa<CXXThrowExpr>(OnlyStmt); -} - } // anonymous namespace + /// CheckFallThroughForBody - Check that we don't fall off the end of a /// function that should return a value. Check that we don't fall off the end /// of a noreturn function. We assume that functions and blocks not marked @@ -669,7 +643,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, ReturnsVoid = CBody->getFallthroughHandler() != nullptr; else ReturnsVoid = FD->getReturnType()->isVoidType(); - HasNoReturn = FD->isNoReturn(); + HasNoReturn = FD->isNoReturn() || FD->hasAttr<InferredNoReturnAttr>(); } else if (const auto *MD = dyn_cast<ObjCMethodDecl>(D)) { ReturnsVoid = MD->getReturnType()->isVoidType(); @@ -719,7 +693,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, } if (const auto *CE = dyn_cast<CallExpr>(LastStmt)) { if (const FunctionDecl *Callee = CE->getDirectCallee(); - Callee && isKnownToAlwaysThrow(Callee)) { + Callee && Callee->hasAttr<InferredNoReturnAttr>()) { return; // Don't warn about fall-through. } } diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index 42ebf2a508a26..b482588dcf6e6 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -2434,9 +2434,12 @@ Sema::PopFunctionScopeInfo(const AnalysisBasedWarnings::Policy *WP, OpenMP().popOpenMPFunctionRegion(Scope.get()); // Issue any analysis-based warnings. - if (WP && D) + if (WP && D) { + if (auto *FD = dyn_cast<FunctionDecl>(D)) { + inferNoReturnAttr(*this, const_cast<FunctionDecl *>(FD)); + } AnalysisWarnings.IssueWarnings(*WP, Scope.get(), D, BlockType); - else + } else for (const auto &PUD : Scope->PossiblyUnreachableDiags) Diag(PUD.Loc, PUD.PD); diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index eba29e609cb05..d20ca94657940 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -40,6 +40,7 @@ #include "clang/Sema/ParsedAttr.h" #include "clang/Sema/Scope.h" #include "clang/Sema/ScopeInfo.h" +#include "clang/Sema/Sema.h" #include "clang/Sema/SemaAMDGPU.h" #include "clang/Sema/SemaARM.h" #include "clang/Sema/SemaAVR.h" @@ -1938,6 +1939,44 @@ 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) { + if (!FD->hasBody()) + return false; + const Stmt *Body = FD->getBody(); + const Stmt *OnlyStmt = nullptr; + + if (const auto *Compound = dyn_cast<CompoundStmt>(Body)) { + if (Compound->size() != 1) + return false; // More than one statement, can't be known to always throw. + OnlyStmt = *Compound->body_begin(); + } else { + OnlyStmt = Body; + } + + // Unwrap ExprWithCleanups if necessary. + if (const auto *EWC = dyn_cast<ExprWithCleanups>(OnlyStmt)) { + OnlyStmt = EWC->getSubExpr(); + } + // Check if the only statement is a throw expression. + return isa<CXXThrowExpr>(OnlyStmt); +} + +void clang::inferNoReturnAttr(Sema &S, 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)) { + FD->addAttr(InferredNoReturnAttr::CreateImplicit(S.Context)); + } +} + static void handleNoReturnAttr(Sema &S, Decl *D, const ParsedAttr &Attrs) { if (hasDeclarator(D)) return; >From cf0818932c865713ed18120137f34ac3ca65b61f Mon Sep 17 00:00:00 2001 From: Samarth Narang <snar...@umass.edu> Date: Thu, 26 Jun 2025 23:45:52 -0400 Subject: [PATCH 7/7] For Spellings = [], don't set Documentation at all. --- clang/include/clang/Basic/Attr.td | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 5bf7e5400173a..f48a95ee09b64 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -968,7 +968,6 @@ def AnalyzerNoReturn : InheritableAttr { def InferredNoReturn : InheritableAttr { let Spellings = []; let SemaHandler = 0; - let Documentation = [Undocumented]; let Subjects = SubjectList<[Function], ErrorDiag>; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits