https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/71945
>From 89a2d60fc012742a74a008fb77213bcd47734503 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Fri, 10 Nov 2023 15:10:44 +0100 Subject: [PATCH 01/13] [coroutines] Introduce [[clang::coro_return_type]] and [[clang::co ro_wrapper]] --- clang/include/clang/Basic/Attr.td | 16 +++++ clang/include/clang/Basic/AttrDocs.td | 67 +++++++++++++++++++ .../clang/Basic/DiagnosticSemaKinds.td | 4 ++ clang/include/clang/Sema/Sema.h | 1 + clang/lib/Sema/SemaDecl.cpp | 25 ++++++- ...a-attribute-supported-attributes-list.test | 2 + .../SemaCXX/coro-return-type-and-wrapper.cpp | 56 ++++++++++++++++ 7 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 clang/test/SemaCXX/coro-return-type-and-wrapper.cpp diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 31434565becaec6..f7a2b83b15ef5bc 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1094,6 +1094,22 @@ def CoroOnlyDestroyWhenComplete : InheritableAttr { let SimpleHandler = 1; } +def CoroReturnType : InheritableAttr { + let Spellings = [Clang<"coro_return_type">]; + let Subjects = SubjectList<[CXXRecord]>; + let LangOpts = [CPlusPlus]; + let Documentation = [CoroReturnTypeAndWrapperDoc]; + let SimpleHandler = 1; +} + +def CoroWrapper : InheritableAttr { + let Spellings = [Clang<"coro_wrapper">]; + let Subjects = SubjectList<[Function]>; + let LangOpts = [CPlusPlus]; + let Documentation = [CoroReturnTypeAndWrapperDoc]; + let SimpleHandler = 1; +} + // OSObject-based attributes. def OSConsumed : InheritableParamAttr { let Spellings = [Clang<"os_consumed">]; diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index fa6f6acd0c30e88..66c92bcaa2d4a4a 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7482,3 +7482,70 @@ generation of the other destruction cases, optimizing the above `foo.destroy` to }]; } + + +def CoroReturnTypeAndWrapperDoc : Documentation { + let Category = DocCatDecl; + let Content = [{ +The `coro_only_destroy_when_complete` attribute should be marked on a C++ class. The coroutines +whose return type is marked with the attribute are assumed to be destroyed only after the coroutine has +reached the final suspend point. + +This is helpful for the optimizers to reduce the size of the destroy function for the coroutines. + +For example, + +.. code-block:: c++ + + A foo() { + dtor d; + co_await something(); + dtor d1; + co_await something(); + dtor d2; + co_return 43; + } + +The compiler may generate the following pseudocode: + +.. code-block:: c++ + + void foo.destroy(foo.Frame *frame) { + switch(frame->suspend_index()) { + case 1: + frame->d.~dtor(); + break; + case 2: + frame->d.~dtor(); + frame->d1.~dtor(); + break; + case 3: + frame->d.~dtor(); + frame->d1.~dtor(); + frame->d2.~dtor(); + break; + default: // coroutine completed or haven't started + break; + } + + frame->promise.~promise_type(); + delete frame; + } + +The `foo.destroy()` function's purpose is to release all of the resources +initialized for the coroutine when it is destroyed in a suspended state. +However, if the coroutine is only ever destroyed at the final suspend state, +the rest of the conditions are superfluous. + +The user can use the `coro_only_destroy_when_complete` attributo suppress +generation of the other destruction cases, optimizing the above `foo.destroy` to: + +.. code-block:: c++ + + void foo.destroy(foo.Frame *frame) { + frame->promise.~promise_type(); + delete frame; + } + + }]; +} diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 4614324babb1c91..0200457b39ce5eb 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11591,6 +11591,10 @@ def err_conflicting_aligned_options : Error < def err_coro_invalid_addr_of_label : Error< "the GNU address of label extension is not allowed in coroutines." >; +def err_coroutine_return_type : Error< + "function returns a coroutine return type %0 but is neither a coroutine nor a coroutine wrapper; " + "non-coroutines should be marked with [[clang::coro_wrapper]] to allow returning coroutine return type" +>; } // end of coroutines issue category let CategoryName = "Documentation Issue" in { diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index f69f366c1750918..4d45698e5786740 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -11183,6 +11183,7 @@ class Sema final { bool buildCoroutineParameterMoves(SourceLocation Loc); VarDecl *buildCoroutinePromise(SourceLocation Loc); void CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body); + void CheckCoroutineWrapper(FunctionDecl *FD); /// Lookup 'coroutine_traits' in std namespace and std::experimental /// namespace. The namespace found is recorded in Namespace. ClassTemplateDecl *lookupCoroutineTraits(SourceLocation KwLoc, diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 3876eb501083acb..3e0674a2c79466d 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -17,6 +17,7 @@ #include "clang/AST/CXXInheritance.h" #include "clang/AST/CharUnits.h" #include "clang/AST/CommentDiagnostic.h" +#include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/DeclTemplate.h" @@ -15811,6 +15812,23 @@ static void diagnoseImplicitlyRetainedSelf(Sema &S) { << FixItHint::CreateInsertion(P.first, "self->"); } +void Sema::CheckCoroutineWrapper(FunctionDecl *FD) { + if (!getLangOpts().Coroutines || !FD || getCurFunction()->isCoroutine()) + return; + RecordDecl *RD = FD->getReturnType()->getAsRecordDecl(); + if (!RD || !RD->getUnderlyingDecl()->hasAttr<CoroReturnTypeAttr>()) + return; + // Allow `promise_type::get_return_object`. + if (FD->getName() == "get_return_object") { + if (auto *GRT = dyn_cast<CXXMethodDecl>(FD)) { + if (auto *PT = GRT->getParent(); PT && PT->getName() == "promise_type") + return; + } + } + if (!FD->hasAttr<CoroWrapperAttr>()) + Diag(FD->getLocation(), diag::err_coroutine_return_type) << RD; +} + Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body, bool IsInstantiation) { FunctionScopeInfo *FSI = getCurFunction(); @@ -15822,8 +15840,11 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body, sema::AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy(); sema::AnalysisBasedWarnings::Policy *ActivePolicy = nullptr; - if (getLangOpts().Coroutines && FSI->isCoroutine()) - CheckCompletedCoroutineBody(FD, Body); + if (getLangOpts().Coroutines) { + if (FSI->isCoroutine()) + CheckCompletedCoroutineBody(FD, Body); + CheckCoroutineWrapper(FD); + } { // Do not call PopExpressionEvaluationContext() if it is a lambda because diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test index 969794073a5f2e8..a57bc011c059483 100644 --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test @@ -57,6 +57,8 @@ // CHECK-NEXT: ConsumableSetOnRead (SubjectMatchRule_record) // CHECK-NEXT: Convergent (SubjectMatchRule_function) // CHECK-NEXT: CoroOnlyDestroyWhenComplete (SubjectMatchRule_record) +// CHECK-NEXT: CoroReturnType (SubjectMatchRule_record) +// CHECK-NEXT: CoroWrapper // CHECK-NEXT: CountedBy (SubjectMatchRule_field) // CHECK-NEXT: DLLExport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface) // CHECK-NEXT: DLLImport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface) diff --git a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp new file mode 100644 index 000000000000000..a54bbdcbc335279 --- /dev/null +++ b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp @@ -0,0 +1,56 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++20 -fsyntax-only -verify -Wall -Wextra +#include "Inputs/std-coroutine.h" + +using std::suspend_always; +using std::suspend_never; + + +template <typename T> struct [[clang::coro_return_type]] Gen { + struct promise_type { + Gen<T> get_return_object() { + return {}; + } + suspend_always initial_suspend(); + suspend_always final_suspend() noexcept; + void unhandled_exception(); + void return_value(const T &t); + + template <typename U> + auto await_transform(const Gen<U> &) { + struct awaitable { + bool await_ready() noexcept { return false; } + void await_suspend(std::coroutine_handle<>) noexcept {} + U await_resume() noexcept { return {}; } + }; + return awaitable{}; + } + }; +}; + +Gen<int> foo_coro(int b); +Gen<int> foo_coro(int b) { co_return b; } + +[[clang::coro_wrapper]] Gen<int> marked_wrapper1(int b) { return foo_coro(b); } + +// expected-error@+1 {{neither a coroutine nor a coroutine wrapper}} +Gen<int> non_marked_wrapper(int b) { return foo_coro(b); } + +namespace using_decl { +template <typename T> using Co = Gen<T>; + +[[clang::coro_wrapper]] Co<int> marked_wrapper1(int b) { return foo_coro(b); } + +// expected-error@+1 {{neither a coroutine nor a coroutine wrapper}} +Co<int> non_marked_wrapper(int b) { return foo_coro(b); } +} // namespace using_decl + +namespace lambdas { +void foo() { + auto coro_lambda = []() -> Gen<int> { + co_return 1; + }; + auto wrapper_lambda = [&]() -> Gen<int> { + return coro_lambda(); + } +} +} \ No newline at end of file >From 78bce9049cf259e4cb5da11f0fbe1952111638da Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Fri, 10 Nov 2023 16:24:35 +0100 Subject: [PATCH 02/13] fix promise_type::get_return_object --- clang/lib/Sema/SemaDecl.cpp | 22 ++++++--- .../SemaCXX/coro-return-type-and-wrapper.cpp | 47 +++++++++++++++++-- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 3e0674a2c79466d..396a92d908fb513 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -27,6 +27,7 @@ #include "clang/AST/NonTrivialTypeVisitor.h" #include "clang/AST/Randstruct.h" #include "clang/AST/StmtCXX.h" +#include "clang/AST/Type.h" #include "clang/Basic/Builtins.h" #include "clang/Basic/HLSLRuntime.h" #include "clang/Basic/PartialDiagnostic.h" @@ -15812,6 +15813,19 @@ static void diagnoseImplicitlyRetainedSelf(Sema &S) { << FixItHint::CreateInsertion(P.first, "self->"); } +// Return whether FD is `promise_type::get_return_object`. +bool isGetReturnObject(FunctionDecl *FD) { + if (!FD->getDeclName().isIdentifier() || + !FD->getName().equals("get_return_object") || !FD->param_empty()) + return false; + CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD); + if (!MD || !MD->isCXXInstanceMember()) + return false; + RecordDecl *PromiseType = MD->getParent(); + return PromiseType && PromiseType->getDeclName().isIdentifier() && + PromiseType->getName().equals("promise_type"); +} + void Sema::CheckCoroutineWrapper(FunctionDecl *FD) { if (!getLangOpts().Coroutines || !FD || getCurFunction()->isCoroutine()) return; @@ -15819,12 +15833,8 @@ void Sema::CheckCoroutineWrapper(FunctionDecl *FD) { if (!RD || !RD->getUnderlyingDecl()->hasAttr<CoroReturnTypeAttr>()) return; // Allow `promise_type::get_return_object`. - if (FD->getName() == "get_return_object") { - if (auto *GRT = dyn_cast<CXXMethodDecl>(FD)) { - if (auto *PT = GRT->getParent(); PT && PT->getName() == "promise_type") - return; - } - } + if (isGetReturnObject(FD)) + return; if (!FD->hasAttr<CoroWrapperAttr>()) Diag(FD->getLocation(), diag::err_coroutine_return_type) << RD; } diff --git a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp index a54bbdcbc335279..05d52233a91da8d 100644 --- a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp +++ b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp @@ -49,8 +49,49 @@ void foo() { auto coro_lambda = []() -> Gen<int> { co_return 1; }; - auto wrapper_lambda = [&]() -> Gen<int> { - return coro_lambda(); - } + // expected-error@+1 {{neither a coroutine nor a coroutine wrapper}} + auto wrapper_lambda = []() -> Gen<int> { + return foo_coro(1); + }; } +} + +namespace std { +template <typename> class function; + +template <typename ReturnValue, typename... Args> +class function<ReturnValue(Args...)> { +public: + template <typename T> function &operator=(T) {} + template <typename T> function(T) {} + // expected-error@+1 {{neither a coroutine nor a coroutine wrapper}} + ReturnValue operator()(Args... args) const { + return callable_->Invoke(args...); // expected-note {{in instantiation of member}} + } + +private: + class Callable { + public: + // expected-error@+1 {{neither a coroutine nor a coroutine wrapper}} + ReturnValue Invoke(Args...) const { return {}; } + }; + Callable* callable_; +}; +} // namespace std + +void use_std_function() { + std::function<int(bool)> foo = [](bool b) { return b ? 1 : 2; }; + // expected-error@+1 {{neither a coroutine nor a coroutine wrapper}} + std::function<Gen<int>(bool)> test1 = [](bool b) { + return foo_coro(b); + }; + std::function<Gen<int>(bool)> test2 = [](bool) -> Gen<int> { + co_return 1; + }; + std::function<Gen<int>(bool)> test3 = foo_coro; + + foo(true); // Fine. + test1(true); // expected-note 2 {{in instantiation of member}} + test2(true); + test3(true); } \ No newline at end of file >From fce73d72548058cde1909ea43b488d98d7046181 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Mon, 13 Nov 2023 10:28:30 +0100 Subject: [PATCH 03/13] add AttrDocs.td --- clang/include/clang/Basic/AttrDocs.td | 83 +++++++++---------- .../SemaCXX/coro-return-type-and-wrapper.cpp | 4 +- 2 files changed, 43 insertions(+), 44 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 66c92bcaa2d4a4a..ffcfa43b4869e70 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7487,65 +7487,62 @@ generation of the other destruction cases, optimizing the above `foo.destroy` to def CoroReturnTypeAndWrapperDoc : Documentation { let Category = DocCatDecl; let Content = [{ -The `coro_only_destroy_when_complete` attribute should be marked on a C++ class. The coroutines -whose return type is marked with the attribute are assumed to be destroyed only after the coroutine has -reached the final suspend point. +The ``coro_return_type`` attribute should be marked on a C++ class to mark it as +a **coroutine return type (CRT)**. -This is helpful for the optimizers to reduce the size of the destroy function for the coroutines. +A function ``R func(P1, .., PN)`` has a coroutine return type (CRT) ``R`` if ``R`` +is marked by ``[[clang::coro_return_type]]`` and ``R`` has a promise type associated to it +(i.e., std::coroutine_traits<R, P1, .., PN>::promise_type is a valid promise type). -For example, +If the return type of a function is a ``CRT`` then the function must be a coroutine. +Otherwise it is invalid. It is allowed for a non-coroutine to return a ``CRT`` +if the function is marked with ``[[clang::coro_wrapper]]``. -.. code-block:: c++ +The ``coro_wrapper`` attribute should be marked on a C++ function to mark it as +a **coroutine wrapper**. A coroutine wrapper is a function which returns a ``CRT``, +is not a coroutine itself and is marked with ``[[clang::coro_wrapper]]``. - A foo() { - dtor d; - co_await something(); - dtor d1; - co_await something(); - dtor d2; - co_return 43; - } +From a language perspective, it is not possible to differentiate between a coroutine and a +function returning a CRT by merely looking at the function signature. +These two annotations allows implementations to enforce that only coroutines and explicitly marked +coroutine wrappers are allowed to return a ``CRT``. -The compiler may generate the following pseudocode: +For example, .. code-block:: c++ - void foo.destroy(foo.Frame *frame) { - switch(frame->suspend_index()) { - case 1: - frame->d.~dtor(); - break; - case 2: - frame->d.~dtor(); - frame->d1.~dtor(); - break; - case 3: - frame->d.~dtor(); - frame->d1.~dtor(); - frame->d2.~dtor(); - break; - default: // coroutine completed or haven't started - break; - } + // This is a CRT. + template <typename T> struct [[clang::coro_return_type]] Task { + using promise_type = some_promise_type; + }; - frame->promise.~promise_type(); - delete frame; + Task<int> increment(int a) { co_return a + 1; } // Fine. This is a coroutine. + Task<int> foo() { return increment(1); } // Error. foo is not a coroutine. + + // Fine for a coroutine wrapper to return a CRT. + Task<int> [[clang::coro_wrapper]] foo() { return increment(1); } + + void bar() { + // Invalid. This intantiates a function which returns a CRT but is not marked as + // a coroutine wrapper. + std::function<Task<int>(int)> f = increment; } -The `foo.destroy()` function's purpose is to release all of the resources -initialized for the coroutine when it is destroyed in a suspended state. -However, if the coroutine is only ever destroyed at the final suspend state, -the rest of the conditions are superfluous. -The user can use the `coro_only_destroy_when_complete` attributo suppress -generation of the other destruction cases, optimizing the above `foo.destroy` to: +For example, .. code-block:: c++ - void foo.destroy(foo.Frame *frame) { - frame->promise.~promise_type(); - delete frame; + A foo() { + dtor d; + co_await something(); + dtor d1; + co_await something(); + dtor d2; + co_return 43; } +The compiler may generate the following pseudocode: + }]; } diff --git a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp index 05d52233a91da8d..e9ec0929a945968 100644 --- a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp +++ b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp @@ -56,6 +56,7 @@ void foo() { } } +namespace std_function { namespace std { template <typename> class function; @@ -94,4 +95,5 @@ void use_std_function() { test1(true); // expected-note 2 {{in instantiation of member}} test2(true); test3(true); -} \ No newline at end of file +} +} // namespace std_function >From 03acdd2453f31e35b9c24435fc18323383dfbe00 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Mon, 13 Nov 2023 10:38:05 +0100 Subject: [PATCH 04/13] trim AttrDocs.td --- clang/include/clang/Basic/AttrDocs.td | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index ffcfa43b4869e70..398d8a84fecd24a 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7527,22 +7527,6 @@ For example, // a coroutine wrapper. std::function<Task<int>(int)> f = increment; } - - -For example, - -.. code-block:: c++ - - A foo() { - dtor d; - co_await something(); - dtor d1; - co_await something(); - dtor d2; - co_return 43; - } - -The compiler may generate the following pseudocode: - - }]; + +}]; } >From 643bea13db75b37c619f4d879b754836174e606b Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Mon, 13 Nov 2023 12:28:30 +0100 Subject: [PATCH 05/13] remove trailing whitespace --- clang/include/clang/Basic/AttrDocs.td | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 398d8a84fecd24a..824bdb0eba886b0 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7488,7 +7488,7 @@ def CoroReturnTypeAndWrapperDoc : Documentation { let Category = DocCatDecl; let Content = [{ The ``coro_return_type`` attribute should be marked on a C++ class to mark it as -a **coroutine return type (CRT)**. +a **coroutine return type (CRT)**. A function ``R func(P1, .., PN)`` has a coroutine return type (CRT) ``R`` if ``R`` is marked by ``[[clang::coro_return_type]]`` and ``R`` has a promise type associated to it @@ -7498,8 +7498,8 @@ If the return type of a function is a ``CRT`` then the function must be a corout Otherwise it is invalid. It is allowed for a non-coroutine to return a ``CRT`` if the function is marked with ``[[clang::coro_wrapper]]``. -The ``coro_wrapper`` attribute should be marked on a C++ function to mark it as -a **coroutine wrapper**. A coroutine wrapper is a function which returns a ``CRT``, +The ``coro_wrapper`` attribute should be marked on a C++ function to mark it as +a **coroutine wrapper**. A coroutine wrapper is a function which returns a ``CRT``, is not a coroutine itself and is marked with ``[[clang::coro_wrapper]]``. From a language perspective, it is not possible to differentiate between a coroutine and a @@ -7527,6 +7527,6 @@ For example, // a coroutine wrapper. std::function<Task<int>(int)> f = increment; } - + }]; } >From 5c73847e4195c5cf2c8e2a22b7ee821cfb4b8754 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Wed, 15 Nov 2023 16:08:54 +0100 Subject: [PATCH 06/13] release notes and address comments --- clang/docs/ReleaseNotes.rst | 5 +++++ .../clang/Basic/DiagnosticSemaKinds.td | 2 +- clang/include/clang/Sema/Sema.h | 4 ++++ clang/lib/Sema/SemaDecl.cpp | 20 ++++--------------- .../SemaCXX/coro-return-type-and-wrapper.cpp | 18 +++++++++++++++++ 5 files changed, 32 insertions(+), 17 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 7a131cb520aa600..110a39d988bfbeb 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -300,6 +300,11 @@ Attribute Changes in Clang to reduce the size of the destroy functions for coroutines which are known to be destroyed after having reached the final suspend point. +- Clang now introduced ``[[clang::coro_return_type]]`` and ``[[clang::coro_wrapper]]`` + attributes. A function returning a type marked with ``[[clang::coro_return_type]]`` + should be a coroutine. A non-coroutine function marked with ``[[clang::coro_wrapper]]`` + is still allowed to return the such a type. + Improvements to Clang's diagnostics ----------------------------------- - Clang constexpr evaluator now prints template arguments when displaying diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 0200457b39ce5eb..cca5761bb374e4c 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11592,7 +11592,7 @@ def err_coro_invalid_addr_of_label : Error< "the GNU address of label extension is not allowed in coroutines." >; def err_coroutine_return_type : Error< - "function returns a coroutine return type %0 but is neither a coroutine nor a coroutine wrapper; " + "function returns a type %0 makred with [[clang::coro_return_type]] but is neither a coroutine nor a coroutine wrapper; " "non-coroutines should be marked with [[clang::coro_wrapper]] to allow returning coroutine return type" >; } // end of coroutines issue category diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 4d45698e5786740..7d029810f0ad67e 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -11183,6 +11183,10 @@ class Sema final { bool buildCoroutineParameterMoves(SourceLocation Loc); VarDecl *buildCoroutinePromise(SourceLocation Loc); void CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body); + + // As a clang extension, enforces that a function returning a type marked with + // [[clang::coro_return_type]] must be a coroutine. A function marked with + // [[clang::coro_wrapper]] are still allowed to return such a type. void CheckCoroutineWrapper(FunctionDecl *FD); /// Lookup 'coroutine_traits' in std namespace and std::experimental /// namespace. The namespace found is recorded in Namespace. diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 396a92d908fb513..8e2963a62c1ec16 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -15813,27 +15813,15 @@ static void diagnoseImplicitlyRetainedSelf(Sema &S) { << FixItHint::CreateInsertion(P.first, "self->"); } -// Return whether FD is `promise_type::get_return_object`. -bool isGetReturnObject(FunctionDecl *FD) { - if (!FD->getDeclName().isIdentifier() || - !FD->getName().equals("get_return_object") || !FD->param_empty()) - return false; - CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD); - if (!MD || !MD->isCXXInstanceMember()) - return false; - RecordDecl *PromiseType = MD->getParent(); - return PromiseType && PromiseType->getDeclName().isIdentifier() && - PromiseType->getName().equals("promise_type"); -} - void Sema::CheckCoroutineWrapper(FunctionDecl *FD) { - if (!getLangOpts().Coroutines || !FD || getCurFunction()->isCoroutine()) + if (!FD || getCurFunction()->isCoroutine()) return; RecordDecl *RD = FD->getReturnType()->getAsRecordDecl(); if (!RD || !RD->getUnderlyingDecl()->hasAttr<CoroReturnTypeAttr>()) return; - // Allow `promise_type::get_return_object`. - if (isGetReturnObject(FD)) + // Allow `get_return_object()`. + if (FD->getDeclName().isIdentifier() && + FD->getName().equals("get_return_object") && FD->param_empty()) return; if (!FD->hasAttr<CoroWrapperAttr>()) Diag(FD->getLocation(), diag::err_coroutine_return_type) << RD; diff --git a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp index e9ec0929a945968..71e28df31e56387 100644 --- a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp +++ b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp @@ -97,3 +97,21 @@ void use_std_function() { test3(true); } } // namespace std_function + +// different_promise_type +class [[clang::coro_return_type]] Task{}; +struct my_promise_type { + Task get_return_object() { + return {}; + } + suspend_always initial_suspend(); + suspend_always final_suspend() noexcept; + void unhandled_exception(); +}; +namespace std { +template<> class coroutine_traits<Task, int> { + using promise_type = my_promise_type; +}; +} +// expected-error@+1 {{neither a coroutine nor a coroutine wrapper}} +Task foo(int) { return Task{}; } \ No newline at end of file >From dd34d4c3438c3bad35e5599ec1f895f658399ec8 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Wed, 15 Nov 2023 16:26:21 +0100 Subject: [PATCH 07/13] update docs --- clang/include/clang/Basic/AttrDocs.td | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 824bdb0eba886b0..7daae46445d7509 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7504,8 +7504,11 @@ is not a coroutine itself and is marked with ``[[clang::coro_wrapper]]``. From a language perspective, it is not possible to differentiate between a coroutine and a function returning a CRT by merely looking at the function signature. -These two annotations allows implementations to enforce that only coroutines and explicitly marked -coroutine wrappers are allowed to return a ``CRT``. + +Clang will enforce that all functions that return a ``CRT`` are either coroutines or marked +with `[[clang::coro_wrapper]]`. Coroutine wrappers, in particular, are susceptible to capturing +references to temporaries and other lifetime issues. This allows to avoid such lifetime +issues with coroutine wrappers. For example, >From 7dbc5c907f8fe320d1d1398a695480307ae074ee Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Thu, 16 Nov 2023 04:56:30 +0100 Subject: [PATCH 08/13] Apply suggestions from code review Co-authored-by: Chuanqi Xu <yedeng...@linux.alibaba.com> --- clang/docs/ReleaseNotes.rst | 2 +- clang/include/clang/Basic/AttrDocs.td | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 110a39d988bfbeb..46b54836bae0b8d 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -303,7 +303,7 @@ Attribute Changes in Clang - Clang now introduced ``[[clang::coro_return_type]]`` and ``[[clang::coro_wrapper]]`` attributes. A function returning a type marked with ``[[clang::coro_return_type]]`` should be a coroutine. A non-coroutine function marked with ``[[clang::coro_wrapper]]`` - is still allowed to return the such a type. + is still allowed to return the such a type. This is helpful for analyzers to recognize coroutines from the function signatures. Improvements to Clang's diagnostics ----------------------------------- diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 7daae46445d7509..38cc57e8fa913b0 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7487,6 +7487,8 @@ generation of the other destruction cases, optimizing the above `foo.destroy` to def CoroReturnTypeAndWrapperDoc : Documentation { let Category = DocCatDecl; let Content = [{ +The `[[clang::coro_return_type]]` attribute is used to help static analyzers to recognize coroutines from the function signatures. + The ``coro_return_type`` attribute should be marked on a C++ class to mark it as a **coroutine return type (CRT)**. @@ -7495,7 +7497,7 @@ is marked by ``[[clang::coro_return_type]]`` and ``R`` has a promise type assoc (i.e., std::coroutine_traits<R, P1, .., PN>::promise_type is a valid promise type). If the return type of a function is a ``CRT`` then the function must be a coroutine. -Otherwise it is invalid. It is allowed for a non-coroutine to return a ``CRT`` +Otherwise the program is invalid. It is allowed for a non-coroutine to return a ``CRT`` if the function is marked with ``[[clang::coro_wrapper]]``. The ``coro_wrapper`` attribute should be marked on a C++ function to mark it as >From e328a3a6ecea234acc3dd72a0c5998f7c7956247 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Thu, 16 Nov 2023 13:23:08 +0100 Subject: [PATCH 09/13] remove space --- clang/include/clang/Basic/AttrDocs.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 38cc57e8fa913b0..6809009dccb9854 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7507,7 +7507,7 @@ is not a coroutine itself and is marked with ``[[clang::coro_wrapper]]``. From a language perspective, it is not possible to differentiate between a coroutine and a function returning a CRT by merely looking at the function signature. -Clang will enforce that all functions that return a ``CRT`` are either coroutines or marked +Clang will enforce that all functions that return a ``CRT`` are either coroutines or marked with `[[clang::coro_wrapper]]`. Coroutine wrappers, in particular, are susceptible to capturing references to temporaries and other lifetime issues. This allows to avoid such lifetime issues with coroutine wrappers. >From 504f2c37fba47f781151c23adcf832a13f50fa04 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Fri, 17 Nov 2023 14:55:58 +0100 Subject: [PATCH 10/13] add lambda in wrapper test --- clang/test/SemaCXX/coro-return-type-and-wrapper.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp index 71e28df31e56387..089020ffd25b932 100644 --- a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp +++ b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp @@ -54,7 +54,15 @@ void foo() { return foo_coro(1); }; } + +Co<int> coor_containing_lambda(int b) { + // expected-error@+1 {{neither a coroutine nor a coroutine wrapper}} + auto wrapper_lambda = []() -> Gen<int> { + return foo_coro(1); + }; + co_return wrapper_lambda(); } +} // namespace lambdas namespace std_function { namespace std { @@ -112,6 +120,6 @@ namespace std { template<> class coroutine_traits<Task, int> { using promise_type = my_promise_type; }; -} +} // namespace std // expected-error@+1 {{neither a coroutine nor a coroutine wrapper}} Task foo(int) { return Task{}; } \ No newline at end of file >From f2daa6966b6112b8912ff915563e8695584ac3ae Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Fri, 17 Nov 2023 15:12:23 +0100 Subject: [PATCH 11/13] lambda coroutine wrapper test --- .../SemaCXX/coro-return-type-and-wrapper.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp index 089020ffd25b932..5f8076f1c782ac3 100644 --- a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp +++ b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp @@ -13,7 +13,7 @@ template <typename T> struct [[clang::coro_return_type]] Gen { suspend_always initial_suspend(); suspend_always final_suspend() noexcept; void unhandled_exception(); - void return_value(const T &t); + void return_value(T t); template <typename U> auto await_transform(const Gen<U> &) { @@ -45,22 +45,31 @@ Co<int> non_marked_wrapper(int b) { return foo_coro(b); } } // namespace using_decl namespace lambdas { +#define CORO_WRAPPER \ + _Pragma("clang diagnostic push") \ + _Pragma("clang diagnostic ignored \"-Wc++23-extensions\"") \ + [[clang::coro_wrapper]] \ + _Pragma("clang diagnostic pop") + void foo() { auto coro_lambda = []() -> Gen<int> { co_return 1; }; // expected-error@+1 {{neither a coroutine nor a coroutine wrapper}} - auto wrapper_lambda = []() -> Gen<int> { + auto not_allowed_wrapper = []() -> Gen<int> { + return foo_coro(1); + }; + auto allowed_wrapper = [] CORO_WRAPPER() -> Gen<int> { return foo_coro(1); }; } -Co<int> coor_containing_lambda(int b) { +Gen<int> coro_containing_lambda() { // expected-error@+1 {{neither a coroutine nor a coroutine wrapper}} auto wrapper_lambda = []() -> Gen<int> { return foo_coro(1); }; - co_return wrapper_lambda(); + co_return co_await wrapper_lambda(); } } // namespace lambdas @@ -122,4 +131,4 @@ template<> class coroutine_traits<Task, int> { }; } // namespace std // expected-error@+1 {{neither a coroutine nor a coroutine wrapper}} -Task foo(int) { return Task{}; } \ No newline at end of file +Task foo(int) { return Task{}; } >From 236ac4867b54d188f1efb31e44dda5819eb34569 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Fri, 17 Nov 2023 15:19:58 +0100 Subject: [PATCH 12/13] moved getCurFunction out of checkCoroutineWrapper. updated doc --- clang/include/clang/Sema/Sema.h | 7 ++++--- clang/lib/Sema/SemaDecl.cpp | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 7d029810f0ad67e..3376bf65ab11171 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -11184,9 +11184,10 @@ class Sema final { VarDecl *buildCoroutinePromise(SourceLocation Loc); void CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body); - // As a clang extension, enforces that a function returning a type marked with - // [[clang::coro_return_type]] must be a coroutine. A function marked with - // [[clang::coro_wrapper]] are still allowed to return such a type. + // As a clang extension, enforces that a non-coroutine function must be marked + // with [[clang::coro_wrapper]] if it returns a type marked with + // [[clang::coro_return_type]]. + // Expects that FD is not a coroutine. void CheckCoroutineWrapper(FunctionDecl *FD); /// Lookup 'coroutine_traits' in std namespace and std::experimental /// namespace. The namespace found is recorded in Namespace. diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 8e2963a62c1ec16..66e639f0b7b4860 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -15814,7 +15814,7 @@ static void diagnoseImplicitlyRetainedSelf(Sema &S) { } void Sema::CheckCoroutineWrapper(FunctionDecl *FD) { - if (!FD || getCurFunction()->isCoroutine()) + if (!FD) return; RecordDecl *RD = FD->getReturnType()->getAsRecordDecl(); if (!RD || !RD->getUnderlyingDecl()->hasAttr<CoroReturnTypeAttr>()) @@ -15841,7 +15841,8 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body, if (getLangOpts().Coroutines) { if (FSI->isCoroutine()) CheckCompletedCoroutineBody(FD, Body); - CheckCoroutineWrapper(FD); + else + CheckCoroutineWrapper(FD); } { >From f4c520795a3c35699361b58590815bfcc9416cb4 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Fri, 17 Nov 2023 15:32:28 +0100 Subject: [PATCH 13/13] doc updates --- clang/include/clang/Basic/AttrDocs.td | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 6809009dccb9854..249cd737768fef7 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7487,7 +7487,8 @@ generation of the other destruction cases, optimizing the above `foo.destroy` to def CoroReturnTypeAndWrapperDoc : Documentation { let Category = DocCatDecl; let Content = [{ -The `[[clang::coro_return_type]]` attribute is used to help static analyzers to recognize coroutines from the function signatures. +The ``[[clang::coro_return_type]]`` attribute is used to help static analyzers to recognize +coroutines from the function signatures. The ``coro_return_type`` attribute should be marked on a C++ class to mark it as a **coroutine return type (CRT)**. @@ -7500,15 +7501,17 @@ If the return type of a function is a ``CRT`` then the function must be a corout Otherwise the program is invalid. It is allowed for a non-coroutine to return a ``CRT`` if the function is marked with ``[[clang::coro_wrapper]]``. -The ``coro_wrapper`` attribute should be marked on a C++ function to mark it as +The ``[[clang::coro_wrapper]]`` attribute should be marked on a C++ function to mark it as a **coroutine wrapper**. A coroutine wrapper is a function which returns a ``CRT``, is not a coroutine itself and is marked with ``[[clang::coro_wrapper]]``. +Clang will enforce that all functions that return a ``CRT`` are either coroutines or marked +with ``[[clang::coro_wrapper]]``. Clang will enforce this with an error. + From a language perspective, it is not possible to differentiate between a coroutine and a function returning a CRT by merely looking at the function signature. -Clang will enforce that all functions that return a ``CRT`` are either coroutines or marked -with `[[clang::coro_wrapper]]`. Coroutine wrappers, in particular, are susceptible to capturing +Coroutine wrappers, in particular, are susceptible to capturing references to temporaries and other lifetime issues. This allows to avoid such lifetime issues with coroutine wrappers. @@ -7533,5 +7536,7 @@ For example, std::function<Task<int>(int)> f = increment; } + Note: ``a_promise_type::get_return_object`` is exempted from this analysis as it is a necessary + implementation detail of any coroutine library. }]; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits