Suppress noreturn warning if last statement in a function is a throw #145166

snarang181 · 2025-06-21T12:42:33Z

Fixes #144952

llvmbot · 2025-06-21T15:01:22Z

@llvm/pr-subscribers-clang

Author: Samarth Narang (snarang181)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/145166.diff

2 Files Affected:

(modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+45)
(added) clang/test/SemaCXX/wreturn-always-throws.cpp (+22)

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
+}

Copilot

Pull Request Overview

This PR adds support to suppress “fall-through” return-type warnings when a function’s last statement always throws, along with a verifying test.

Introduce isKnownToAlwaysThrow helper to detect single-statement throw functions
Update CheckFallThroughForBody to skip warnings for calls to those functions
Add a new test (wreturn-always-throws.cpp) to verify suppression

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
clang/test/SemaCXX/wreturn-always-throws.cpp	New test covering a non-void function that ends in a throw
clang/lib/Sema/AnalysisBasedWarnings.cpp	Helper and logic to suppress return-type warnings on throwers

clang/lib/Sema/AnalysisBasedWarnings.cpp

Copilot · 2025-06-21T15:03:37Z

clang/lib/Sema/AnalysisBasedWarnings.cpp

+            if (const auto *EWC = dyn_cast<ExprWithCleanups>(LastStmt)) {
+              LastStmt = EWC->getSubExpr();
+            }


[nitpick] The unwrap of ExprWithCleanups is duplicated in two places. Consider extracting a helper function to reduce duplication and centralize cleanup unwrapping logic.

Suggested change

if (const auto *EWC = dyn_cast<ExprWithCleanups>(LastStmt)) {

LastStmt = EWC->getSubExpr();

}

LastStmt = unwrapExprWithCleanups(LastStmt);

clang/lib/Sema/AnalysisBasedWarnings.cpp

github-actions · 2025-06-21T15:07:42Z

✅ With the latest revision this PR passed the C/C++ code formatter.

zwuis

Thank you for the patch! Please add a release note in clang/docs/ReleaseNotes.rst.

I'm not sure if this case should be handled:

if (condition) {
  throw 1;
} else {
  throw 2;
}

Maybe we need to open an issue for it.

clang/lib/Sema/AnalysisBasedWarnings.cpp

snarang181 · 2025-06-21T16:36:53Z

Thank you for the patch! Please add a release note in clang/docs/ReleaseNotes.rst.

@zwuis, added a description of the patch in the release notes and addressed your review comments.

philnik777 · 2025-06-23T08:10:42Z

I feel like this is a solution that can grow out of hand very easily. IMO the better solution would be to have -Wmissing-noreturn place a [[noreturn]] into the AST to avoid incorrect follow-up diagnostics.

TiborGY · 2025-06-23T15:24:01Z

I feel like this is a solution that can grow out of hand very easily. IMO the better solution would be to have -Wmissing-noreturn place a [[noreturn]] into the AST to avoid incorrect follow-up diagnostics.

I agree, in principle I would expect Clang to be able to prove (in simple cases) that a function cannot return, and then treat it as-if they were marked as [[noreturn]] in the source code, not just for purposes of diagnostics but also optimization. Suppressing the -Wreturn-type warning does not achieve that.

erichkeane

This seems reasonable to me, though I'd like to give other reviewers a chance. I think we need to be less optimistic in the release notes (clarifying that we are doing a very mild pattern match), else I think this is sensible.

clang/lib/Sema/AnalysisBasedWarnings.cpp

erichkeane

Want 1 more test and a preference on interface, else I'm pretty good here.

clang/lib/Sema/Sema.cpp

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

Add templated test case

