Skip to content

Conversation

@snarang181
Copy link
Contributor

@snarang181 snarang181 commented Jun 28, 2025

Implements #146223.

@snarang181 snarang181 force-pushed the clang-emit-noreturn-suggestion branch from 6f450c2 to c6b4a42 Compare June 28, 2025 19:34
@snarang181 snarang181 marked this pull request as ready for review June 28, 2025 19:50
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 28, 2025

@llvm/pr-subscribers-clang

Author: Samarth Narang (snarang181)

Changes

Implements #146223.


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1-2)
  • (modified) clang/lib/Sema/SemaDeclAttr.cpp (+7)
  • (added) clang/test/SemaCXX/wmissing-noreturn-suggestion.cpp (+23)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d9847fadc21e5..73a2618bb580c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -654,9 +654,8 @@ Improvements to Clang's diagnostics
   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.
+  are currently recognized. Additionally, if -Wmissing-noreturn is enabled, emit a suggestion to explicitly mark the function with [[noreturn]].
 
-  
 Improvements to Clang's time-trace
 ----------------------------------
 
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 52313e6a15ff1..e7da8e673a9ec 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -1979,6 +1979,13 @@ void clang::inferNoReturnAttr(Sema &S, const Decl *D) {
   if (!FD->hasAttr<NoReturnAttr>() && !FD->hasAttr<InferredNoReturnAttr>() &&
       isKnownToAlwaysThrow(FD)) {
     NonConstFD->addAttr(InferredNoReturnAttr::CreateImplicit(S.Context));
+
+    // Conditionally, emit the suggestion warning.
+    if (!Diags.isIgnored(diag::warn_suggest_noreturn_function,
+                         FD->getLocation())) {
+      S.Diag(FD->getLocation(), diag::warn_suggest_noreturn_function)
+          << 0 << FD;
+    }
   }
 }
 
diff --git a/clang/test/SemaCXX/wmissing-noreturn-suggestion.cpp b/clang/test/SemaCXX/wmissing-noreturn-suggestion.cpp
new file mode 100644
index 0000000000000..7548ba8904a71
--- /dev/null
+++ b/clang/test/SemaCXX/wmissing-noreturn-suggestion.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -fexceptions -Wreturn-type -Wmissing-noreturn -verify %s
+
+namespace std {
+  class string {
+  public:
+    string(const char*);
+  };
+  class runtime_error {
+  public:
+    runtime_error(const string&);
+  };
+}
+
+// This function always throws. Suggest [[noreturn]].
+void throwError(const std::string& msg) { // expected-warning {{function 'throwError' could be declared with attribute 'noreturn'}}
+  throw std::runtime_error(msg);
+}
+
+// The non-void caller should not warn about missing return.
+int ensureZero(int i) {
+  if (i == 0) return 0;
+  throwError("ERROR"); // no-warning
+}

@snarang181 snarang181 force-pushed the clang-emit-noreturn-suggestion branch from c6b4a42 to 1c9aa48 Compare June 29, 2025 15:16
@snarang181 snarang181 force-pushed the clang-emit-noreturn-suggestion branch from 1c9aa48 to 8a0e8f8 Compare June 29, 2025 15:18
Copy link
Contributor

@hstk30-hw hstk30-hw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 nit, else this is ok. please make sure the comment gets added, but then feel free to merge.

@snarang181
Copy link
Contributor Author

The linux test failure seems unrelated to these changes.

@snarang181 snarang181 merged commit 1cdc7f8 into llvm:main Jun 30, 2025
6 of 7 checks passed
@cor3ntin
Copy link
Contributor

This fails for templates https://compiler-explorer.com/z/x6a8G13Ms
Presumably because of #37650

@snarang181 @erichkeane

@horenmar
Copy link

horenmar commented Jul 13, 2025

It is not just templates: https://compiler-explorer.com/z/jWvzzPz86

I just got warning on this code

namespace {
    [[noreturn]]
    void this_throws() {
        throw std::runtime_error("Some msg");
    }
    void this_doesnt_throw() {}
}
[92/468] Building CXX object tests/ExtraTests/CMakeFiles/PrefixedMacros.dir/X01-PrefixedMacros.cpp.o
/mnt/c/ubuntu/Catch2/tests/ExtraTests/X01-PrefixedMacros.cpp:26:10: warning: function 'this_throws' could be declared with attribute 'noreturn' [-Wmissing-noreturn]
   26 |     void this_throws() {
      |          ^
1 warning generated.

Clang version:

$ clang++ --version
Ubuntu clang version 21.0.0 (++20250708042308+dcf485609c5c-1~exp1~20250708162410.2527)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/lib/llvm-21/bin

cor3ntin added a commit to cor3ntin/llvm-project that referenced this pull request Jul 13, 2025
Fix a false positve warning which was introduced by llvm#146234.
@cor3ntin
Copy link
Contributor

I opened #148552

cor3ntin added a commit that referenced this pull request Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants