llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: nerix (Nerixyz) <details> <summary>Changes</summary> Declaring a coroutine `[[noreturn]]` doesn't make sense, because it will always return its handle. Clang previously crashed when trying to warn about this (diagnostic ID was 0). This PR adds a warning specific to coroutines. As explained in https://github.com/llvm/llvm-project/issues/127327#issuecomment-2661630422, Clang will show two warnings: ``` main.cpp(41,24): warning: function 'f' declared 'noreturn' should not return [-Winvalid-noreturn] 41 | [[noreturn]] coroutine f() { co_await awaitable{}; } | ^ main.cpp(41,52): warning: coroutines cannot be declared 'noreturn' as they always return a coroutine handle [-Winvalid-noreturn] 41 | [[noreturn]] coroutine f() { co_await awaitable{}; } | ^ 2 warnings generated. ``` I'm not sure if that's reasonable, or if the first warning should be omitted. Fixes #<!-- -->127327. --- Full diff: https://github.com/llvm/llvm-project/pull/127623.diff 3 Files Affected: - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3) - (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+2-2) - (added) clang/test/SemaCXX/coroutine-noreturn.cpp (+27) ``````````diff diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index f10af8f5bd6b2..9136fb83ae0b5 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10606,6 +10606,9 @@ def warn_noreturn_function_has_return_expr : Warning< def warn_falloff_noreturn_function : Warning< "function declared 'noreturn' should not return">, InGroup<InvalidNoreturn>; +def warn_noreturn_coroutine : Warning< + "coroutines cannot be declared 'noreturn' as they always return a coroutine handle">, + InGroup<InvalidNoreturn>; def err_noreturn_block_has_return_expr : Error< "block declared 'noreturn' should not return">; def err_carries_dependency_missing_on_first_decl : Error< diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 589869d018657..dc344b60384e3 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -588,10 +588,10 @@ struct CheckFallThroughDiagnostics { static CheckFallThroughDiagnostics MakeForCoroutine(const Decl *Func) { CheckFallThroughDiagnostics D; D.FuncLoc = Func->getLocation(); - D.diag_MaybeFallThrough_HasNoReturn = 0; + D.diag_MaybeFallThrough_HasNoReturn = diag::warn_noreturn_coroutine; D.diag_MaybeFallThrough_ReturnsNonVoid = diag::warn_maybe_falloff_nonvoid_coroutine; - D.diag_AlwaysFallThrough_HasNoReturn = 0; + D.diag_AlwaysFallThrough_HasNoReturn = diag::warn_noreturn_coroutine; D.diag_AlwaysFallThrough_ReturnsNonVoid = diag::warn_falloff_nonvoid_coroutine; D.diag_NeverFallThroughOrReturn = 0; diff --git a/clang/test/SemaCXX/coroutine-noreturn.cpp b/clang/test/SemaCXX/coroutine-noreturn.cpp new file mode 100644 index 0000000000000..30454ef0f37cf --- /dev/null +++ b/clang/test/SemaCXX/coroutine-noreturn.cpp @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -Winvalid-noreturn -verify + +#include "Inputs/std-coroutine.h" + +struct Promise; + +struct Awaitable { + bool await_ready(); + void await_suspend(std::coroutine_handle<>); + void await_resume(); +}; + +struct Coro : std::coroutine_handle<> { + using promise_type = Promise; +}; + +struct Promise { + Coro get_return_object(); + std::suspend_always initial_suspend() noexcept; + std::suspend_always final_suspend() noexcept; + void return_void(); + void unhandled_exception(); +}; + +[[noreturn]] Coro test() { // expected-warning {{function 'test' declared 'noreturn' should not return}} + co_await Awaitable{}; +} // expected-warning {{coroutines cannot be declared 'noreturn' as they always return a coroutine handle}} `````````` </details> https://github.com/llvm/llvm-project/pull/127623 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