…a throw (llvm#145166) Fixes llvm#144952

alexfh · 2025-07-14T15:07:47Z

Something went wrong here, since valid code started triggering the -Winvalid-noreturn diagnostic after this commit: https://gcc.godbolt.org/z/zvs9MG7rz

template<typename T>
struct S {
void f();
};

template<typename T>
void S<T>::f() { throw 0; }

template<>
void S<int>::f() {}

<source>:10:19: error: function declared 'noreturn' should not return [-Werror,-Winvalid-noreturn]
   10 | void S<int>::f() {}
      |                   ^

Please fix or revert soon!

alexfh · 2025-07-15T10:40:29Z

A general comment about #144952 and this PR: any cross-function analysis like this one will only work on a translation unit level, which has obvious shortcomings (placing the implicitly noreturn function and its caller in different translation units will return the false positive diagnostic). I think, it may be better to explicitly mark functions [[noreturn]] in the code rather than relying on the compiler to derive this property.

alexfh · 2025-07-15T22:23:34Z

@snarang181 friendly ping!

snarang181 · 2025-07-15T23:35:35Z

@snarang181 friendly ping!

Hi! I haven't been well and afk, I'll try to get to this as soon as possible. Thank you all for your comments.

TiborGY · 2025-07-15T23:39:32Z

Just out of curiosity, could implementing this warning to be LTO only solve this? Anything requiring whole-program analysis sounds like a task to be done during LTO. IIRC GCC has some warnings that are only made possible by LTO.

…

On Tue, Jul 15, 2025 at 12:40 PM Alexander Kornienko < ***@***.***> wrote: *alexfh* left a comment (llvm/llvm-project#145166) <#145166 (comment)> A general comment about #144952 <#144952> and this PR: any cross-function analysis like this one will only work on a translation unit level, which has obvious shortcomings (placing the implicitly noreturn function and its caller in different translation units will return the false positive diagnostic). I think, it may be better to explicitly mark functions [[noreturn]] in the code rather than relying on the compiler to derive this property. — Reply to this email directly, view it on GitHub <#145166 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHD2KHQTI6E6DZDNDTW2K433ITLDJAVCNFSM6AAAAAB72CXXLCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTANZTGEYDGNRWGI> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

alexfh · 2025-07-16T01:00:36Z

Just out of curiosity, could implementing this warning to be LTO only solve this? Anything requiring whole-program analysis sounds like a task to be done during LTO. IIRC GCC has some warnings that are only made possible by LTO.

I don't think this would be the right direction. First of all, LTO is inherently a more expensive build mode, which (in my surroundings at least) is mostly used for releases, but not in the normal development cycle. Making this diagnostic LTO-only will shift the moment when the corresponding issues are found from development to release build, breaking the latter without showing up earlier. If the release builds are performed automatically on a certain schedule, this will be a very unfortunate moment for the first detection of these problems.

I personally think that relying on explicit [[noreturn]] attributes and the -Wmissing-noreturn diagnostic to suggest annotating functions that never return should be a more robust approach with less undesired surprises.

alexfh · 2025-07-16T01:29:09Z

@snarang181 friendly ping!

Hi! I haven't been well and afk, I'll try to get to this as soon as possible. Thank you all for your comments.

Oh, sad to hear that. Get well soon!

alexfh · 2025-07-21T13:20:25Z

@snarang181 how soon will you be able to address this? This diagnostic keeps being broken at HEAD, and we need to decide whether this change should be reverted until you have time to get back to this. I'll prepare a revert for now and will proceed with it, if I don't get a reply soon.

snarang181 · 2025-07-21T17:18:54Z

@snarang181 how soon will you be able to address this? This diagnostic keeps being broken at HEAD, and we need to decide whether this change should be reverted until you have time to get back to this. I'll prepare a revert for now and will proceed with it, if I don't get a reply soon.

@alexfh, I can start looking at this now. Is the proposed plan of action marking the functions [[noreturn]] ? As per #145166 (comment) , marking the functions ``[[noreturn]]` was discouraged.

alexfh · 2025-07-22T01:04:04Z

Is the proposed plan of action marking the functions [[noreturn]] ? As per #145166 (comment) , marking the functions ``[[noreturn]]` was discouraged.

There are two problems here:

the problem demonstrated by the example in https://gcc.godbolt.org/z/bv1ac475d, i.e. that the InferredNoReturnAttr is attached to the explicit specialization of a function that does actually return (I think, it's not controversial that this is an incorrect behavior and should be fixed).
the general approach with deriving an implicit noreturn attribute from a function definition - this is debatable, and @erichkeane seems to agree with your approach, but I believe cross-function diagnostics in general have a potential for inconsistent behavior, e.g. when moving functions across translation units. I'd prefer to rely on explicitly stated noreturn annotations rather than deriving them from function definitions. IMO [[noreturn]] is a part of the function API, since it affects how callers can use it.

I'd like @erichkeane and @AaronBallman to chime in on the second part - maybe this should be reconsidered.

IMO, the motivating issue for this PR should be solved in the user code rather than in the compiler (#144952 (comment)).

Now, getting back to the issue # 1: if you have a trivial fix for this, please go ahead and implement it (assuming the decision on # 2 is to leave this feature in Clang). If the fix is not obvious, I'd suggest to revert the commit and re-send it for review together with the fix.

snarang181 · 2025-07-22T13:16:09Z

Now, getting back to the issue # 1: if you have a trivial fix for this, please go ahead and implement it (assuming the decision on # 2 is to leave this feature in Clang). If the fix is not obvious, I'd suggest to revert the commit and re-send it for review together with the fix.

@alexfh, I have #150003 up as a patch for the problem you are seeing.

…pecializations (#150003) This patch fixes incorrect behavior in Clang where [[noreturn]] (either spelled or inferred) was being inherited by explicit specializations of function templates or member function templates, even when those specializations returned normally. Follow up on #145166

… template specializations (#150003) This patch fixes incorrect behavior in Clang where [[noreturn]] (either spelled or inferred) was being inherited by explicit specializations of function templates or member function templates, even when those specializations returned normally. Follow up on llvm/llvm-project#145166

…pecializations (#150003) This patch fixes incorrect behavior in Clang where [[noreturn]] (either spelled or inferred) was being inherited by explicit specializations of function templates or member function templates, even when those specializations returned normally. Follow up on llvm/llvm-project#145166

…pecializations (llvm#150003) This patch fixes incorrect behavior in Clang where [[noreturn]] (either spelled or inferred) was being inherited by explicit specializations of function templates or member function templates, even when those specializations returned normally. Follow up on llvm#145166

…pecializations (llvm#150003) This patch fixes incorrect behavior in Clang where [[noreturn]] (either spelled or inferred) was being inherited by explicit specializations of function templates or member function templates, even when those specializations returned normally. Follow up on llvm#145166 (cherry picked from commit 22fef00)

… template specializations (#150003) This patch fixes incorrect behavior in Clang where [[noreturn]] (either spelled or inferred) was being inherited by explicit specializations of function templates or member function templates, even when those specializations returned normally. Follow up on llvm/llvm-project#145166 (cherry picked from commit 22fef00)

snarang181 force-pushed the clang-no-return-fix branch 8 times, most recently from 2c2324a to 359dfb1 Compare June 21, 2025 14:50

snarang181 marked this pull request as ready for review June 21, 2025 15:00

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 21, 2025

snarang181 mentioned this pull request Jun 21, 2025

False positive Wreturn-type warning when a function either returns or throws #144952

Closed

snarang181 requested review from kadircet, Copilot and erichkeane and removed request for Copilot June 21, 2025 15:02

Copilot AI reviewed Jun 21, 2025

View reviewed changes

snarang181 force-pushed the clang-no-return-fix branch from 1009d62 to b148279 Compare June 21, 2025 15:09

snarang181 self-assigned this Jun 21, 2025

snarang181 force-pushed the clang-no-return-fix branch from 5e2fc5f to 4340fec Compare June 21, 2025 15:31

zwuis reviewed Jun 21, 2025

View reviewed changes

clang/lib/Sema/AnalysisBasedWarnings.cpp Outdated Show resolved Hide resolved

clang/lib/Sema/AnalysisBasedWarnings.cpp Outdated Show resolved Hide resolved

clang/lib/Sema/AnalysisBasedWarnings.cpp Outdated Show resolved Hide resolved

zwuis requested a review from shafik June 21, 2025 16:18

snarang181 requested a review from zwuis June 21, 2025 16:36

erichkeane reviewed Jun 24, 2025

View reviewed changes

clang/lib/Sema/AnalysisBasedWarnings.cpp Outdated Show resolved Hide resolved

clang/lib/Sema/AnalysisBasedWarnings.cpp Outdated Show resolved Hide resolved

snarang181 force-pushed the clang-no-return-fix branch from df23c9a to cdb0568 Compare June 24, 2025 23:16

snarang181 force-pushed the clang-no-return-fix branch from 3fd24e6 to 8cca1b0 Compare June 27, 2025 03:35

For Spellings = [], don't set Documentation at all.

cf08189

snarang181 marked this pull request as draft June 27, 2025 09:40

erichkeane reviewed Jun 27, 2025

View reviewed changes

clang/lib/Sema/Sema.cpp Outdated Show resolved Hide resolved

clang/test/SemaCXX/wreturn-always-throws.cpp Show resolved Hide resolved

snarang181 added 2 commits June 27, 2025 09:58

Fix tablegen

81773c5

Minor changes to function definition

28c9745

Add templated test case

snarang181 requested a review from erichkeane June 27, 2025 14:19

snarang181 marked this pull request as ready for review June 27, 2025 14:19

erichkeane approved these changes Jun 27, 2025

View reviewed changes

snarang181 merged commit 794edd1 into llvm:main Jun 27, 2025
10 checks passed

snarang181 deleted the clang-no-return-fix branch June 27, 2025 14:52

rlavaee pushed a commit to rlavaee/llvm-project that referenced this pull request Jul 1, 2025

[clang] Suppress noreturn warning if last statement in a function is …

951ff85

…a throw (llvm#145166) Fixes llvm#144952

snarang181 mentioned this pull request Jul 22, 2025

[clang] Avoid inheriting [[noreturn]] in explicit function template specializations #150003

Merged

Suppress noreturn warning if last statement in a function is a throw #145166

Suppress noreturn warning if last statement in a function is a throw #145166

Uh oh!

Conversation

snarang181 commented Jun 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 21, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Jun 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Jun 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zwuis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snarang181 commented Jun 21, 2025

Uh oh!

philnik777 commented Jun 23, 2025

Uh oh!

TiborGY commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexfh commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexfh commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexfh commented Jul 15, 2025

Uh oh!

snarang181 commented Jul 15, 2025

Uh oh!

TiborGY commented Jul 15, 2025 via email

Uh oh!

alexfh commented Jul 16, 2025

Uh oh!

alexfh commented Jul 16, 2025

Uh oh!

alexfh commented Jul 21, 2025

Uh oh!

snarang181 commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexfh commented Jul 22, 2025

Uh oh!

snarang181 commented Jul 22, 2025

Uh oh!

Uh oh!

snarang181 commented Jun 21, 2025 •

edited

Loading

github-actions bot commented Jun 21, 2025 •

edited

Loading

TiborGY commented Jun 23, 2025 •

edited

Loading

alexfh commented Jul 14, 2025 •

edited

Loading

alexfh commented Jul 15, 2025 •

edited

Loading

snarang181 commented Jul 21, 2025 •

edited

Loading