Author: Samarth Narang Date: 2025-06-27T10:52:22-04:00 New Revision: 794edd187cee49e3f12cc2ce587d60fc85e08dd6
URL: https://github.com/llvm/llvm-project/commit/794edd187cee49e3f12cc2ce587d60fc85e08dd6 DIFF: https://github.com/llvm/llvm-project/commit/794edd187cee49e3f12cc2ce587d60fc85e08dd6.diff LOG: [clang] Suppress noreturn warning if last statement in a function is a throw (#145166) Fixes https://github.com/llvm/llvm-project/issues/144952 Added: clang/test/SemaCXX/wreturn-always-throws.cpp Modified: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/Attr.td clang/include/clang/Sema/Sema.h clang/lib/Sema/AnalysisBasedWarnings.cpp clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaDeclAttr.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index b84f3c572e5ac..d9847fadc21e5 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -649,6 +649,13 @@ Improvements to Clang's diagnostics #GH69470, #GH59391, #GH58172, #GH46215, #GH45915, #GH45891, #GH44490, #GH36703, #GH32903, #GH23312, #GH69874. +- 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/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 5d8ce6b650d9c..340f439a45bb9 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 Subjects = SubjectList<[Function], ErrorDiag>; + let Documentation = [InternalOnly]; +} + 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..9afe3c7710476 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, const Decl *D); + /// 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 2a107a36e24b4..f3798bb6a7450 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -643,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(); @@ -681,6 +681,28 @@ 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 = D->getAsFunction()) { + 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 && Callee->hasAttr<InferredNoReturnAttr>()) { + return; // Don't warn about fall-through. + } + } + // Direct throw. + if (isa<CXXThrowExpr>(LastStmt)) { + 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/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index 42ebf2a508a26..dfc5a2767f579 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -2434,9 +2434,10 @@ Sema::PopFunctionScopeInfo(const AnalysisBasedWarnings::Policy *WP, OpenMP().popOpenMPFunctionRegion(Scope.get()); // Issue any analysis-based warnings. - if (WP && D) + if (WP && D) { + inferNoReturnAttr(*this, D); 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..52313e6a15ff1 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,49 @@ 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, const Decl *D) { + auto *FD = dyn_cast<FunctionDecl>(D); + if (!FD) + return; + + 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)); + } +} + static void handleNoReturnAttr(Sema &S, Decl *D, const ParsedAttr &Attrs) { if (hasDeclarator(D)) return; diff --git a/clang/test/SemaCXX/wreturn-always-throws.cpp b/clang/test/SemaCXX/wreturn-always-throws.cpp new file mode 100644 index 0000000000000..addcadd1183dc --- /dev/null +++ b/clang/test/SemaCXX/wreturn-always-throws.cpp @@ -0,0 +1,46 @@ +// 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 &); + }; +} + +// Non-template version. + +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 +} + +int alwaysThrows() { + throw std::runtime_error("This function always throws"); // no-warning +} + +// Template version. + +template<typename T> +void throwErrorTemplate(const T& msg) { + throw msg; +} + +template <typename T> +int ensureZeroTemplate(T i) { + if (i == 0) return 0; + throwErrorTemplate("ERROR"); // no-warning +} + +void testTemplates() { + throwErrorTemplate("ERROR"); + (void)ensureZeroTemplate(42); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
