[clang] [coroutines] Introduce [[clang::coro_return_type]] and [[clang::coro_wrapper]] (PR #71945)

Utkarsh Saxena via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 15 19:56:40 PST 2023


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 <usx at google.com>
Date: Fri, 10 Nov 2023 15:10:44 +0100
Subject: [PATCH 1/8] [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 at +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 at +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 <usx at google.com>
Date: Fri, 10 Nov 2023 16:24:35 +0100
Subject: [PATCH 2/8] 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 at +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 at +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 at +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 at +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 <usx at google.com>
Date: Mon, 13 Nov 2023 10:28:30 +0100
Subject: [PATCH 3/8] 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 <usx at google.com>
Date: Mon, 13 Nov 2023 10:38:05 +0100
Subject: [PATCH 4/8] 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 <usx at google.com>
Date: Mon, 13 Nov 2023 12:28:30 +0100
Subject: [PATCH 5/8] 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 <usx at google.com>
Date: Wed, 15 Nov 2023 16:08:54 +0100
Subject: [PATCH 6/8] 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 at +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 <usx at google.com>
Date: Wed, 15 Nov 2023 16:26:21 +0100
Subject: [PATCH 7/8] 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 <usx at google.com>
Date: Thu, 16 Nov 2023 04:56:30 +0100
Subject: [PATCH 8/8] Apply suggestions from code review

Co-authored-by: Chuanqi Xu <yedeng.yd at 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



More information about the cfe-commits mailing list