[PATCH] D47454: [coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)
GorNishanov created this revision.
GorNishanov added reviewers: modocache, rsmith, lewissbaker.
Herald added a subscriber: EricWF.
Complete the implementation of p0914r1.
Implicit object parameter should be passed to a promise constructor.
Fixes: https://bugs.llvm.org/show_bug.cgi?id=37604
https://reviews.llvm.org/D47454
Files:
lib/Sema/SemaCoroutine.cpp
test/CodeGenCoroutines/coro-params.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -831,7 +831,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag1) {
+extern "C" int f(mismatch_gro_type_tag1) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -848,7 +848,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag2) {
+extern "C" int f(mismatch_gro_type_tag2) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an lvalue of type 'void *'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -866,7 +866,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag3) {
+extern "C" int f(mismatch_gro_type_tag3) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -885,7 +885,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag4) {
+extern "C" int f(mismatch_gro_type_tag4) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'char *'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -1246,7 +1246,10 @@
co_return;
}
+struct some_class;
+
struct good_promise_custom_constructor {
+ good_promise_custom_constructor(some_class&, float, int);
good_promise_custom_constructor(double, float, int);
good_promise_custom_constructor() = delete;
coro get_return_object();
@@ -1261,9 +1264,20 @@
co_return;
}
+struct some_class {
+ coro
+ good_coroutine_calls_custom_constructor(float, int) {
+co_return;
+ }
+ coro
+ static good_coroutine_calls_custom_constructor(double, float, int) {
+co_return;
+ }
+};
+
struct bad_promise_no_matching_constructor {
bad_promise_no_matching_constructor(int, int, int);
- // expected-note@+1 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
+ // expected-note@+1 2 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
bad_promise_no_matching_constructor() = delete;
coro get_return_object();
suspend_always initial_suspend();
@@ -1278,6 +1292,14 @@
co_return;
}
+struct some_class2 {
+coro
+bad_coroutine_calls_with_no_matching_constructor(int, int, int) {
+ // expected-error@-1 {{call to deleted constructor}}
+ co_return;
+}
+};
+
} // namespace CoroHandleMemberFunctionTest
class awaitable_no_unused_warn {
Index: test/CodeGenCoroutines/coro-params.cpp
===
--- test/CodeGenCoroutines/coro-params.cpp
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -156,3 +156,28 @@
// CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJv28promise_matching_constructorifdEE12promise_typeC1ES1_ifd(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
co_return;
}
+
+struct some_class;
+
+struct method {};
+
+template struct std::experimental::coroutine_traits {
+ struct promise_type {
+promise_type(some_class&, float);
+method get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend();
+void return_void();
+void unhandled_exception();
+ };
+};
+
+struct some_class {
+ method good_coroutine_calls_custom_constructor(float);
+};
+
+// CHECK-LABEL: define void @_ZN10some_class39good_coroutine_calls_custom_constructorEf(%struct.some_class*
+method some_class::good_coroutine_calls_custom_constructor(float) {
+ // CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJ6methodR10some_classfEE12promise_typeC1ES3_f(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, %struct.some_class* dereferenceable(1) %{{.+}}, float
+ co_return;
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -510,6 +510,20 @@
// Build a list of arguments, based on the coroutine functions arguments,
// that will be passed to the promise type's constructor.
llvm::SmallVector CtorArgExprs;
+
+ // Add implicit object parameter.
+ if (auto *MD = dyn_cast(FD)) {
+if (MD->isInstance() && !isLambdaCallOperator(MD)) {
+
[PATCH] D47454: [coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL79: [coroutines] Pass implicit object parameter to
promise ctor (fix BUG37604) (authored by GorNishanov, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D47454?vs=148831&id=148833#toc
Repository:
rL LLVM
https://reviews.llvm.org/D47454
Files:
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp
Index: cfe/trunk/test/SemaCXX/coroutines.cpp
===
--- cfe/trunk/test/SemaCXX/coroutines.cpp
+++ cfe/trunk/test/SemaCXX/coroutines.cpp
@@ -831,7 +831,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag1) {
+extern "C" int f(mismatch_gro_type_tag1) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -848,7 +848,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag2) {
+extern "C" int f(mismatch_gro_type_tag2) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an lvalue of type 'void *'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -866,7 +866,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag3) {
+extern "C" int f(mismatch_gro_type_tag3) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -885,7 +885,7 @@
};
};
-extern "C" int f(mismatch_gro_type_tag4) {
+extern "C" int f(mismatch_gro_type_tag4) {
// expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'char *'}}
co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
}
@@ -1246,7 +1246,10 @@
co_return;
}
+struct some_class;
+
struct good_promise_custom_constructor {
+ good_promise_custom_constructor(some_class&, float, int);
good_promise_custom_constructor(double, float, int);
good_promise_custom_constructor() = delete;
coro get_return_object();
@@ -1261,9 +1264,20 @@
co_return;
}
+struct some_class {
+ coro
+ good_coroutine_calls_custom_constructor(float, int) {
+co_return;
+ }
+ coro
+ static good_coroutine_calls_custom_constructor(double, float, int) {
+co_return;
+ }
+};
+
struct bad_promise_no_matching_constructor {
bad_promise_no_matching_constructor(int, int, int);
- // expected-note@+1 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
+ // expected-note@+1 2 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
bad_promise_no_matching_constructor() = delete;
coro get_return_object();
suspend_always initial_suspend();
@@ -1278,6 +1292,14 @@
co_return;
}
+struct some_class2 {
+coro
+bad_coroutine_calls_with_no_matching_constructor(int, int, int) {
+ // expected-error@-1 {{call to deleted constructor}}
+ co_return;
+}
+};
+
} // namespace CoroHandleMemberFunctionTest
class awaitable_no_unused_warn {
Index: cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
@@ -156,3 +156,28 @@
// CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJv28promise_matching_constructorifdEE12promise_typeC1ES1_ifd(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
co_return;
}
+
+struct some_class;
+
+struct method {};
+
+template struct std::experimental::coroutine_traits {
+ struct promise_type {
+promise_type(some_class&, float);
+method get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend();
+void return_void();
+void unhandled_exception();
+ };
+};
+
+struct some_class {
+ method good_coroutine_calls_custom_constructor(float);
+};
+
+// CHECK-LABEL: define void @_ZN10some_class39good_coroutine_calls_custom_constructorEf(%struct.some_class*
+method some_class::good_coroutine_calls_custom_constructor(float) {
+ // CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJ6methodR10some_classfEE12promise_typeC1ES3_f(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, %struct.some_class* dereferenceable(1) %{{.+}}, float
+ co_return;
+}
Index: cfe/trunk/lib/Sema/SemaCoroutine.cpp
===
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp
@@ -510,6 +510,20 @@
// Build a list of arguments, based on the coroutine functions arguments,
// that will be passed to the promi
[PATCH] D37115: [coroutines] Do not attempt to typo-correct when coroutine is looking for required members
GorNishanov added a comment. ping https://reviews.llvm.org/D37115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35046: [coroutines] Include "this" type when looking up coroutine_traits
GorNishanov added a comment.
@toby-allsopp: You mentioned that @EricWF already got this in. Can you close
("abandon") this patch if it is no longer needed.
https://reviews.llvm.org/D35046
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37115: [coroutines] Do not attempt to typo-correct when coroutine is looking for required members
GorNishanov added a comment. @rsmith , here is a fix for the funny diagnostics you mentioned. https://reviews.llvm.org/D37115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46140: [coroutines] std::task type (WIP)
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. Herald added a subscriber: jfb. LGTM With couple of suggestions. 1. Remove swap. It is not part of the proposal at the moment. 2. If you feel like, add a lazy alias to task, that way we can taste both names and see how it feels. My apologies for taking so long to review. https://reviews.llvm.org/D46140 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42605: [Sema] Toggle diags when finding allocators (NFCI)
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D42605 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42606: [Coroutines] Use allocator overload when available
GorNishanov added inline comments.
Comment at: lib/Sema/SemaCoroutine.cpp:1062
+ // an argument list."
+ for (auto *PD : FD.parameters()) {
+if (PD->getType()->isDependentType())
This does not implement TS specified behavior for non static member functions:
[dcl.fct.def.coroutine]/7 states that an argument list is build up as follows:
> The first argument is the amount of space requested, and has type
> std::size_t. The lvalues p1 ... pn are the succeeding arguments.
Where p1 ... pn are defined earlier in
[dcl.fct.def.coroutine]/3 as:
> For a coroutine f that is a non-static member function, let P1 denote the
> type of the implicit object parameter (13.3.1) and P2 ... Pn be the types of
> the function parameters; otherwise let P1 ... Pn be the types of the function
> parameters.
Essentially for non-static member functions, we need to insert implicit object
parameter.
Note that lookupPromiseType in SemaCoroutine.cpp when building specialization
of `std::experimental::coroutine_traits<...>` includes implicit object
parameter:
```
// If the function is a non-static member function, add the type
// of the implicit object parameter before the formal parameters.
if (auto *MD = dyn_cast(FD)) {
if (MD->isInstance()) {
// [over.match.funcs]4
// For non-static member functions, the type of the implicit object
// parameter is
// -- "lvalue reference to cv X" for functions declared without a
// ref-qualifier or with the & ref-qualifier
// -- "rvalue reference to cv X" for functions declared with the &&
// ref-qualifier
QualType T =
MD->getThisType(S.Context)->getAs()->getPointeeType();
T = FnType->getRefQualifier() == RQ_RValue
? S.Context.getRValueReferenceType(T)
: S.Context.getLValueReferenceType(T, /*SpelledAsLValue*/ true);
AddArg(T);
}
}
```
Repository:
rC Clang
https://reviews.llvm.org/D42606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42606: [Coroutines] Use allocator overload when available
GorNishanov added inline comments.
Comment at: lib/Sema/SemaCoroutine.cpp:1062
+ // an argument list."
+ for (auto *PD : FD.parameters()) {
+if (PD->getType()->isDependentType())
GorNishanov wrote:
> This does not implement TS specified behavior for non static member functions:
>
> [dcl.fct.def.coroutine]/7 states that an argument list is build up as
> follows:
>
> > The first argument is the amount of space requested, and has type
> > std::size_t. The lvalues p1 ... pn are the succeeding arguments.
>
> Where p1 ... pn are defined earlier in
>
> [dcl.fct.def.coroutine]/3 as:
>
> > For a coroutine f that is a non-static member function, let P1 denote the
> > type of the implicit object parameter (13.3.1) and P2 ... Pn be the types
> > of the function parameters; otherwise let P1 ... Pn be the types of the
> > function parameters.
>
> Essentially for non-static member functions, we need to insert implicit
> object parameter.
>
> Note that lookupPromiseType in SemaCoroutine.cpp when building specialization
> of `std::experimental::coroutine_traits<...>` includes implicit object
> parameter:
>
> ```
> // If the function is a non-static member function, add the type
> // of the implicit object parameter before the formal parameters.
> if (auto *MD = dyn_cast(FD)) {
> if (MD->isInstance()) {
> // [over.match.funcs]4
> // For non-static member functions, the type of the implicit object
> // parameter is
> // -- "lvalue reference to cv X" for functions declared without a
> // ref-qualifier or with the & ref-qualifier
> // -- "rvalue reference to cv X" for functions declared with the &&
> // ref-qualifier
> QualType T =
> MD->getThisType(S.Context)->getAs()->getPointeeType();
> T = FnType->getRefQualifier() == RQ_RValue
> ? S.Context.getRValueReferenceType(T)
> : S.Context.getLValueReferenceType(T, /*SpelledAsLValue*/ true);
> AddArg(T);
> }
> }
> ```
I think promise constructor argument preview is also missing the implicit
object parameter.
Repository:
rC Clang
https://reviews.llvm.org/D42606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42606: [Coroutines] Use allocator overload when available
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D42606 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31608: [coroutines] Add emission of initial and final suspends
GorNishanov added a comment. In https://reviews.llvm.org/D31608#762783, @alekseyshl wrote: > Leaks and warnings are reported in coro-await.cpp: > http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/1400/steps/check-clang%20asan/logs/stdio. > Please fix. Thank you! The fix is incoming in a minute. https://reviews.llvm.org/D31608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31608: [coroutines] Add emission of initial and final suspends
GorNishanov added a comment. Fixed: r303714 = 8832327ab89f3668378d70d1c4e5a218446ce36e https://reviews.llvm.org/D31608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31670: [coroutines] Implement correct GRO lifetime
GorNishanov updated this revision to Diff 100027.
GorNishanov added a comment.
merge with tot, preparing to land
https://reviews.llvm.org/D31670
Files:
lib/CodeGen/CGCoroutine.cpp
test/CodeGenCoroutines/coro-gro.cpp
Index: test/CodeGenCoroutines/coro-gro.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-gro.cpp
@@ -0,0 +1,86 @@
+// Verifies lifetime of __gro local variable
+// Verify that coroutine promise and allocated memory are freed up on exception.
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s
+
+namespace std::experimental {
+template struct coroutine_traits;
+
+template struct coroutine_handle {
+ coroutine_handle() = default;
+ static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+ static coroutine_handle from_address(void *) noexcept;
+ coroutine_handle() = default;
+ template
+ coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
+};
+
+struct GroType {
+ ~GroType();
+ operator int() noexcept;
+};
+
+template <> struct std::experimental::coroutine_traits {
+ struct promise_type {
+GroType get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+promise_type();
+~promise_type();
+void unhandled_exception() noexcept;
+ };
+};
+
+struct Cleanup { ~Cleanup(); };
+void doSomething() noexcept;
+
+// CHECK: define i32 @_Z1fv(
+int f() {
+ // CHECK: %[[RetVal:.+]] = alloca i32
+ // CHECK: %[[GroActive:.+]] = alloca i1
+
+ // CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64()
+ // CHECK: call i8* @_Znwm(i64 %[[Size]])
+ // CHECK: store i1 false, i1* %[[GroActive]]
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeC1Ev(
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type17get_return_objectEv(
+ // CHECK: store i1 true, i1* %[[GroActive]]
+
+ Cleanup cleanup;
+ doSomething();
+ co_return;
+
+ // CHECK: call void @_Z11doSomethingv(
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type11return_voidEv(
+ // CHECK: call void @_ZN7CleanupD1Ev(
+
+ // Destroy promise and free the memory.
+
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeD1Ev(
+ // CHECK: %[[Mem:.+]] = call i8* @llvm.coro.free(
+ // CHECK: call void @_ZdlPv(i8* %[[Mem]])
+
+ // Initialize retval from Gro and destroy Gro
+
+ // CHECK: %[[Conv:.+]] = call i32 @_ZN7GroTypecviEv(
+ // CHECK: store i32 %[[Conv]], i32* %[[RetVal]]
+ // CHECK: %[[IsActive:.+]] = load i1, i1* %[[GroActive]]
+ // CHECK: br i1 %[[IsActive]], label %[[CleanupGro:.+]], label %[[Done:.+]]
+
+ // CHECK: [[CleanupGro]]:
+ // CHECK: call void @_ZN7GroTypeD1Ev(
+ // CHECK: br label %[[Done]]
+
+ // CHECK: [[Done]]:
+ // CHECK: %[[LoadRet:.+]] = load i32, i32* %[[RetVal]]
+ // CHECK: ret i32 %[[LoadRet]]
+}
Index: lib/CodeGen/CGCoroutine.cpp
===
--- lib/CodeGen/CGCoroutine.cpp
+++ lib/CodeGen/CGCoroutine.cpp
@@ -11,6 +11,7 @@
//
//===--===//
+#include "CGCleanup.h"
#include "CodeGenFunction.h"
#include "llvm/ADT/ScopeExit.h"
#include "clang/AST/StmtCXX.h"
@@ -326,6 +327,72 @@
};
}
+namespace {
+struct GetReturnObjectManager {
+ CodeGenFunction &CGF;
+ CGBuilderTy &Builder;
+ const CoroutineBodyStmt &S;
+
+ Address GroActiveFlag;
+ CodeGenFunction::AutoVarEmission GroEmission;
+
+ GetReturnObjectManager(CodeGenFunction &CGF, const CoroutineBodyStmt &S)
+ : CGF(CGF), Builder(CGF.Builder), S(S), GroActiveFlag(Address::invalid()),
+GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {}
+
+ // The gro variable has to outlive coroutine frame and coroutine promise, but,
+ // it can only be initialized after coroutine promise was created, thus, we
+ // split its emission in two parts. EmitGroAlloca emits an alloca and sets up
+ // cleanups. Later when coroutine promise is available we initialize the gro
+ // and sets the flag that the cleanup is now active.
+
+ void EmitGroAlloca() {
+auto *GroDeclStmt = dyn_cast(S.getResultDecl());
+if (!GroDeclStmt) {
+ // If get_return_object returns void, no need to do an alloca.
+ return;
+}
+
+auto *GroVarDecl = cast(GroDeclStmt->getSingleDecl());
+
+// Set GRO flag that it is not initialized yet
+GroActiveFlag =
+ CGF.CreateTempAlloca(Builder.getInt1Ty(), CharUnits::One(), "gro.active");
+Builder.CreateStore(Builder.getFalse(), GroActiveFlag);
+
+GroEmission = CGF.EmitAutoVarAlloca(*GroVar
[PATCH] D33477: [coroutines] Implement correct GRO lifetime
GorNishanov created this revision.
Herald added a subscriber: EricWF.
Sema creates a declaration for gro variable as:
auto $gro = $promise.get_return_object();
However, gro variable has to outlive coroutine frame and coroutine promise, but,
it can only be initialized after the coroutine promise was created, thus, we
split its emission in two parts: EmitGroAlloca emits an alloca and sets up
the cleanups. Later when the coroutine promise is available we initialize
the gro and set the flag that the cleanup is now active.
Duplicate of: https://reviews.llvm.org/D31670 (which arc patch refuses to apply
for some reason)
https://reviews.llvm.org/D33477
Files:
lib/CodeGen/CGCoroutine.cpp
test/CodeGenCoroutines/coro-gro.cpp
Index: test/CodeGenCoroutines/coro-gro.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-gro.cpp
@@ -0,0 +1,86 @@
+// Verifies lifetime of __gro local variable
+// Verify that coroutine promise and allocated memory are freed up on exception.
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s
+
+namespace std::experimental {
+template struct coroutine_traits;
+
+template struct coroutine_handle {
+ coroutine_handle() = default;
+ static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+ static coroutine_handle from_address(void *) noexcept;
+ coroutine_handle() = default;
+ template
+ coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
+};
+
+struct GroType {
+ ~GroType();
+ operator int() noexcept;
+};
+
+template <> struct std::experimental::coroutine_traits {
+ struct promise_type {
+GroType get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+promise_type();
+~promise_type();
+void unhandled_exception() noexcept;
+ };
+};
+
+struct Cleanup { ~Cleanup(); };
+void doSomething() noexcept;
+
+// CHECK: define i32 @_Z1fv(
+int f() {
+ // CHECK: %[[RetVal:.+]] = alloca i32
+ // CHECK: %[[GroActive:.+]] = alloca i1
+
+ // CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64()
+ // CHECK: call i8* @_Znwm(i64 %[[Size]])
+ // CHECK: store i1 false, i1* %[[GroActive]]
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeC1Ev(
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type17get_return_objectEv(
+ // CHECK: store i1 true, i1* %[[GroActive]]
+
+ Cleanup cleanup;
+ doSomething();
+ co_return;
+
+ // CHECK: call void @_Z11doSomethingv(
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type11return_voidEv(
+ // CHECK: call void @_ZN7CleanupD1Ev(
+
+ // Destroy promise and free the memory.
+
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeD1Ev(
+ // CHECK: %[[Mem:.+]] = call i8* @llvm.coro.free(
+ // CHECK: call void @_ZdlPv(i8* %[[Mem]])
+
+ // Initialize retval from Gro and destroy Gro
+
+ // CHECK: %[[Conv:.+]] = call i32 @_ZN7GroTypecviEv(
+ // CHECK: store i32 %[[Conv]], i32* %[[RetVal]]
+ // CHECK: %[[IsActive:.+]] = load i1, i1* %[[GroActive]]
+ // CHECK: br i1 %[[IsActive]], label %[[CleanupGro:.+]], label %[[Done:.+]]
+
+ // CHECK: [[CleanupGro]]:
+ // CHECK: call void @_ZN7GroTypeD1Ev(
+ // CHECK: br label %[[Done]]
+
+ // CHECK: [[Done]]:
+ // CHECK: %[[LoadRet:.+]] = load i32, i32* %[[RetVal]]
+ // CHECK: ret i32 %[[LoadRet]]
+}
Index: lib/CodeGen/CGCoroutine.cpp
===
--- lib/CodeGen/CGCoroutine.cpp
+++ lib/CodeGen/CGCoroutine.cpp
@@ -11,6 +11,7 @@
//
//===--===//
+#include "CGCleanup.h"
#include "CodeGenFunction.h"
#include "llvm/ADT/ScopeExit.h"
#include "clang/AST/StmtCXX.h"
@@ -326,6 +327,72 @@
};
}
+namespace {
+struct GetReturnObjectManager {
+ CodeGenFunction &CGF;
+ CGBuilderTy &Builder;
+ const CoroutineBodyStmt &S;
+
+ Address GroActiveFlag;
+ CodeGenFunction::AutoVarEmission GroEmission;
+
+ GetReturnObjectManager(CodeGenFunction &CGF, const CoroutineBodyStmt &S)
+ : CGF(CGF), Builder(CGF.Builder), S(S), GroActiveFlag(Address::invalid()),
+GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {}
+
+ // The gro variable has to outlive coroutine frame and coroutine promise, but,
+ // it can only be initialized after coroutine promise was created, thus, we
+ // split its emission in two parts. EmitGroAlloca emits an alloca and sets up
+ // cleanups. Later when coroutine promise is available we initialize the gro
+ // and sets the flag that the cleanup is now active.
+
+ void EmitGro
[PATCH] D33477: [coroutines] Implement correct GRO lifetime
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303716: [coroutines] Implement correct GRO lifetime
(authored by GorNishanov).
Changed prior to commit:
https://reviews.llvm.org/D33477?vs=100029&id=100030#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33477
Files:
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-gro.cpp
Index: cfe/trunk/test/CodeGenCoroutines/coro-gro.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-gro.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-gro.cpp
@@ -0,0 +1,86 @@
+// Verifies lifetime of __gro local variable
+// Verify that coroutine promise and allocated memory are freed up on exception.
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s
+
+namespace std::experimental {
+template struct coroutine_traits;
+
+template struct coroutine_handle {
+ coroutine_handle() = default;
+ static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+ static coroutine_handle from_address(void *) noexcept;
+ coroutine_handle() = default;
+ template
+ coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
+};
+
+struct GroType {
+ ~GroType();
+ operator int() noexcept;
+};
+
+template <> struct std::experimental::coroutine_traits {
+ struct promise_type {
+GroType get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+promise_type();
+~promise_type();
+void unhandled_exception() noexcept;
+ };
+};
+
+struct Cleanup { ~Cleanup(); };
+void doSomething() noexcept;
+
+// CHECK: define i32 @_Z1fv(
+int f() {
+ // CHECK: %[[RetVal:.+]] = alloca i32
+ // CHECK: %[[GroActive:.+]] = alloca i1
+
+ // CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64()
+ // CHECK: call i8* @_Znwm(i64 %[[Size]])
+ // CHECK: store i1 false, i1* %[[GroActive]]
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeC1Ev(
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type17get_return_objectEv(
+ // CHECK: store i1 true, i1* %[[GroActive]]
+
+ Cleanup cleanup;
+ doSomething();
+ co_return;
+
+ // CHECK: call void @_Z11doSomethingv(
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type11return_voidEv(
+ // CHECK: call void @_ZN7CleanupD1Ev(
+
+ // Destroy promise and free the memory.
+
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeD1Ev(
+ // CHECK: %[[Mem:.+]] = call i8* @llvm.coro.free(
+ // CHECK: call void @_ZdlPv(i8* %[[Mem]])
+
+ // Initialize retval from Gro and destroy Gro
+
+ // CHECK: %[[Conv:.+]] = call i32 @_ZN7GroTypecviEv(
+ // CHECK: store i32 %[[Conv]], i32* %[[RetVal]]
+ // CHECK: %[[IsActive:.+]] = load i1, i1* %[[GroActive]]
+ // CHECK: br i1 %[[IsActive]], label %[[CleanupGro:.+]], label %[[Done:.+]]
+
+ // CHECK: [[CleanupGro]]:
+ // CHECK: call void @_ZN7GroTypeD1Ev(
+ // CHECK: br label %[[Done]]
+
+ // CHECK: [[Done]]:
+ // CHECK: %[[LoadRet:.+]] = load i32, i32* %[[RetVal]]
+ // CHECK: ret i32 %[[LoadRet]]
+}
Index: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
===
--- cfe/trunk/lib/CodeGen/CGCoroutine.cpp
+++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp
@@ -11,6 +11,7 @@
//
//===--===//
+#include "CGCleanup.h"
#include "CodeGenFunction.h"
#include "llvm/ADT/ScopeExit.h"
#include "clang/AST/StmtCXX.h"
@@ -326,6 +327,72 @@
};
}
+namespace {
+struct GetReturnObjectManager {
+ CodeGenFunction &CGF;
+ CGBuilderTy &Builder;
+ const CoroutineBodyStmt &S;
+
+ Address GroActiveFlag;
+ CodeGenFunction::AutoVarEmission GroEmission;
+
+ GetReturnObjectManager(CodeGenFunction &CGF, const CoroutineBodyStmt &S)
+ : CGF(CGF), Builder(CGF.Builder), S(S), GroActiveFlag(Address::invalid()),
+GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {}
+
+ // The gro variable has to outlive coroutine frame and coroutine promise, but,
+ // it can only be initialized after coroutine promise was created, thus, we
+ // split its emission in two parts. EmitGroAlloca emits an alloca and sets up
+ // cleanups. Later when coroutine promise is available we initialize the gro
+ // and sets the flag that the cleanup is now active.
+
+ void EmitGroAlloca() {
+auto *GroDeclStmt = dyn_cast(S.getResultDecl());
+if (!GroDeclStmt) {
+ // If get_return_object returns void, no need to do an alloca.
+ return;
+}
+
+auto *GroVarDecl = cast(GroDeclStmt->getSingleDecl()
[PATCH] D31670: [coroutines] Implement correct GRO lifetime
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. Landed as: https://reviews.llvm.org/rL303716 https://reviews.llvm.org/D31670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31670: [coroutines] Implement correct GRO lifetime
GorNishanov closed this revision. GorNishanov added a comment. Closed by commit https://reviews.llvm.org/rL303716: [coroutines] Implement correct GRO lifetime (authored by GorNishanov) https://reviews.llvm.org/D31670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33479: [coroutines] [NFC] Add tests for return_void, unhandled_exception and promise dtor
GorNishanov created this revision.
- Test that coroutine promise destructor is called.
- Test that we call return_void on fallthrough
- Test that we call unhandled exception in a try catch surrounding the body
https://reviews.llvm.org/D33479
Files:
test/CodeGenCoroutines/Inputs/coroutine.h
test/CodeGenCoroutines/coro-promise-dtor.cpp
test/CodeGenCoroutines/coro-ret-void.cpp
test/CodeGenCoroutines/coro-unhandled-exception.cpp
Index: test/CodeGenCoroutines/coro-unhandled-exception.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-unhandled-exception.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -triple=x86_64-pc-windows-msvc18.0.0 -emit-llvm %s -o - -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck --check-prefix=CHECK-LPAD %s
+
+#include "Inputs/coroutine.h"
+
+namespace coro = std::experimental::coroutines_v1;
+
+namespace std {
+ using exception_ptr = int;
+ exception_ptr current_exception();
+}
+
+struct coro_t {
+ struct promise_type {
+coro_t get_return_object() {
+ coro::coroutine_handle{};
+ return {};
+}
+coro::suspend_never initial_suspend() { return {}; }
+coro::suspend_never final_suspend() { return {}; }
+void return_void(){}
+void unhandled_exception() noexcept;
+ };
+};
+
+struct Cleanup { ~Cleanup(); };
+void may_throw();
+
+coro_t f() {
+ Cleanup x;
+ may_throw();
+ co_return;
+}
+
+// CHECK: @"\01?f@@YA?AUcoro_t@@XZ"(
+// CHECK: invoke void @"\01?may_throw@@YAXXZ"()
+// CHECK: to label %{{.+}} unwind label %[[EHCLEANUP:.+]]
+// CHECK: [[EHCLEANUP]]:
+// CHECK: %[[INNERPAD:.+]] = cleanuppad within none []
+// CHECK: call void @"\01??_DCleanup@@QEAAXXZ"(
+// CHECK: cleanupret from %[[INNERPAD]] unwind label %[[CATCHSW:.+]]
+// CHECK: [[CATCHSW]]:
+// CHECK: %[[CATCHSWTOK:.+]] = catchswitch within none [label %[[CATCH:.+]]] unwind label
+// CHECK: [[CATCH]]:
+// CHECK: %[[CATCHTOK:.+]] = catchpad within [[CATCHSWTOK:.+]]
+// CHECK: call void @"\01?unhandled_exception@promise_type@coro_t@@QEAAXXZ"
+// CHECK: catchret from %[[CATCHTOK]] to label %[[CATCHRETDEST:.+]]
+// CHECK: [[CATCHRETDEST]]:
+// CHECK-NEXT: br label %[[TRYCONT:.+]]
+// CHECK: [[TRYCONT]]:
+// CHECK-NEXT: br label %[[COROFIN:.+]]
+// CHECK: [[COROFIN]]:
+// CHECK-NEXT: invoke void @"\01?final_suspend@promise_type@coro_t@@QEAA?AUsuspend_never@coroutines_v1@experimental@std@@XZ"(
+
+// CHECK-LPAD: @_Z1fv(
+// CHECK-LPAD: invoke void @_Z9may_throwv()
+// CHECK-LPAD: to label %[[CONT:.+]] unwind label %[[CLEANUP:.+]]
+// CHECK-LPAD: [[CLEANUP]]:
+// CHECK-LPAD: call void @_ZN7CleanupD1Ev(%struct.Cleanup* %x) #2
+// CHECK-LPAD: br label %[[CATCH:.+]]
+
+// CHECK-LPAD: [[CATCH]]:
+// CHECK-LPAD:call i8* @__cxa_begin_catch
+// CHECK-LPAD:call void @_ZN6coro_t12promise_type19unhandled_exceptionEv(%"struct.coro_t::promise_type"* %__promise) #2
+// CHECK-LPAD:invoke void @__cxa_end_catch()
+// CHECK-LPAD-NEXT: to label %[[CATCHRETDEST:.+]] unwind label
+// CHECK-LPAD: [[CATCHRETDEST]]:
+// CHECK-LPAD-NEXT: br label %[[TRYCONT:.+]]
+// CHECK-LPAD: [[TRYCONT]]:
+// CHECK-LPAD-NEXT: br label %[[COROFIN:.+]]
+// CHECK-LPAD: [[COROFIN]]:
+// CHECK-LPAD-NEXT: invoke void @_ZN6coro_t12promise_type13final_suspendEv(
Index: test/CodeGenCoroutines/coro-ret-void.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-ret-void.cpp
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+namespace coro = std::experimental::coroutines_v1;
+
+struct coro1 {
+ struct promise_type {
+coro1 get_return_object();
+coro::suspend_never initial_suspend();
+coro::suspend_never final_suspend();
+void return_void();
+ };
+};
+
+coro1 f() {
+ co_await coro::suspend_never{};
+}
+
+// CHECK-LABEL: define void @_Z1fv(
+// CHECK: call void @_ZNSt12experimental13coroutines_v113suspend_never12await_resumeEv(%"struct.std::experimental::coroutines_v1::suspend_never"*
+// CHECK: call void @_ZN5coro112promise_type11return_voidEv(%"struct.coro1::promise_type"* %__promise)
+
+struct coro2 {
+ struct promise_type {
+coro2 get_return_object();
+coro::suspend_never initial_suspend();
+coro::suspend_never final_suspend();
+void return_value(int);
+ };
+};
+
+coro2 g() {
+ co_return 42;
+}
+
+// CHECK-LABEL: define void @_Z1gv(
+// CHECK: call void @_ZNSt12experimental13coroutines_v113suspend_never12await_resumeEv(%"struct.std::experimental::coroutines_v1::suspend_never"*
+// CHECK: call void @_ZN5coro212promise_type12return_valueEi(%"struct.coro2::promise_type"* %__promise, i32 42)
Index: t
[PATCH] D33479: [coroutines] [NFC] Add tests for return_void, unhandled_exception and promise dtor
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D33479 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33481: [coroutines] Improved diagnostics when unhandled_exception is missing in the promise_type
GorNishanov created this revision.
Now we helpfully provide a note pointing at the promise_type in question.
https://reviews.llvm.org/D33481
Files:
lib/Sema/SemaCoroutine.cpp
test/SemaCXX/coroutine-unhandled_exception-warning.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -540,7 +540,7 @@
}
template coro bad_implicit_return_dependent(bad_promise_6); //
expected-note {{in instantiation}}
-struct bad_promise_7 {
+struct bad_promise_7 { // expected-note 2 {{defined here}}
coro get_return_object();
suspend_always initial_suspend();
suspend_always final_suspend();
Index: test/SemaCXX/coroutine-unhandled_exception-warning.cpp
===
--- test/SemaCXX/coroutine-unhandled_exception-warning.cpp
+++ test/SemaCXX/coroutine-unhandled_exception-warning.cpp
@@ -16,7 +16,11 @@
using std::experimental::suspend_always;
using std::experimental::suspend_never;
+#ifndef DISABLE_WARNING
+struct promise_void { // expected-note {{defined here}}
+#else
struct promise_void {
+#endif
void get_return_object();
suspend_always initial_suspend();
suspend_always final_suspend();
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -120,8 +120,7 @@
return PromiseType;
}
-/// Look up the std::coroutine_traits<...>::promise_type for the given
-/// function type.
+/// Look up the std::experimental::coroutine_handle.
static QualType lookupCoroutineHandleType(Sema &S, QualType PromiseType,
SourceLocation Loc) {
if (PromiseType.isNull())
@@ -729,8 +728,7 @@
}
if (isa(Body)) {
-// FIXME(EricWF): Nothing todo. the body is already a transformed coroutine
-// body statement.
+// Nothing todo. the body is already a transformed coroutine body
statement.
return;
}
@@ -1030,6 +1028,8 @@
: diag::
warn_coroutine_promise_unhandled_exception_required_with_exceptions;
S.Diag(Loc, DiagID) << PromiseRecordDecl;
+S.Diag(PromiseRecordDecl->getLocation(), diag::note_defined_here)
+<< PromiseRecordDecl;
return !RequireUnhandledException;
}
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -540,7 +540,7 @@
}
template coro bad_implicit_return_dependent(bad_promise_6); // expected-note {{in instantiation}}
-struct bad_promise_7 {
+struct bad_promise_7 { // expected-note 2 {{defined here}}
coro get_return_object();
suspend_always initial_suspend();
suspend_always final_suspend();
Index: test/SemaCXX/coroutine-unhandled_exception-warning.cpp
===
--- test/SemaCXX/coroutine-unhandled_exception-warning.cpp
+++ test/SemaCXX/coroutine-unhandled_exception-warning.cpp
@@ -16,7 +16,11 @@
using std::experimental::suspend_always;
using std::experimental::suspend_never;
+#ifndef DISABLE_WARNING
+struct promise_void { // expected-note {{defined here}}
+#else
struct promise_void {
+#endif
void get_return_object();
suspend_always initial_suspend();
suspend_always final_suspend();
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -120,8 +120,7 @@
return PromiseType;
}
-/// Look up the std::coroutine_traits<...>::promise_type for the given
-/// function type.
+/// Look up the std::experimental::coroutine_handle.
static QualType lookupCoroutineHandleType(Sema &S, QualType PromiseType,
SourceLocation Loc) {
if (PromiseType.isNull())
@@ -729,8 +728,7 @@
}
if (isa(Body)) {
-// FIXME(EricWF): Nothing todo. the body is already a transformed coroutine
-// body statement.
+// Nothing todo. the body is already a transformed coroutine body statement.
return;
}
@@ -1030,6 +1028,8 @@
: diag::
warn_coroutine_promise_unhandled_exception_required_with_exceptions;
S.Diag(Loc, DiagID) << PromiseRecordDecl;
+S.Diag(PromiseRecordDecl->getLocation(), diag::note_defined_here)
+<< PromiseRecordDecl;
return !RequireUnhandledException;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33481: [coroutines] Improved diagnostics when unhandled_exception is missing in the promise_type
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D33481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33479: [coroutines] [NFC] Add tests for return_void, unhandled_exception and promise dtor
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303748: [coroutines] [NFC] Add tests for return_void,
unhandled_exception and promise… (authored by GorNishanov).
Changed prior to commit:
https://reviews.llvm.org/D33479?vs=100034&id=100084#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33479
Files:
cfe/trunk/test/CodeGenCoroutines/Inputs/coroutine.h
cfe/trunk/test/CodeGenCoroutines/coro-promise-dtor.cpp
cfe/trunk/test/CodeGenCoroutines/coro-ret-void.cpp
cfe/trunk/test/CodeGenCoroutines/coro-unhandled-exception.cpp
Index: cfe/trunk/test/CodeGenCoroutines/coro-promise-dtor.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-promise-dtor.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-promise-dtor.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -triple=x86_64-pc-windows-msvc18.0.0 -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s
+// -triple=x86_64-unknown-linux-gnu
+
+#include "Inputs/coroutine.h"
+
+namespace coro = std::experimental::coroutines_v1;
+
+struct coro_t {
+ void* p;
+ ~coro_t();
+ struct promise_type {
+coro_t get_return_object();
+coro::suspend_never initial_suspend();
+coro::suspend_never final_suspend();
+void return_void();
+promise_type();
+~promise_type();
+void unhandled_exception();
+ };
+};
+
+struct Cleanup { ~Cleanup(); };
+void may_throw();
+
+coro_t f() {
+ Cleanup cleanup;
+ may_throw();
+ co_return;
+}
+
+// CHECK-LABEL: define void @"\01?f@@YA?AUcoro_t@@XZ"(
+// CHECK: %gro.active = alloca i1
+// CHECK: store i1 false, i1* %gro.active
+
+// CHECK: invoke %"struct.coro_t::promise_type"* @"\01??0promise_type@coro_t@@QEAA@XZ"(
+// CHECK: invoke void @"\01?get_return_object@promise_type@coro_t@@QEAA?AU2@XZ"(
+// CHECK: store i1 true, i1* %gro.active
+
+// CHECK: %[[IS_ACTIVE:.+]] = load i1, i1* %gro.active
+// CHECK: br i1 %[[IS_ACTIVE]], label %[[CLEANUP1:.+]], label
+
+// CHECK: [[CLEANUP1]]:
+// CHECK: %[[NRVO:.+]] = load i1, i1* %nrvo
+// CHECK: br i1 %[[NRVO]], label %{{.+}}, label %[[DTOR:.+]]
+
+// CHECK: [[DTOR]]:
+// CHECK: call void @"\01??_Dcoro_t@@QEAAXXZ"(
Index: cfe/trunk/test/CodeGenCoroutines/Inputs/coroutine.h
===
--- cfe/trunk/test/CodeGenCoroutines/Inputs/coroutine.h
+++ cfe/trunk/test/CodeGenCoroutines/Inputs/coroutine.h
@@ -0,0 +1,80 @@
+#pragma once
+
+namespace std { namespace experimental { inline namespace coroutines_v1 {
+
+template struct coroutine_traits {
+ using promise_type = typename R::promise_type;
+};
+
+template struct coroutine_handle;
+
+template <> struct coroutine_handle {
+ static coroutine_handle from_address(void *addr) noexcept {
+coroutine_handle me;
+me.ptr = addr;
+return me;
+ }
+ void operator()() { resume(); }
+ void *address() const { return ptr; }
+ void resume() const { __builtin_coro_resume(ptr); }
+ void destroy() const { __builtin_coro_destroy(ptr); }
+ bool done() const { return __builtin_coro_done(ptr); }
+ coroutine_handle &operator=(decltype(nullptr)) {
+ptr = nullptr;
+return *this;
+ }
+ coroutine_handle(decltype(nullptr)) : ptr(nullptr) {}
+ coroutine_handle() : ptr(nullptr) {}
+// void reset() { ptr = nullptr; } // add to P0057?
+ explicit operator bool() const { return ptr; }
+
+protected:
+ void *ptr;
+};
+
+template struct coroutine_handle : coroutine_handle<> {
+ using coroutine_handle<>::operator=;
+
+ static coroutine_handle from_address(void *addr) noexcept {
+coroutine_handle me;
+me.ptr = addr;
+return me;
+ }
+
+ Promise &promise() const {
+return *reinterpret_cast(
+__builtin_coro_promise(ptr, alignof(Promise), false));
+ }
+ static coroutine_handle from_promise(Promise &promise) {
+coroutine_handle p;
+p.ptr = __builtin_coro_promise(&promise, alignof(Promise), true);
+return p;
+ }
+};
+
+ template
+ bool operator==(coroutine_handle<_PromiseT> const& _Left,
+coroutine_handle<_PromiseT> const& _Right) noexcept
+ {
+return _Left.address() == _Right.address();
+ }
+
+ template
+ bool operator!=(coroutine_handle<_PromiseT> const& _Left,
+coroutine_handle<_PromiseT> const& _Right) noexcept
+ {
+return !(_Left == _Right);
+ }
+
+struct suspend_always {
+ bool await_ready() { return false; }
+ void await_suspend(coroutine_handle<>) {}
+ void await_resume() {}
+};
+struct suspend_never {
+ bool await_ready() { return true; }
+ void await_suspend(coroutine_handle<>) {}
+ void await_resume() {}
+};
+
+}}}
Index: cfe/trunk/test/CodeGenCoroutines/coro-unhandled-exception.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-unhandled-exception.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-unhandled-exception.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -std=c++14 -fcorou
[PATCH] D33481: [coroutines] Improved diagnostics when unhandled_exception is missing in the promise_type
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303752: [coroutines] Improved diagnostics when
unhandled_exception is missing in the… (authored by GorNishanov).
Changed prior to commit:
https://reviews.llvm.org/D33481?vs=100037&id=100086#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33481
Files:
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/SemaCXX/coroutine-unhandled_exception-warning.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp
Index: cfe/trunk/test/SemaCXX/coroutines.cpp
===
--- cfe/trunk/test/SemaCXX/coroutines.cpp
+++ cfe/trunk/test/SemaCXX/coroutines.cpp
@@ -540,7 +540,7 @@
}
template coro bad_implicit_return_dependent(bad_promise_6); //
expected-note {{in instantiation}}
-struct bad_promise_7 {
+struct bad_promise_7 { // expected-note 2 {{defined here}}
coro get_return_object();
suspend_always initial_suspend();
suspend_always final_suspend();
Index: cfe/trunk/test/SemaCXX/coroutine-unhandled_exception-warning.cpp
===
--- cfe/trunk/test/SemaCXX/coroutine-unhandled_exception-warning.cpp
+++ cfe/trunk/test/SemaCXX/coroutine-unhandled_exception-warning.cpp
@@ -16,7 +16,11 @@
using std::experimental::suspend_always;
using std::experimental::suspend_never;
+#ifndef DISABLE_WARNING
+struct promise_void { // expected-note {{defined here}}
+#else
struct promise_void {
+#endif
void get_return_object();
suspend_always initial_suspend();
suspend_always final_suspend();
Index: cfe/trunk/lib/Sema/SemaCoroutine.cpp
===
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp
@@ -120,8 +120,7 @@
return PromiseType;
}
-/// Look up the std::coroutine_traits<...>::promise_type for the given
-/// function type.
+/// Look up the std::experimental::coroutine_handle.
static QualType lookupCoroutineHandleType(Sema &S, QualType PromiseType,
SourceLocation Loc) {
if (PromiseType.isNull())
@@ -729,8 +728,7 @@
}
if (isa(Body)) {
-// FIXME(EricWF): Nothing todo. the body is already a transformed coroutine
-// body statement.
+// Nothing todo. the body is already a transformed coroutine body
statement.
return;
}
@@ -1030,6 +1028,8 @@
: diag::
warn_coroutine_promise_unhandled_exception_required_with_exceptions;
S.Diag(Loc, DiagID) << PromiseRecordDecl;
+S.Diag(PromiseRecordDecl->getLocation(), diag::note_defined_here)
+<< PromiseRecordDecl;
return !RequireUnhandledException;
}
Index: cfe/trunk/test/SemaCXX/coroutines.cpp
===
--- cfe/trunk/test/SemaCXX/coroutines.cpp
+++ cfe/trunk/test/SemaCXX/coroutines.cpp
@@ -540,7 +540,7 @@
}
template coro bad_implicit_return_dependent(bad_promise_6); // expected-note {{in instantiation}}
-struct bad_promise_7 {
+struct bad_promise_7 { // expected-note 2 {{defined here}}
coro get_return_object();
suspend_always initial_suspend();
suspend_always final_suspend();
Index: cfe/trunk/test/SemaCXX/coroutine-unhandled_exception-warning.cpp
===
--- cfe/trunk/test/SemaCXX/coroutine-unhandled_exception-warning.cpp
+++ cfe/trunk/test/SemaCXX/coroutine-unhandled_exception-warning.cpp
@@ -16,7 +16,11 @@
using std::experimental::suspend_always;
using std::experimental::suspend_never;
+#ifndef DISABLE_WARNING
+struct promise_void { // expected-note {{defined here}}
+#else
struct promise_void {
+#endif
void get_return_object();
suspend_always initial_suspend();
suspend_always final_suspend();
Index: cfe/trunk/lib/Sema/SemaCoroutine.cpp
===
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp
@@ -120,8 +120,7 @@
return PromiseType;
}
-/// Look up the std::coroutine_traits<...>::promise_type for the given
-/// function type.
+/// Look up the std::experimental::coroutine_handle.
static QualType lookupCoroutineHandleType(Sema &S, QualType PromiseType,
SourceLocation Loc) {
if (PromiseType.isNull())
@@ -729,8 +728,7 @@
}
if (isa(Body)) {
-// FIXME(EricWF): Nothing todo. the body is already a transformed coroutine
-// body statement.
+// Nothing todo. the body is already a transformed coroutine body statement.
return;
}
@@ -1030,6 +1028,8 @@
: diag::
warn_coroutine_promise_unhandled_exception_required_with_exceptions;
S.Diag(Loc, DiagID) << PromiseRecordDecl;
+S.Diag(PromiseRecordDecl->getLocation(), diag::note_defined_here)
+<< PromiseRecordDecl;
return !RequireUnhandledException;
}
[PATCH] D33498: [coroutines] Make generic lambda coroutines work
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D33498 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33498: [coroutines] Make generic lambda coroutines work
GorNishanov created this revision.
Herald added a subscriber: EricWF.
1. Coroutine cannot be constexpr (added a check in SemaLambda.cpp not to mark
coroutine as constexpr)
2. TransformCoroutineBodyStmt should transform ResultDecl and ReturnStmt
https://reviews.llvm.org/D33498
Files:
lib/Sema/SemaLambda.cpp
lib/Sema/TreeTransform.h
test/CodeGenCoroutines/coro-lambda.cpp
Index: test/CodeGenCoroutines/coro-lambda.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-lambda.cpp
@@ -0,0 +1,58 @@
+// Verify that we synthesized the coroutine for a lambda inside of a function
template.
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu
-emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes |
FileCheck %s
+
+namespace std::experimental {
+template struct coroutine_traits {
+ using promise_type = typename R::promise_type;
+};
+
+template struct coroutine_handle;
+template <> struct coroutine_handle {
+ static coroutine_handle from_address(void *) noexcept;
+ coroutine_handle() = default;
+ template
+ coroutine_handle(coroutine_handle) noexcept;
+};
+template struct coroutine_handle : coroutine_handle {
+ coroutine_handle() = default;
+ static coroutine_handle from_address(void *) noexcept;
+};
+}
+
+struct suspend_always {
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
+};
+
+struct Task {
+ struct promise_type {
+Task get_return_object();
+void return_void() {}
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void unhandled_exception() noexcept;
+ };
+};
+
+template auto SyncAwait(_AwrT &&A) {
+ if (!A.await_ready()) {
+auto AwaitAsync = [&]() -> Task {
+ try { (void)(co_await A); } catch (...) {}
+};
+Task t = AwaitAsync();
+ }
+ return A.await_resume();
+}
+
+void f() {
+ suspend_always test;
+ SyncAwait(test);
+}
+
+// Verify that we synthesized the coroutine for a lambda inside SyncAwait
+// CHECK-LABEL: define linkonce_odr void
@_ZZ9SyncAwaitIR14suspend_alwaysEDaOT_ENKUlvE_clEv(
+// CHECK: alloca %"struct.Task::promise_type"
+// CHECK: call token @llvm.coro.id(
+// CHECK: call i8 @llvm.coro.suspend(
+// CHECK: call i1 @llvm.coro.end(
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -6945,6 +6945,19 @@
if (DeallocRes.isInvalid())
return StmtError();
Builder.Deallocate = DeallocRes.get();
+
+assert(S->getResultDecl() && "ResultDecl must already be built");
+StmtResult ResultDecl = getDerived().TransformStmt(S->getResultDecl());
+if (ResultDecl.isInvalid())
+ return StmtError();
+Builder.ResultDecl = ResultDecl.get();
+
+if (auto *ReturnStmt = S->getReturnStmt()) {
+ StmtResult Res = getDerived().TransformStmt(ReturnStmt);
+ if (Res.isInvalid())
+return StmtError();
+ Builder.ReturnStmt = Res.get();
+}
}
return getDerived().RebuildCoroutineBodyStmt(Builder);
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -1591,6 +1591,7 @@
// its constexpr-ness, supressing diagnostics while doing so.
if (getLangOpts().CPlusPlus1z && !CallOperator->isInvalidDecl() &&
!CallOperator->isConstexpr() &&
+ !isa(CallOperator->getBody()) &&
!Class->getDeclContext()->isDependentContext()) {
TentativeAnalysisScope DiagnosticScopeGuard(*this);
CallOperator->setConstexpr(
Index: test/CodeGenCoroutines/coro-lambda.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-lambda.cpp
@@ -0,0 +1,58 @@
+// Verify that we synthesized the coroutine for a lambda inside of a function template.
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s
+
+namespace std::experimental {
+template struct coroutine_traits {
+ using promise_type = typename R::promise_type;
+};
+
+template struct coroutine_handle;
+template <> struct coroutine_handle {
+ static coroutine_handle from_address(void *) noexcept;
+ coroutine_handle() = default;
+ template
+ coroutine_handle(coroutine_handle) noexcept;
+};
+template struct coroutine_handle : coroutine_handle {
+ coroutine_handle() = default;
+ static coroutine_handle from_address(void *) noexcept;
+};
+}
+
+struct suspend_always {
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
+};
+
+struct Task {
+ struct promise_type {
+Task get_return_object();
+void return_void() {}
+suspend_always initial_suspen
[PATCH] D33498: [coroutines] Make generic lambda coroutines work
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303764: [coroutines] Make generic lambda coroutines work
(authored by GorNishanov).
Changed prior to commit:
https://reviews.llvm.org/D33498?vs=100097&id=100102#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33498
Files:
cfe/trunk/lib/Sema/SemaLambda.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/test/CodeGenCoroutines/coro-lambda.cpp
Index: cfe/trunk/test/CodeGenCoroutines/coro-lambda.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-lambda.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-lambda.cpp
@@ -0,0 +1,58 @@
+// Verify that we synthesized the coroutine for a lambda inside of a function
template.
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu
-emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes |
FileCheck %s
+
+namespace std::experimental {
+template struct coroutine_traits {
+ using promise_type = typename R::promise_type;
+};
+
+template struct coroutine_handle;
+template <> struct coroutine_handle {
+ static coroutine_handle from_address(void *) noexcept;
+ coroutine_handle() = default;
+ template
+ coroutine_handle(coroutine_handle) noexcept;
+};
+template struct coroutine_handle : coroutine_handle {
+ coroutine_handle() = default;
+ static coroutine_handle from_address(void *) noexcept;
+};
+}
+
+struct suspend_always {
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
+};
+
+struct Task {
+ struct promise_type {
+Task get_return_object();
+void return_void() {}
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void unhandled_exception() noexcept;
+ };
+};
+
+template auto SyncAwait(_AwrT &&A) {
+ if (!A.await_ready()) {
+auto AwaitAsync = [&]() -> Task {
+ try { (void)(co_await A); } catch (...) {}
+};
+Task t = AwaitAsync();
+ }
+ return A.await_resume();
+}
+
+void f() {
+ suspend_always test;
+ SyncAwait(test);
+}
+
+// Verify that we synthesized the coroutine for a lambda inside SyncAwait
+// CHECK-LABEL: define linkonce_odr void
@_ZZ9SyncAwaitIR14suspend_alwaysEDaOT_ENKUlvE_clEv(
+// CHECK: alloca %"struct.Task::promise_type"
+// CHECK: call token @llvm.coro.id(
+// CHECK: call i8 @llvm.coro.suspend(
+// CHECK: call i1 @llvm.coro.end(
Index: cfe/trunk/lib/Sema/TreeTransform.h
===
--- cfe/trunk/lib/Sema/TreeTransform.h
+++ cfe/trunk/lib/Sema/TreeTransform.h
@@ -6945,6 +6945,19 @@
if (DeallocRes.isInvalid())
return StmtError();
Builder.Deallocate = DeallocRes.get();
+
+assert(S->getResultDecl() && "ResultDecl must already be built");
+StmtResult ResultDecl = getDerived().TransformStmt(S->getResultDecl());
+if (ResultDecl.isInvalid())
+ return StmtError();
+Builder.ResultDecl = ResultDecl.get();
+
+if (auto *ReturnStmt = S->getReturnStmt()) {
+ StmtResult Res = getDerived().TransformStmt(ReturnStmt);
+ if (Res.isInvalid())
+return StmtError();
+ Builder.ReturnStmt = Res.get();
+}
}
return getDerived().RebuildCoroutineBodyStmt(Builder);
Index: cfe/trunk/lib/Sema/SemaLambda.cpp
===
--- cfe/trunk/lib/Sema/SemaLambda.cpp
+++ cfe/trunk/lib/Sema/SemaLambda.cpp
@@ -1591,6 +1591,7 @@
// its constexpr-ness, supressing diagnostics while doing so.
if (getLangOpts().CPlusPlus1z && !CallOperator->isInvalidDecl() &&
!CallOperator->isConstexpr() &&
+ !isa(CallOperator->getBody()) &&
!Class->getDeclContext()->isDependentContext()) {
TentativeAnalysisScope DiagnosticScopeGuard(*this);
CallOperator->setConstexpr(
Index: cfe/trunk/test/CodeGenCoroutines/coro-lambda.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-lambda.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-lambda.cpp
@@ -0,0 +1,58 @@
+// Verify that we synthesized the coroutine for a lambda inside of a function template.
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s
+
+namespace std::experimental {
+template struct coroutine_traits {
+ using promise_type = typename R::promise_type;
+};
+
+template struct coroutine_handle;
+template <> struct coroutine_handle {
+ static coroutine_handle from_address(void *) noexcept;
+ coroutine_handle() = default;
+ template
+ coroutine_handle(coroutine_handle) noexcept;
+};
+template struct coroutine_handle : coroutine_handle {
+ coroutine_handle() = default;
+ static coroutine_handle from_address(void *) noexcept;
+};
+}
+
+struct suspend_always {
+ bool await_ready() noexcep
[PATCH] D33507: [coroutines] Add support for coroutines with non-scalar parameters
GorNishanov created this revision.
Simple types like int are handled by LLVM Coroutines just fine.
But for non-scalar parameters we need to create copies of those parameters in
the coroutine frame and make all uses of those parameters to refer to parameter
copies.
https://reviews.llvm.org/D33507
Files:
lib/CodeGen/CGCoroutine.cpp
lib/Sema/SemaCoroutine.cpp
test/CodeGenCoroutines/coro-params.cpp
Index: test/CodeGenCoroutines/coro-params.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -0,0 +1,95 @@
+// Verifies that parameters are copied with move constructors
+// Verifies that parameter copies are destroyed
+// Vefifies that parameter copies are used in the body of the coroutine
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes -fexceptions | FileCheck %s
+
+namespace std::experimental {
+template struct coroutine_traits;
+
+template struct coroutine_handle {
+ coroutine_handle() = default;
+ static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+ static coroutine_handle from_address(void *) noexcept;
+ coroutine_handle() = default;
+ template
+ coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
+};
+
+template struct std::experimental::coroutine_traits {
+ struct promise_type {
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+promise_type();
+~promise_type() noexcept;
+void unhandled_exception() noexcept;
+ };
+};
+
+// TODO: Not supported yet
+struct CopyOnly {
+ int val;
+ CopyOnly(const CopyOnly&) noexcept;
+ CopyOnly(CopyOnly&&) = delete;
+ ~CopyOnly();
+};
+
+struct MoveOnly {
+ int val;
+ MoveOnly(const MoveOnly&) = delete;
+ MoveOnly(MoveOnly&&) noexcept;
+ ~MoveOnly();
+};
+
+struct MoveAndCopy {
+ int val;
+ MoveAndCopy(const MoveAndCopy&)noexcept;
+ MoveAndCopy(MoveAndCopy&&) noexcept;
+ ~MoveAndCopy();
+};
+
+void consume(int,int,int) noexcept;
+
+// TODO: Add support for CopyOnly params
+// CHECK: define void @_Z1fi8MoveOnly11MoveAndCopy(i32 %val, %struct.MoveOnly* %[[MoParam:.+]], %struct.MoveAndCopy* %[[McParam:.+]]) #0 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*
+void f(int val, MoveOnly moParam, MoveAndCopy mcParam) {
+ // CHECK: %[[MoCopy:.+]] = alloca %struct.MoveOnly
+ // CHECK: %[[McCopy:.+]] = alloca %struct.MoveAndCopy
+ // CHECK: store i32 %val, i32* %[[ValAddr:.+]]
+
+ // CHECK: call i8* @llvm.coro.begin(
+ // CHECK-NEXT: call void @_ZN8MoveOnlyC1EOS_(%struct.MoveOnly* %[[MoCopy]], %struct.MoveOnly* dereferenceable(4) %[[MoParam]])
+ // CHECK-NEXT: call void @_ZN11MoveAndCopyC1EOS_(%struct.MoveAndCopy* %[[McCopy]], %struct.MoveAndCopy* dereferenceable(4) %[[McParam]]) #
+ // CHECK-NEXT: invoke void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeC1Ev(
+
+ // CHECK: call void @_ZN14suspend_always12await_resumeEv(
+ // CHECK: %[[IntParam:.+]] = load i32, i32* %val.addr
+ // CHECK: %[[MoGep:.+]] = getelementptr inbounds %struct.MoveOnly, %struct.MoveOnly* %[[MoCopy]], i32 0, i32 0
+ // CHECK: %[[MoVal:.+]] = load i32, i32* %[[MoGep]]
+ // CHECK: %[[McGep:.+]] = getelementptr inbounds %struct.MoveAndCopy, %struct.MoveAndCopy* %[[McCopy]], i32 0, i32 0
+ // CHECK: %[[McVal:.+]] = load i32, i32* %[[McGep]]
+ // CHECK: call void @_Z7consumeiii(i32 %[[IntParam]], i32 %[[MoVal]], i32 %[[McVal]])
+
+ consume(val, moParam.val, mcParam.val);
+ co_return;
+
+ // Skip to final suspend:
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_type13final_suspendEv(
+ // CHECK: call void @_ZN14suspend_always12await_resumeEv(
+
+ // Destroy promise, then parameter copies:
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeD1Ev(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise) #2
+ // CHECK-NEXT: call void @_ZN11MoveAndCopyD1Ev(%struct.MoveAndCopy* %[[McCopy]])
+ // CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(%struct.MoveOnly* %[[MoCopy]]
+ // CHECK-NEXT: call i8* @llvm.coro.free(
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -1160,8 +1160,68 @@
return true;
}
+// Create a static_cast\(expr).
+static Expr *castForMoving(Sema &S, Expr *E, QualType T = QualType()) {
+ if (T.isNull())
+T = E->getType();
+ QualType TargetType = S.BuildReferenceType(
+ T, /*SpelledAsLValue*/ false, SourceLocation(), DeclarationName());
+ SourceLocation ExprLoc = E->getLocStart
[PATCH] D33507: [coroutines] Add support for coroutines with non-scalar parameters
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D33507 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33507: [coroutines] Add support for coroutines with non-scalar parameters
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303803: [coroutines] Add support for coroutines with
non-scalar parameters (authored by GorNishanov).
Changed prior to commit:
https://reviews.llvm.org/D33507?vs=100132&id=100152#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33507
Files:
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
Index: cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
@@ -0,0 +1,95 @@
+// Verifies that parameters are copied with move constructors
+// Verifies that parameter copies are destroyed
+// Vefifies that parameter copies are used in the body of the coroutine
+// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes -fexceptions | FileCheck %s
+
+namespace std::experimental {
+template struct coroutine_traits;
+
+template struct coroutine_handle {
+ coroutine_handle() = default;
+ static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle {
+ static coroutine_handle from_address(void *) noexcept;
+ coroutine_handle() = default;
+ template
+ coroutine_handle(coroutine_handle) noexcept;
+};
+}
+
+struct suspend_always {
+ bool await_ready() noexcept;
+ void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
+};
+
+template struct std::experimental::coroutine_traits {
+ struct promise_type {
+void get_return_object() noexcept;
+suspend_always initial_suspend() noexcept;
+suspend_always final_suspend() noexcept;
+void return_void() noexcept;
+promise_type();
+~promise_type() noexcept;
+void unhandled_exception() noexcept;
+ };
+};
+
+// TODO: Not supported yet
+struct CopyOnly {
+ int val;
+ CopyOnly(const CopyOnly&) noexcept;
+ CopyOnly(CopyOnly&&) = delete;
+ ~CopyOnly();
+};
+
+struct MoveOnly {
+ int val;
+ MoveOnly(const MoveOnly&) = delete;
+ MoveOnly(MoveOnly&&) noexcept;
+ ~MoveOnly();
+};
+
+struct MoveAndCopy {
+ int val;
+ MoveAndCopy(const MoveAndCopy&)noexcept;
+ MoveAndCopy(MoveAndCopy&&) noexcept;
+ ~MoveAndCopy();
+};
+
+void consume(int,int,int) noexcept;
+
+// TODO: Add support for CopyOnly params
+// CHECK: define void @_Z1fi8MoveOnly11MoveAndCopy(i32 %val, %struct.MoveOnly* %[[MoParam:.+]], %struct.MoveAndCopy* %[[McParam:.+]]) #0 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*
+void f(int val, MoveOnly moParam, MoveAndCopy mcParam) {
+ // CHECK: %[[MoCopy:.+]] = alloca %struct.MoveOnly
+ // CHECK: %[[McCopy:.+]] = alloca %struct.MoveAndCopy
+ // CHECK: store i32 %val, i32* %[[ValAddr:.+]]
+
+ // CHECK: call i8* @llvm.coro.begin(
+ // CHECK-NEXT: call void @_ZN8MoveOnlyC1EOS_(%struct.MoveOnly* %[[MoCopy]], %struct.MoveOnly* dereferenceable(4) %[[MoParam]])
+ // CHECK-NEXT: call void @_ZN11MoveAndCopyC1EOS_(%struct.MoveAndCopy* %[[McCopy]], %struct.MoveAndCopy* dereferenceable(4) %[[McParam]]) #
+ // CHECK-NEXT: invoke void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeC1Ev(
+
+ // CHECK: call void @_ZN14suspend_always12await_resumeEv(
+ // CHECK: %[[IntParam:.+]] = load i32, i32* %val.addr
+ // CHECK: %[[MoGep:.+]] = getelementptr inbounds %struct.MoveOnly, %struct.MoveOnly* %[[MoCopy]], i32 0, i32 0
+ // CHECK: %[[MoVal:.+]] = load i32, i32* %[[MoGep]]
+ // CHECK: %[[McGep:.+]] = getelementptr inbounds %struct.MoveAndCopy, %struct.MoveAndCopy* %[[McCopy]], i32 0, i32 0
+ // CHECK: %[[McVal:.+]] = load i32, i32* %[[McGep]]
+ // CHECK: call void @_Z7consumeiii(i32 %[[IntParam]], i32 %[[MoVal]], i32 %[[McVal]])
+
+ consume(val, moParam.val, mcParam.val);
+ co_return;
+
+ // Skip to final suspend:
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_type13final_suspendEv(
+ // CHECK: call void @_ZN14suspend_always12await_resumeEv(
+
+ // Destroy promise, then parameter copies:
+ // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJvi8MoveOnly11MoveAndCopyEE12promise_typeD1Ev(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise) #2
+ // CHECK-NEXT: call void @_ZN11MoveAndCopyD1Ev(%struct.MoveAndCopy* %[[McCopy]])
+ // CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(%struct.MoveOnly* %[[MoCopy]]
+ // CHECK-NEXT: call i8* @llvm.coro.free(
+}
Index: cfe/trunk/lib/Sema/SemaCoroutine.cpp
===
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp
@@ -1160,8 +1160,68 @@
return true;
}
+// Create a static_cast\(expr).
+static Expr *castForMoving(Sema &S, Expr *E, QualType T = QualType()) {
+ if (T.isNull())
+T = E->getType();
+ QualType
[PATCH] D33532: [coroutines] Fix fallthrough diagnostics for coroutines
GorNishanov added inline comments.
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:378
const Stmt *S = CS.getStmt();
-if ((isa(S) && !IsCoroutine) || isa(S)) {
+if (isa(S) || isa(S)) {
HasLiveReturn = true;
Is this check no longer needed because of the changes to
AnalysisDeclContext.cpp?
At some point it was needed because we were emitting "return gro" to produce
the immediate return value from the coroutine, but, we wanted to flag the case
when the user forgot to add "co_return"
https://reviews.llvm.org/D33532
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33532: [coroutines] Fix fallthrough diagnostics for coroutines
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D33532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33534: [coroutines] Diagnose when promise types fail to declare either return_void or return_value.
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D33534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33536: [coroutines] Bump __cpp_coroutines version
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D33536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33624: [coroutines] Mark cxx_status.html of Coroutines TS as (SVN)
GorNishanov created this revision. It is time! https://reviews.llvm.org/D33624 Files: www/cxx_status.html Index: www/cxx_status.html === --- www/cxx_status.html +++ www/cxx_status.html @@ -831,9 +831,9 @@ [DRAFT TS] Coroutines - http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0057r7.pdf";>P0057R7 - - WIP + https://isocpp.org/files/papers/N4663.pdf";>N4663 + -fcoroutines-ts-stdlib=libc++ + SVN [TS] Library Fundamentals, Version 1 (invocation type traits) Index: www/cxx_status.html === --- www/cxx_status.html +++ www/cxx_status.html @@ -831,9 +831,9 @@ [DRAFT TS] Coroutines - http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0057r7.pdf";>P0057R7 - - WIP + https://isocpp.org/files/papers/N4663.pdf";>N4663 + -fcoroutines-ts-stdlib=libc++ + SVN [TS] Library Fundamentals, Version 1 (invocation type traits) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33624: [coroutines] Mark cxx_status.html of Coroutines TS as (SVN)
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. Drumroll.. Approved! https://reviews.llvm.org/D33624 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33624: [coroutines] Mark cxx_status.html of Coroutines TS as (SVN)
This revision was automatically updated to reflect the committed changes. Closed by commit rL304081: [coroutines] Mark cxx_status.html of Coroutines TS as (SVN) (authored by GorNishanov). Changed prior to commit: https://reviews.llvm.org/D33624?vs=100538&id=100539#toc Repository: rL LLVM https://reviews.llvm.org/D33624 Files: cfe/trunk/www/cxx_status.html Index: cfe/trunk/www/cxx_status.html === --- cfe/trunk/www/cxx_status.html +++ cfe/trunk/www/cxx_status.html @@ -831,9 +831,9 @@ [DRAFT TS] Coroutines - http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0057r7.pdf";>P0057R7 - - WIP + https://isocpp.org/files/papers/N4663.pdf";>N4663 + -fcoroutines-ts-stdlib=libc++ + SVN [TS] Library Fundamentals, Version 1 (invocation type traits) Index: cfe/trunk/www/cxx_status.html === --- cfe/trunk/www/cxx_status.html +++ cfe/trunk/www/cxx_status.html @@ -831,9 +831,9 @@ [DRAFT TS] Coroutines - http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0057r7.pdf";>P0057R7 - - WIP + https://isocpp.org/files/papers/N4663.pdf";>N4663 + -fcoroutines-ts-stdlib=libc++ + SVN [TS] Library Fundamentals, Version 1 (invocation type traits) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33625: [coroutines] Diagnose invalid result types for `await_resume` and `await_suspend` and add missing conversions.
GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: test/SemaCXX/coroutines.cpp:842
+struct bad_await_suspend_return {
+ bool await_ready();
+ // expected-error@+1 {{the return type of 'await_suspend' is required to be
'void' or 'bool' (have 'char')}}
For test completeness, I would have a positive case for something like a
std::true_type returning await_ready
https://reviews.llvm.org/D33625
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33625: [coroutines] Diagnose invalid result types for `await_resume` and `await_suspend` and add missing conversions.
GorNishanov added inline comments.
Comment at: lib/Sema/SemaCoroutine.cpp:324
struct ReadySuspendResumeResult {
+ enum AwaitCallType { ACT_Ready, ACT_Suspend, ACT_Resume };
Expr *Results[3];
enum class?
https://reviews.llvm.org/D33625
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33632: [coroutines] cxx_status.hml: add non-breaking hyphen
GorNishanov created this revision. Herald added a subscriber: EricWF. https://reviews.llvm.org/D33632 Files: www/cxx_status.html Index: www/cxx_status.html === --- www/cxx_status.html +++ www/cxx_status.html @@ -832,7 +832,7 @@ [DRAFT TS] Coroutines https://isocpp.org/files/papers/N4663.pdf";>N4663 - -fcoroutines-ts-stdlib=libc++ + -fcoroutines‑ts-stdlib=libc++ SVN Index: www/cxx_status.html === --- www/cxx_status.html +++ www/cxx_status.html @@ -832,7 +832,7 @@ [DRAFT TS] Coroutines https://isocpp.org/files/papers/N4663.pdf";>N4663 - -fcoroutines-ts-stdlib=libc++ + -fcoroutines‑ts-stdlib=libc++ SVN ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33632: [coroutines] www/cxx_status.html: add non-breaking hyphen
This revision was automatically updated to reflect the committed changes. Closed by commit rL304091: [coroutines] www/cxx_status.html: add non-breaking hyphen (authored by GorNishanov). Changed prior to commit: https://reviews.llvm.org/D33632?vs=100552&id=100553#toc Repository: rL LLVM https://reviews.llvm.org/D33632 Files: cfe/trunk/www/cxx_status.html Index: cfe/trunk/www/cxx_status.html === --- cfe/trunk/www/cxx_status.html +++ cfe/trunk/www/cxx_status.html @@ -832,7 +832,7 @@ [DRAFT TS] Coroutines https://isocpp.org/files/papers/N4663.pdf";>N4663 - -fcoroutines-ts-stdlib=libc++ + -fcoroutines‑ts-stdlib=libc++ SVN Index: cfe/trunk/www/cxx_status.html === --- cfe/trunk/www/cxx_status.html +++ cfe/trunk/www/cxx_status.html @@ -832,7 +832,7 @@ [DRAFT TS] Coroutines https://isocpp.org/files/papers/N4663.pdf";>N4663 - -fcoroutines-ts-stdlib=libc++ + -fcoroutines‑ts-stdlib=libc++ SVN ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33632: [coroutines] www/cxx_status.html: add non-breaking hyphen
GorNishanov updated this revision to Diff 100555. GorNishanov added a comment. one more non-breaking hyphen to make it pretty in chrome. In edge it is perfect already, naturally https://reviews.llvm.org/D33632 Files: www/cxx_status.html Index: www/cxx_status.html === --- www/cxx_status.html +++ www/cxx_status.html @@ -832,7 +832,7 @@ [DRAFT TS] Coroutines https://isocpp.org/files/papers/N4663.pdf";>N4663 - -fcoroutines‑ts-stdlib=libc++ + ‑fcoroutines‑ts-stdlib=libc++ SVN Index: www/cxx_status.html === --- www/cxx_status.html +++ www/cxx_status.html @@ -832,7 +832,7 @@ [DRAFT TS] Coroutines https://isocpp.org/files/papers/N4663.pdf";>N4663 - -fcoroutines‑ts-stdlib=libc++ + ‑fcoroutines‑ts-stdlib=libc++ SVN ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33632: [coroutines] www/cxx_status.html: add non-breaking hyphen
This revision was automatically updated to reflect the committed changes. Closed by commit rL304092: [coroutines] www/cxx_status.html: add non-breaking hyphen (authored by GorNishanov). Changed prior to commit: https://reviews.llvm.org/D33632?vs=100555&id=100556#toc Repository: rL LLVM https://reviews.llvm.org/D33632 Files: cfe/trunk/www/cxx_status.html Index: cfe/trunk/www/cxx_status.html === --- cfe/trunk/www/cxx_status.html +++ cfe/trunk/www/cxx_status.html @@ -832,7 +832,7 @@ [DRAFT TS] Coroutines https://isocpp.org/files/papers/N4663.pdf";>N4663 - -fcoroutines‑ts-stdlib=libc++ + ‑fcoroutines‑ts-stdlib=libc++ SVN Index: cfe/trunk/www/cxx_status.html === --- cfe/trunk/www/cxx_status.html +++ cfe/trunk/www/cxx_status.html @@ -832,7 +832,7 @@ [DRAFT TS] Coroutines https://isocpp.org/files/papers/N4663.pdf";>N4663 - -fcoroutines‑ts-stdlib=libc++ + ‑fcoroutines‑ts-stdlib=libc++ SVN ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33663: CGCleanup: Use correct insertion point for AllocaInst
GorNishanov created this revision.
This is a follow up for the https://reviews.llvm.org/rL297084 which made sure
that dominance is preserved during expression cleanup emission.
We ran into a case where dominance fixup code was inserting the store in the
middle of allocas.
%x = alloca i32, align 4
store i32* %x, i32** %tmp.exprcleanup, align 4 ; <= HERE
%ref.tmp3 = alloca %struct.A, align 1
%agg.tmp5 = alloca
%"struct.std::experimental::coroutines_v1::coroutine_handle.0", align 4
%tmp.exprcleanup = alloca i32*, align 4
%allocapt = bitcast i32 undef to i32
store i32 %0, i32* %.addr, align 4
This fix makes sure that if we are fixing up domination for an alloca
instruction we will do it after the AllocaInsertPt
https://reviews.llvm.org/D33663
Files:
lib/CodeGen/CGCleanup.cpp
test/CodeGenCoroutines/coro-await-domination.cpp
Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14
-emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+ struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+ };
+};
+
+struct A {
+ ~A();
+ bool await_ready();
+ int await_resume();
+ template void await_suspend(F);
+};
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+// %tmp.exprcleanup = alloca i32*, align 8
+// store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+ int x = 42;
+ x = co_await A{};
+}
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -455,6 +455,8 @@
llvm::BasicBlock::iterator InsertBefore;
if (auto *Invoke = dyn_cast(Inst))
InsertBefore = Invoke->getNormalDest()->getFirstInsertionPt();
+else if (isa(Inst))
+ InsertBefore = std::next(AllocaInsertPt->getIterator());
else
InsertBefore = std::next(Inst->getIterator());
CGBuilderTy(CGM, &*InsertBefore).CreateStore(Inst, Tmp);
Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+ struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+ };
+};
+
+struct A {
+ ~A();
+ bool await_ready();
+ int await_resume();
+ template void await_suspend(F);
+};
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+// %tmp.exprcleanup = alloca i32*, align 8
+// store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+ int x = 42;
+ x = co_await A{};
+}
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -455,6 +455,8 @@
llvm::BasicBlock::iterator InsertBefore;
if (auto *Invoke = dyn_cast(Inst))
InsertBefore = Invoke->getNormalDest()->getFirstInsertionPt();
+else if (isa(Inst))
+ InsertBefore = std::next(AllocaInsertPt->getIterator());
else
InsertBefore = std::next(Inst->getIterator());
CGBuilderTy(CGM, &*InsertBefore).CreateStore(Inst, Tmp);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33660: [coroutines] Fix assertion during -Wuninitialized analysis
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LBTM (Looks brilliant to me) https://reviews.llvm.org/D33660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33663: CGCleanup: Use correct insertion point for AllocaInst
GorNishanov updated this revision to Diff 100889.
GorNishanov added a comment.
Make sure that we use AllocaInsertPt for only static allocas in the entry block
https://reviews.llvm.org/D33663
Files:
lib/CodeGen/CGCleanup.cpp
test/CodeGenCoroutines/coro-await-domination.cpp
Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14
-emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+ struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+ };
+};
+
+struct A {
+ ~A();
+ bool await_ready();
+ int await_resume();
+ template void await_suspend(F);
+};
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+// %tmp.exprcleanup = alloca i32*, align 8
+// store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+ int x = 42;
+ x = co_await A{};
+}
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -416,6 +416,12 @@
EHStack.popNullFixups();
}
+static bool isStaticAlloca(llvm::Value* V) {
+ if (auto *AI = dyn_cast(V))
+return AI->isStaticAlloca();
+ return false;
+}
+
/// Pops cleanup blocks until the given savepoint is reached.
void CodeGenFunction::PopCleanupBlocks(
EHScopeStack::stable_iterator Old,
@@ -455,6 +461,8 @@
llvm::BasicBlock::iterator InsertBefore;
if (auto *Invoke = dyn_cast(Inst))
InsertBefore = Invoke->getNormalDest()->getFirstInsertionPt();
+else if (isStaticAlloca(Inst))
+ InsertBefore = std::next(AllocaInsertPt->getIterator());
else
InsertBefore = std::next(Inst->getIterator());
CGBuilderTy(CGM, &*InsertBefore).CreateStore(Inst, Tmp);
Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+ struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+ };
+};
+
+struct A {
+ ~A();
+ bool await_ready();
+ int await_resume();
+ template void await_suspend(F);
+};
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+// %tmp.exprcleanup = alloca i32*, align 8
+// store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+ int x = 42;
+ x = co_await A{};
+}
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -416,6 +416,12 @@
EHStack.popNullFixups();
}
+static bool isStaticAlloca(llvm::Value* V) {
+ if (auto *AI = dyn_cast(V))
+return AI->isStaticAlloca();
+ return false;
+}
+
/// Pops cleanup blocks until the given savepoint is reached.
void CodeGenFunction::PopCleanupBlocks(
EHScopeStack::stable_iterator Old,
@@ -455,6 +461,8 @@
llvm::BasicBlock::iterator InsertBefore;
if (auto *Invoke = dyn_cast(Inst))
InsertBefore = Invoke->getNormalDest()->getFirstInsertionPt();
+else if (isStaticAlloca(Inst))
+ InsertBefore = std::next(AllocaInsertPt->getIterator());
else
InsertBefore = std::next(Inst->getIterator());
CGBuilderTy(CGM, &*InsertBefore).CreateStore(Inst, Tmp);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33663: CGCleanup: Use correct insertion point for AllocaInst
GorNishanov added inline comments.
Comment at: lib/CodeGen/CGCleanup.cpp:458
InsertBefore = Invoke->getNormalDest()->getFirstInsertionPt();
+else if (isa(Inst))
+ InsertBefore = std::next(AllocaInsertPt->getIterator());
rnk wrote:
> This doesn't seem right, `Inst` could be a dynamic alloca. If it's static, we
> definitely don't need to store and reload it. All static allocas better be in
> the entry block... You might want to use `isStaticAlloca`, but that still
> feels like we're hacking around some deeper problem.
An extra context. Without the fix:
```
coro f(int) {
int x = co_await A{}; // compiles fine
}
```
```
coro f(int) {
int x = 42;
x = co_await A{}; // does not compile due to broken IR
}
```
https://reviews.llvm.org/D33663
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33733: CGCleanup: No need to do domination fixups for static allocas
GorNishanov created this revision.
This is a follow up for the https://reviews.llvm.org/rL297084 which made sure
that dominance is preserved during expression cleanup emission.
We ran into a case where dominance fixup code was inserting the store in the
middle of allocas.
%x = alloca i32, align 4
store i32* %x, i32** %tmp.exprcleanup, align 4 ; <= HERE
%ref.tmp3 = alloca %struct.A, align 1
%agg.tmp5 = alloca
%"struct.std::experimental::coroutines_v1::coroutine_handle.0", align 4
%tmp.exprcleanup = alloca i32*, align 4
%allocapt = bitcast i32 undef to i32
store i32 %0, i32* %.addr, align 4
This fix make sure that domination fixup is not done static allocas.
(Alternative of https://reviews.llvm.org/D33663)
https://reviews.llvm.org/D33733
Files:
lib/CodeGen/CGCleanup.cpp
test/CodeGenCoroutines/coro-await-domination.cpp
Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14
-emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+ struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+ };
+};
+
+struct A {
+ ~A();
+ bool await_ready();
+ int await_resume();
+ template void await_suspend(F);
+};
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+// %tmp.exprcleanup = alloca i32*, align 8
+// store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+ int x = 42;
+ x = co_await A{};
+}
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -448,6 +448,11 @@
auto *Inst = dyn_cast_or_null(*ReloadedValue);
if (!Inst)
continue;
+// If it is a static alloca, no need to spill.
+if (auto *AI = dyn_cast(Inst))
+ if (AI->isStaticAlloca())
+continue;
+
Address Tmp =
CreateDefaultAlignTempAlloca(Inst->getType(), "tmp.exprcleanup");
Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+ struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+ };
+};
+
+struct A {
+ ~A();
+ bool await_ready();
+ int await_resume();
+ template void await_suspend(F);
+};
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+// %tmp.exprcleanup = alloca i32*, align 8
+// store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+ int x = 42;
+ x = co_await A{};
+}
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -448,6 +448,11 @@
auto *Inst = dyn_cast_or_null(*ReloadedValue);
if (!Inst)
continue;
+// If it is a static alloca, no need to spill.
+if (auto *AI = dyn_cast(Inst))
+ if (AI->isStaticAlloca())
+continue;
+
Address Tmp =
CreateDefaultAlignTempAlloca(Inst->getType(), "tmp.exprcleanup");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33663: CGCleanup: Use correct insertion point for AllocaInst
GorNishanov abandoned this revision. GorNishanov added a comment. Abandoning... Reid fixed the problem with "r304335 - Don't try to spill static allocas when emitting expr cleanups with branches" Thank you Reid for looking into it! https://reviews.llvm.org/D33663 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33750: CGCleanup: (NFC) add a test that used to trigger broken IR
GorNishanov created this revision.
Coroutine related test that used to trigger broken IR prior to r304335.
https://reviews.llvm.org/D33750
Files:
test/CodeGenCoroutines/coro-await-domination.cpp
Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14
-emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+ struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+ };
+};
+
+struct A {
+ ~A();
+ bool await_ready();
+ int await_resume() { return 8; }
+ template void await_suspend(F);
+};
+
+extern "C" void consume(int);
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+// %tmp.exprcleanup = alloca i32*, align 8
+// store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+ int x = 42;
+ x = co_await A{};
+ consume(x);
+}
+
Index: test/CodeGenCoroutines/coro-await-domination.cpp
===
--- /dev/null
+++ test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+ struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+ };
+};
+
+struct A {
+ ~A();
+ bool await_ready();
+ int await_resume() { return 8; }
+ template void await_suspend(F);
+};
+
+extern "C" void consume(int);
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+// %tmp.exprcleanup = alloca i32*, align 8
+// store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+ int x = 42;
+ x = co_await A{};
+ consume(x);
+}
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33750: CGCleanup: (NFC) add another test for r304335 - Don't try to spill static allocas
This revision was automatically updated to reflect the committed changes.
Closed by commit rL304380: CGCleanup: (NFC) add another test for r304335 -
Don't try to spill static… (authored by GorNishanov).
Changed prior to commit:
https://reviews.llvm.org/D33750?vs=100951&id=100955#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33750
Files:
cfe/trunk/test/CodeGenCoroutines/coro-await-domination.cpp
Index: cfe/trunk/test/CodeGenCoroutines/coro-await-domination.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-await-domination.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14
-emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+ struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+ };
+};
+
+struct A {
+ ~A();
+ bool await_ready();
+ int await_resume() { return 8; }
+ template void await_suspend(F);
+};
+
+extern "C" void consume(int);
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+// %tmp.exprcleanup = alloca i32*, align 8
+// store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+ int x = 42;
+ x = co_await A{};
+ consume(x);
+}
+
Index: cfe/trunk/test/CodeGenCoroutines/coro-await-domination.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-await-domination.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-await-domination.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/coroutine.h"
+
+using namespace std::experimental;
+
+struct coro {
+ struct promise_type {
+coro get_return_object();
+suspend_never initial_suspend();
+suspend_never final_suspend();
+void return_void();
+static void unhandled_exception();
+ };
+};
+
+struct A {
+ ~A();
+ bool await_ready();
+ int await_resume() { return 8; }
+ template void await_suspend(F);
+};
+
+extern "C" void consume(int);
+
+// Verifies that domination is properly built during cleanup.
+// Without CGCleanup.cpp fix verifier was reporting:
+// Instruction does not dominate all uses!
+// %tmp.exprcleanup = alloca i32*, align 8
+// store i32* %x, i32** %tmp.exprcleanup, align 8
+
+
+// CHECK-LABEL: f(
+extern "C" coro f(int) {
+ int x = 42;
+ x = co_await A{};
+ consume(x);
+}
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33733: CGCleanup: No need to do domination fixups for static allocas
GorNishanov closed this revision. GorNishanov added a comment. Tested checked in. Thank you for looking into this, Reid! r304380 = 1c0d15a4bf10876bb233e921907e114d52bee82e https://reviews.llvm.org/D33733 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33797: [coroutines] Fix rebuilding of dependent coroutine parameters
GorNishanov created this revision.
We were not handling correctly rebuilding of parameter and were not creating
copies for them.
With this checking, we will be always rebuilding parameter moves in
TreeTransform's TransformCoroutineBodyStmt.
https://reviews.llvm.org/D33797
Files:
lib/Sema/CoroutineStmtBuilder.h
lib/Sema/SemaCoroutine.cpp
lib/Sema/TreeTransform.h
test/CodeGenCoroutines/coro-params.cpp
Index: test/CodeGenCoroutines/coro-params.cpp
===
--- test/CodeGenCoroutines/coro-params.cpp
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -93,3 +93,26 @@
// CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(%struct.MoveOnly* %[[MoCopy]]
// CHECK-NEXT: call i8* @llvm.coro.free(
}
+
+// CHECK-LABEL: void @_Z16dependent_paramsI1AEvT_(%struct.A* %x
+template
+void dependent_params(T x) {
+ // CHECK: %[[x_copy:.+]] = alloca %struct.A
+
+ // CHECK: call i8* @llvm.coro.begin
+ // CHECK-NEXT: call void @_ZN1AC1EOS_(%struct.A* %x1, %struct.A*
dereferenceable(512) %x)
+ // CHECK-NEXT: call void
@_ZNSt12experimental16coroutine_traitsIJv1AEE12promise_typeC1Ev
+
+ co_return;
+}
+
+struct A {
+ int WontFitIntoRegisterForSure[128];
+ A();
+ A(A&&);
+ ~A();
+};
+
+void call_dependent_params() {
+ dependent_params(A{});
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -6959,6 +6959,7 @@
Builder.ReturnStmt = Res.get();
}
}
+ Builder.buildParameterMoves();
return getDerived().RebuildCoroutineBodyStmt(Builder);
}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -822,6 +822,12 @@
return this->IsValid;
}
+bool CoroutineStmtBuilder::buildParameterMoves() {
+ assert(this->IsValid && "coroutine already invalid");
+ assert(this->ParamMoves.empty() && "param moves already built");
+ return this->IsValid = makeParamMoves();
+}
+
bool CoroutineStmtBuilder::buildDependentStatements() {
assert(this->IsValid && "coroutine already invalid");
assert(!this->IsPromiseDependentType &&
Index: lib/Sema/CoroutineStmtBuilder.h
===
--- lib/Sema/CoroutineStmtBuilder.h
+++ lib/Sema/CoroutineStmtBuilder.h
@@ -51,6 +51,9 @@
/// name lookup.
bool buildDependentStatements();
+ /// \brief Build just parameter moves. To use for rebuilding in
TreeTransform.
+ bool buildParameterMoves();
+
bool isInvalid() const { return !this->IsValid; }
private:
Index: test/CodeGenCoroutines/coro-params.cpp
===
--- test/CodeGenCoroutines/coro-params.cpp
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -93,3 +93,26 @@
// CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(%struct.MoveOnly* %[[MoCopy]]
// CHECK-NEXT: call i8* @llvm.coro.free(
}
+
+// CHECK-LABEL: void @_Z16dependent_paramsI1AEvT_(%struct.A* %x
+template
+void dependent_params(T x) {
+ // CHECK: %[[x_copy:.+]] = alloca %struct.A
+
+ // CHECK: call i8* @llvm.coro.begin
+ // CHECK-NEXT: call void @_ZN1AC1EOS_(%struct.A* %x1, %struct.A* dereferenceable(512) %x)
+ // CHECK-NEXT: call void @_ZNSt12experimental16coroutine_traitsIJv1AEE12promise_typeC1Ev
+
+ co_return;
+}
+
+struct A {
+ int WontFitIntoRegisterForSure[128];
+ A();
+ A(A&&);
+ ~A();
+};
+
+void call_dependent_params() {
+ dependent_params(A{});
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -6959,6 +6959,7 @@
Builder.ReturnStmt = Res.get();
}
}
+ Builder.buildParameterMoves();
return getDerived().RebuildCoroutineBodyStmt(Builder);
}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -822,6 +822,12 @@
return this->IsValid;
}
+bool CoroutineStmtBuilder::buildParameterMoves() {
+ assert(this->IsValid && "coroutine already invalid");
+ assert(this->ParamMoves.empty() && "param moves already built");
+ return this->IsValid = makeParamMoves();
+}
+
bool CoroutineStmtBuilder::buildDependentStatements() {
assert(this->IsValid && "coroutine already invalid");
assert(!this->IsPromiseDependentType &&
Index: lib/Sema/CoroutineStmtBuilder.h
===
--- lib/Sema/CoroutineStmtBuilder.h
+++ lib/Sema/CoroutineStmtBuilder.h
@@ -51,6 +51,9 @@
/// name lookup.
bool buildDependentStatements();
+ /// \brief Build just parameter moves. To use for rebuilding in TreeTransform.
+ bool buildParameterMoves();
+
bool isInvalid() const { return !this->IsValid; }
private:
___
cfe-commits mailing lis
[PATCH] D33797: [coroutines] Fix rebuilding of dependent coroutine parameters
GorNishanov added inline comments. Comment at: test/CodeGenCoroutines/coro-params.cpp:103 + // CHECK: call i8* @llvm.coro.begin + // CHECK-NEXT: call void @_ZN1AC1EOS_(%struct.A* %x1, %struct.A* dereferenceable(512) %x) + // CHECK-NEXT: call void @_ZNSt12experimental16coroutine_traitsIJv1AEE12promise_typeC1Ev use %[[x_copy]] instead of %x1 here https://reviews.llvm.org/D33797 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33797: [coroutines] Fix rebuilding of dependent coroutine parameters
GorNishanov updated this revision to Diff 101282.
GorNishanov added a comment.
test updated to use %[[copy_x]] instead of %x1
https://reviews.llvm.org/D33797
Files:
lib/Sema/CoroutineStmtBuilder.h
lib/Sema/SemaCoroutine.cpp
lib/Sema/TreeTransform.h
test/CodeGenCoroutines/coro-params.cpp
Index: test/CodeGenCoroutines/coro-params.cpp
===
--- test/CodeGenCoroutines/coro-params.cpp
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -93,3 +93,26 @@
// CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(%struct.MoveOnly* %[[MoCopy]]
// CHECK-NEXT: call i8* @llvm.coro.free(
}
+
+// CHECK-LABEL: void @_Z16dependent_paramsI1AEvT_(%struct.A* %x
+template
+void dependent_params(T x) {
+ // CHECK: %[[x_copy:.+]] = alloca %struct.A
+
+ // CHECK: call i8* @llvm.coro.begin
+ // CHECK-NEXT: call void @_ZN1AC1EOS_(%struct.A* %[[x_copy]], %struct.A*
dereferenceable(512) %x)
+ // CHECK-NEXT: invoke void
@_ZNSt12experimental16coroutine_traitsIJv1AEE12promise_typeC1Ev
+
+ co_return;
+}
+
+struct A {
+ int WontFitIntoRegisterForSure[128];
+ A() noexcept;
+ A(A&&) noexcept;
+ ~A();
+};
+
+void call_dependent_params() {
+ dependent_params(A{});
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -6959,6 +6959,7 @@
Builder.ReturnStmt = Res.get();
}
}
+ Builder.buildParameterMoves();
return getDerived().RebuildCoroutineBodyStmt(Builder);
}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -822,6 +822,12 @@
return this->IsValid;
}
+bool CoroutineStmtBuilder::buildParameterMoves() {
+ assert(this->IsValid && "coroutine already invalid");
+ assert(this->ParamMoves.empty() && "param moves already built");
+ return this->IsValid = makeParamMoves();
+}
+
bool CoroutineStmtBuilder::buildDependentStatements() {
assert(this->IsValid && "coroutine already invalid");
assert(!this->IsPromiseDependentType &&
Index: lib/Sema/CoroutineStmtBuilder.h
===
--- lib/Sema/CoroutineStmtBuilder.h
+++ lib/Sema/CoroutineStmtBuilder.h
@@ -51,6 +51,9 @@
/// name lookup.
bool buildDependentStatements();
+ /// \brief Build just parameter moves. To use for rebuilding in
TreeTransform.
+ bool buildParameterMoves();
+
bool isInvalid() const { return !this->IsValid; }
private:
Index: test/CodeGenCoroutines/coro-params.cpp
===
--- test/CodeGenCoroutines/coro-params.cpp
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -93,3 +93,26 @@
// CHECK-NEXT: call void @_ZN8MoveOnlyD1Ev(%struct.MoveOnly* %[[MoCopy]]
// CHECK-NEXT: call i8* @llvm.coro.free(
}
+
+// CHECK-LABEL: void @_Z16dependent_paramsI1AEvT_(%struct.A* %x
+template
+void dependent_params(T x) {
+ // CHECK: %[[x_copy:.+]] = alloca %struct.A
+
+ // CHECK: call i8* @llvm.coro.begin
+ // CHECK-NEXT: call void @_ZN1AC1EOS_(%struct.A* %[[x_copy]], %struct.A* dereferenceable(512) %x)
+ // CHECK-NEXT: invoke void @_ZNSt12experimental16coroutine_traitsIJv1AEE12promise_typeC1Ev
+
+ co_return;
+}
+
+struct A {
+ int WontFitIntoRegisterForSure[128];
+ A() noexcept;
+ A(A&&) noexcept;
+ ~A();
+};
+
+void call_dependent_params() {
+ dependent_params(A{});
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -6959,6 +6959,7 @@
Builder.ReturnStmt = Res.get();
}
}
+ Builder.buildParameterMoves();
return getDerived().RebuildCoroutineBodyStmt(Builder);
}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -822,6 +822,12 @@
return this->IsValid;
}
+bool CoroutineStmtBuilder::buildParameterMoves() {
+ assert(this->IsValid && "coroutine already invalid");
+ assert(this->ParamMoves.empty() && "param moves already built");
+ return this->IsValid = makeParamMoves();
+}
+
bool CoroutineStmtBuilder::buildDependentStatements() {
assert(this->IsValid && "coroutine already invalid");
assert(!this->IsPromiseDependentType &&
Index: lib/Sema/CoroutineStmtBuilder.h
===
--- lib/Sema/CoroutineStmtBuilder.h
+++ lib/Sema/CoroutineStmtBuilder.h
@@ -51,6 +51,9 @@
/// name lookup.
bool buildDependentStatements();
+ /// \brief Build just parameter moves. To use for rebuilding in TreeTransform.
+ bool buildParameterMoves();
+
bool isInvalid() const { return !this->IsValid; }
private:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/ma
[PATCH] D34021: [coroutines] Fix co_await for range statement
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D34021 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34194: [coroutines] Allow co_await and co_yield expressions that return an lvalue to compile
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM Comment at: test/CodeGenCoroutines/coro-await.cpp:301 + +// Verifies that we don't chrash when returning an lvalue from a await_resume() +// expression. s/chrash/crash s/a await_resume/an await_resume/ https://reviews.llvm.org/D34194 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34194: [coroutines] Allow co_await and co_yield expressions that return an lvalue to compile
GorNishanov requested changes to this revision.
GorNishanov added inline comments.
This revision now requires changes to proceed.
Comment at: lib/CodeGen/CGCoroutine.cpp:255
+
+static QualType getCoroutineSuspendExprReturnType(const ASTContext &Ctx,
+ const CoroutineSuspendExpr *E) {
#ifndef NDEBUG
...
#endif
https://reviews.llvm.org/D34194
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34194: [coroutines] Allow co_await and co_yield expressions that return an lvalue to compile
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM with changes https://reviews.llvm.org/D34194 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34194: [coroutines] Allow co_await and co_yield expressions that return an lvalue to compile
GorNishanov added a comment. Added John McCall as he made great suggestions last time I touched emitSuspendExpression https://reviews.llvm.org/D34194 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34194: [coroutines] Allow co_await and co_yield expressions that return an lvalue to compile
GorNishanov added inline comments.
Comment at: test/CodeGenCoroutines/coro-await.cpp:310
+ int& x = co_await a;
+ // CHECK: await2.ready:
+ // CHECK-NEXT: %[[RES2:.+]] = call dereferenceable(4) i32*
@_ZN24AwaitResumeReturnsLValue12await_resumeEv(%struct.AwaitResumeReturnsLValue*
%ref.tmp{{.+}})
In release compiler, labels won't have friendly names. //CHECK: await2.ready
will fail
https://reviews.llvm.org/D34194
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41820: [coroutines] Pass coro func args to promise ctor
GorNishanov requested changes to this revision.
GorNishanov added a comment.
This revision now requires changes to proceed.
Thank you for doing this change!
Comment at: lib/Sema/SemaCoroutine.cpp:475
+// Create a static_cast\(expr).
+static Expr *castForMoving(Sema &S, Expr *E, QualType T = QualType()) {
I would keep this block of functions in their original place. (Or move them
here as a separate NFC) commit.
Easier to review what was changed and what was simply moved.
Comment at: lib/Sema/SemaCoroutine.cpp:570
+auto RefExpr = ExprEmpty();
+auto Moves = ScopeInfo->CoroutineParameterMoves;
+if (Moves.find(PD) != Moves.end()) {
This creates a copy of the CoroutineParameterMoves map in every iteration of
the loop.
It seems like an intent is just to create a shorter alias "Moves" to it to
refer later.
I suggest:
1) Make Moves a reference to the map: `auto &Moves = ...`
2) Move it out of the loop
Comment at: lib/Sema/SemaCoroutine.cpp:572
+if (Moves.find(PD) != Moves.end()) {
+ // If a reference to the function parameter exists in the coroutine
+ // frame, use that reference.
Instead of doing lookup twice, once, using `find`, another, using
`operator[]`, you can just say:
```
auto EntryIter = Moves.find(PD);
if (EntryIter != Moves.end()) {
auto *VD = cast(cast(EntryIter->second)->getSingleDecl());
...
```
Comment at: lib/Sema/SemaCoroutine.cpp:574
+ // frame, use that reference.
+ auto *VD = cast(cast(Moves[PD])->getSingleDecl());
+ RefExpr = BuildDeclRefExpr(VD, VD->getType(), ExprValueKind::VK_LValue,
This VD hides outer VD referring to the promise.
I would rename one of them to some other name.
Comment at: lib/Sema/SemaCoroutine.cpp:619
FD->addDecl(VD);
assert(!VD->isInvalidDecl());
return VD;
I believe this assert needs to be removed. We can get an invalid decl if we
failed to find an appropriate constructor. (Couple of negative tests in
SemaCXX/coroutines.cpp would help to flush those cases out)
Comment at: test/CodeGenCoroutines/coro-alloc.cpp:196
}
+
+struct promise_matching_constructor {};
I would move this test to coro-params.cpp, as it is closer to parameter moves
than to allocations.
I would also add a negative test or two to SemaCXX/coroutines.cpp to verify
that we emit sane errors when something goes wrong with promise constructor and
parameter copies.
Repository:
rC Clang
https://reviews.llvm.org/D41820
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47673: [Coroutines] Less IR for noexcept await_resume
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM with some suggestions. Comment at: lib/CodeGen/CGCoroutine.cpp:224 + bool ResumeCanThrow = true; + if (const auto *MCE = dyn_cast(S.getResumeExpr())) +if (const auto *Proto = This long sequence of if statements seems to be asking to be put into its own predicate function: expressionCanThrow or something like that. Repository: rC Clang https://reviews.llvm.org/D47673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48519: [Sema] isValidCoroutineContext FIXME and citations
GorNishanov accepted this revision. GorNishanov added a subscriber: rsmith. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Sema/SemaCoroutine.cpp:260 else if (MD && MD->isMoveAssignmentOperator()) return DiagInvalid(DiagMoveAssign); + // [basic.start.main]p3: "The function main shall not be a coroutine." modocache wrote: > @GorNishanov Is there anything in the TS that states copy and move assignment > operators shall not include await or yield expressions? These were added > D25292 but I'm not sure whether I'm missing something in the TS text, or if > maybe this language was in a prior revision of the TS. Yes. N4499/[special] said: A special member function shall not be a coroutine. I think @rsmith wanted to relax it, but, I am not sure if he had a use case in mind. I am thinking putting the restriction from N4499 back. My approach is if in doubt, be more restrictive initially, then, we can relax if use cases are discovered. It will be a non-breaking change. Repository: rC Clang https://reviews.llvm.org/D48519 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54075: [coroutines] Fix fallthrough warning on try/catch
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang https://reviews.llvm.org/D54075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block
GorNishanov added inline comments.
Comment at: test/SemaCXX/coroutines.cpp:299-311
// FIXME: The spec says this is ill-formed.
void operator=(CtorDtor&) {
co_yield 0; // expected-error {{'co_yield' cannot be used in a copy
assignment operator}}
}
void operator=(CtorDtor const &) {
co_yield 0; // expected-error {{'co_yield' cannot be used in a copy
assignment operator}}
}
modocache wrote:
> lewissbaker wrote:
> > Not related to this diff, but...
> >
> > I don't think that these should be ill-formed.
> >
> > According to N4775 there are only exclusions added for [class.ctor] and
> > [class.dtor].
> > I can't see anything in the spec that says that assignment special member
> > functions cannot be coroutines.
> That's a great point. Could you create a Bugzilla for this work? And maybe we
> can get @GorNishanov's opinion?
In 2015, such as https://wg21.link/N4499 there was a blank prohibition:
"A special member function shall not be a coroutine."
Later, at @rsmith suggestion, we relaxed it a little and banned only
constructors and destructors.
I am not yet aware of any use cases where such coroutines would be useful.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59076/new/
https://reviews.llvm.org/D59076
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block
GorNishanov requested changes to this revision.
GorNishanov added inline comments.
This revision now requires changes to proceed.
Comment at: lib/Sema/SemaCoroutine.cpp:675
+ // Second emphasis of [expr.await]p2: must be outside of an exception
handler.
+ if (S.getCurScope()->getFlags() & Scope::CatchScope) {
+S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword;
lewissbaker wrote:
> modocache wrote:
> > modocache wrote:
> > > lewissbaker wrote:
> > > > modocache wrote:
> > > > > EricWF wrote:
> > > > > > We can still build a valid AST after encountering this error, no?
> > > > > >
> > > > > >
> > > > > I believe so. Just to be clear: you'd like me to continue building
> > > > > the AST even after emitting this error diagnostic? My understanding
> > > > > is that most of this file bails soon after any error is encountered
> > > > > (correct me if that's wrong). I'm happy to change that, but I wonder
> > > > > if it'd be better to do that in a separate diff...?
> > > > Do the scope flags reset when a lambda occurs within a catch-block?
> > > >
> > > > eg. The following should still be valid.
> > > > ```
> > > > try { ... }
> > > > catch (...) {
> > > > []() -> task { co_await foo(); }();
> > > > }
> > > > ```
> > > I believe they're reset, the example you posted compiles fine with this
> > > patch. I'll add a test case specifically for this and confirm exactly
> > > where the scope flags are reset or ignored in the lambda case.
> > When the parser encounters a lambda, it takes the path
> > `Parser::ParseLambdaExpression ->
> > Parser::ParseLambdaExpressionAfterIntroducer ->
> > Parser::ParseCompoundStatementBody`, which creates an inner scope for the
> > body of the lambda. This inner scope does not have the `Scope::CatchScope`
> > flag, so it doesn't result in the error.
> >
> > Although, your question did make me realize that this compiles just fine,
> > even with this patch:
> >
> > ```
> > void example() {
> > try {
> > throw;
> > } catch (...) {
> > try {
> > co_await a;
> > }
> > }
> > }
> > ```
> >
> > But I believe this above case should also be treated as an error, right?
> Yes, I believe that co_await within a try-block within a catch-block should
> not be allowed.
Yes. That will result in suspension of the coroutine and we don't yet know how
to suspend in catch blocks.
Also, I agree with @EricWF, this error should not prevent us from continuing
semantic analysis with the rest of the function body.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59076/new/
https://reviews.llvm.org/D59076
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59076/new/ https://reviews.llvm.org/D59076 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value
GorNishanov accepted this revision. GorNishanov added a comment. LGTM! Thank you for doing this. Repository: rC Clang https://reviews.llvm.org/D51741 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37433: [cxx_status] Mark coroutine TS support as clang 5.0 and change class from svn to full for P0096R4
GorNishanov created this revision. Shall we? https://reviews.llvm.org/D37433 Files: www/cxx_status.html Index: www/cxx_status.html === --- www/cxx_status.html +++ www/cxx_status.html @@ -872,7 +872,7 @@ - + Clang 5 (http://wg21.link/p0096r4";>P0096R4) @@ -898,9 +898,9 @@ [DRAFT TS] Coroutines - https://isocpp.org/files/papers/N4663.pdf";>N4663 + http://open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4680.pdf";>N4680 -fcoroutines-ts-stdlib=libc++ - SVN + Clang 5 (http://open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4680.pdf";>N4680) [TS] Library Fundamentals, Version 1 (invocation type traits) Index: www/cxx_status.html === --- www/cxx_status.html +++ www/cxx_status.html @@ -872,7 +872,7 @@ - + Clang 5 (http://wg21.link/p0096r4";>P0096R4) @@ -898,9 +898,9 @@ [DRAFT TS] Coroutines - https://isocpp.org/files/papers/N4663.pdf";>N4663 + http://open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4680.pdf";>N4680 -fcoroutines-ts-stdlib=libc++ - SVN + Clang 5 (http://open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4680.pdf";>N4680) [TS] Library Fundamentals, Version 1 (invocation type traits) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37454: [coroutines] desugar auto type of await_resume before checking whether it is void or bool
GorNishanov created this revision.
https://reviews.llvm.org/D37454
Files:
lib/Sema/SemaCoroutine.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -66,6 +66,12 @@
void await_resume() {}
};
+struct auto_await_suspend {
+ bool await_ready();
+ template auto await_suspend(F) {}
+ void await_resume();
+};
+
struct DummyVoidTag {};
DummyVoidTag no_specialization() { // expected-error {{this function cannot be
a coroutine: 'std::experimental::coroutine_traits' has no member
named 'promise_type'}}
co_await a;
@@ -159,6 +165,10 @@
co_yield yield; // expected-error {{no member named 'await_ready' in
'not_awaitable'}}
}
+void check_auto_await_suspend() {
+ co_await auto_await_suspend{}; // OK
+}
+
void coreturn(int n) {
co_await a;
if (n == 0)
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -444,6 +444,9 @@
else {
// non-class prvalues always have cv-unqualified types
QualType AdjRetType = RetType.getUnqualifiedType();
+ if (auto *AT = dyn_cast(AdjRetType))
+AdjRetType = AT->desugar();
+
if (RetType->isReferenceType() ||
(AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -66,6 +66,12 @@
void await_resume() {}
};
+struct auto_await_suspend {
+ bool await_ready();
+ template auto await_suspend(F) {}
+ void await_resume();
+};
+
struct DummyVoidTag {};
DummyVoidTag no_specialization() { // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits' has no member named 'promise_type'}}
co_await a;
@@ -159,6 +165,10 @@
co_yield yield; // expected-error {{no member named 'await_ready' in 'not_awaitable'}}
}
+void check_auto_await_suspend() {
+ co_await auto_await_suspend{}; // OK
+}
+
void coreturn(int n) {
co_await a;
if (n == 0)
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -444,6 +444,9 @@
else {
// non-class prvalues always have cv-unqualified types
QualType AdjRetType = RetType.getUnqualifiedType();
+ if (auto *AT = dyn_cast(AdjRetType))
+AdjRetType = AT->desugar();
+
if (RetType->isReferenceType() ||
(AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37454: [coroutines] Make sure auto return type of await_resume is properly handled
GorNishanov updated this revision to Diff 113800.
GorNishanov retitled this revision from "[coroutines] desugar auto type of
await_resume before checking whether it is void or bool" to "[coroutines] Make
sure auto return type of await_resume is properly handled".
GorNishanov added a comment.
use isVoidType() and isBooleantType() instead of cruft that used to be there
before
https://reviews.llvm.org/D37454
Files:
lib/Sema/SemaCoroutine.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -66,6 +66,12 @@
void await_resume() {}
};
+struct auto_await_suspend {
+ bool await_ready();
+ template auto await_suspend(F) {}
+ void await_resume();
+};
+
struct DummyVoidTag {};
DummyVoidTag no_specialization() { // expected-error {{this function cannot be
a coroutine: 'std::experimental::coroutine_traits' has no member
named 'promise_type'}}
co_await a;
@@ -159,6 +165,10 @@
co_yield yield; // expected-error {{no member named 'await_ready' in
'not_awaitable'}}
}
+void check_auto_await_suspend() {
+ co_await auto_await_suspend{}; // OK
+}
+
void coreturn(int n) {
co_await a;
if (n == 0)
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -438,14 +438,14 @@
// - await-suspend is the expression e.await_suspend(h), which shall be
// a prvalue of type void or bool.
QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
+
// Experimental support for coroutine_handle returning await_suspend.
if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc))
Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
else {
// non-class prvalues always have cv-unqualified types
- QualType AdjRetType = RetType.getUnqualifiedType();
if (RetType->isReferenceType() ||
- (AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
+ (!RetType->isBooleanType() && !RetType->isVoidType())) {
S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
diag::err_await_suspend_invalid_return_type)
<< RetType;
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -66,6 +66,12 @@
void await_resume() {}
};
+struct auto_await_suspend {
+ bool await_ready();
+ template auto await_suspend(F) {}
+ void await_resume();
+};
+
struct DummyVoidTag {};
DummyVoidTag no_specialization() { // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits' has no member named 'promise_type'}}
co_await a;
@@ -159,6 +165,10 @@
co_yield yield; // expected-error {{no member named 'await_ready' in 'not_awaitable'}}
}
+void check_auto_await_suspend() {
+ co_await auto_await_suspend{}; // OK
+}
+
void coreturn(int n) {
co_await a;
if (n == 0)
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -438,14 +438,14 @@
// - await-suspend is the expression e.await_suspend(h), which shall be
// a prvalue of type void or bool.
QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
+
// Experimental support for coroutine_handle returning await_suspend.
if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc))
Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
else {
// non-class prvalues always have cv-unqualified types
- QualType AdjRetType = RetType.getUnqualifiedType();
if (RetType->isReferenceType() ||
- (AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
+ (!RetType->isBooleanType() && !RetType->isVoidType())) {
S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
diag::err_await_suspend_invalid_return_type)
<< RetType;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37454: [coroutines] Make sure auto return type of await_resume is properly handled
GorNishanov marked an inline comment as done.
GorNishanov added inline comments.
Comment at: lib/Sema/SemaCoroutine.cpp:451
if (RetType->isReferenceType() ||
(AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
rsmith wrote:
> Don't desugar the type; instead, use `AdjRetType->isBooleanType() ||
> AdjRetType->isVoidType()` here. You can also delete the
> `getUnqualifiedType()` call above if you do this, and just use `RetType`
> here, since `->isBooleanType()` on `const bool` returns `true`.
Cool! Much shorter and prettier. Thank you!
https://reviews.llvm.org/D37454
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37454: [coroutines] Make sure auto return type of await_resume is properly handled
GorNishanov marked an inline comment as done.
GorNishanov added inline comments.
Comment at: test/SemaCXX/coroutines.cpp:169
+void check_auto_await_suspend() {
+ co_await auto_await_suspend{}; // OK
+}
javed.absar wrote:
> maybe change the comment to something more meaningful, or remove it.
Sure thing. I'll remove the comment.
https://reviews.llvm.org/D37454
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37454: [coroutines] Make sure auto return type of await_resume is properly handled
This revision was automatically updated to reflect the committed changes.
Closed by commit rL312565: [coroutines] Make sure auto return type of
await_resume is properly handled (authored by GorNishanov).
Changed prior to commit:
https://reviews.llvm.org/D37454?vs=113800&id=113894#toc
Repository:
rL LLVM
https://reviews.llvm.org/D37454
Files:
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp
Index: cfe/trunk/lib/Sema/SemaCoroutine.cpp
===
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp
@@ -438,14 +438,14 @@
// - await-suspend is the expression e.await_suspend(h), which shall be
// a prvalue of type void or bool.
QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
+
// Experimental support for coroutine_handle returning await_suspend.
if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc))
Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
else {
// non-class prvalues always have cv-unqualified types
- QualType AdjRetType = RetType.getUnqualifiedType();
if (RetType->isReferenceType() ||
- (AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
+ (!RetType->isBooleanType() && !RetType->isVoidType())) {
S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
diag::err_await_suspend_invalid_return_type)
<< RetType;
Index: cfe/trunk/test/SemaCXX/coroutines.cpp
===
--- cfe/trunk/test/SemaCXX/coroutines.cpp
+++ cfe/trunk/test/SemaCXX/coroutines.cpp
@@ -66,6 +66,12 @@
void await_resume() {}
};
+struct auto_await_suspend {
+ bool await_ready();
+ template auto await_suspend(F) {}
+ void await_resume();
+};
+
struct DummyVoidTag {};
DummyVoidTag no_specialization() { // expected-error {{this function cannot be
a coroutine: 'std::experimental::coroutine_traits' has no member
named 'promise_type'}}
co_await a;
@@ -159,6 +165,10 @@
co_yield yield; // expected-error {{no member named 'await_ready' in
'not_awaitable'}}
}
+void check_auto_await_suspend() {
+ co_await auto_await_suspend{}; // Should compile successfully.
+}
+
void coreturn(int n) {
co_await a;
if (n == 0)
Index: cfe/trunk/lib/Sema/SemaCoroutine.cpp
===
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp
@@ -438,14 +438,14 @@
// - await-suspend is the expression e.await_suspend(h), which shall be
// a prvalue of type void or bool.
QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
+
// Experimental support for coroutine_handle returning await_suspend.
if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc))
Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
else {
// non-class prvalues always have cv-unqualified types
- QualType AdjRetType = RetType.getUnqualifiedType();
if (RetType->isReferenceType() ||
- (AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
+ (!RetType->isBooleanType() && !RetType->isVoidType())) {
S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
diag::err_await_suspend_invalid_return_type)
<< RetType;
Index: cfe/trunk/test/SemaCXX/coroutines.cpp
===
--- cfe/trunk/test/SemaCXX/coroutines.cpp
+++ cfe/trunk/test/SemaCXX/coroutines.cpp
@@ -66,6 +66,12 @@
void await_resume() {}
};
+struct auto_await_suspend {
+ bool await_ready();
+ template auto await_suspend(F) {}
+ void await_resume();
+};
+
struct DummyVoidTag {};
DummyVoidTag no_specialization() { // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits' has no member named 'promise_type'}}
co_await a;
@@ -159,6 +165,10 @@
co_yield yield; // expected-error {{no member named 'await_ready' in 'not_awaitable'}}
}
+void check_auto_await_suspend() {
+ co_await auto_await_suspend{}; // Should compile successfully.
+}
+
void coreturn(int n) {
co_await a;
if (n == 0)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D62550: [coroutines][PR41909] Don't build dependent coroutine statements for generic lambda
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM! Thank you for the fix Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62550/new/ https://reviews.llvm.org/D62550 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D63381: Allow copy/move assignment operator to be coroutine as per N4775
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM! Thank you for the fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63381/new/ https://reviews.llvm.org/D63381 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45121: [coroutines] Add noop_coroutine to
GorNishanov closed this revision. GorNishanov added a comment. Committed as: https://reviews.llvm.org/rCXX329237 and a flurry of cleanup fixes: https://reviews.llvm.org/rCXX329239 https://reviews.llvm.org/rCXX329240 https://reviews.llvm.org/rCXX329245 https://reviews.llvm.org/D45121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45860: [Coroutines] Catch exceptions in await_resume
GorNishanov added a comment.
Thank you for doing this. It looks very elegant, but, it is a little bit wrong.
It creates two different initial_suspend objects.
Run this example:
https://wandbox.org/permlink/Q1Zd2NUlolmw9YmX
You will observe that in trunk we are getting the output:
0x216808c: constructed(initial)
0x216808c: await_ready
0x216808c: await_suspend
0x216808c: await_resume
0x216808c: destroyed
0x21680b8: constructed(final)
0x21680b8: await_ready
0x21680b8: await_suspend
consumed 10 values with sum 45
0x21680b8: destroyed
promise destroyed
With this change, the output becomes:
0x1965c4c: constructed(initial)
0x1965c4c: await_ready
0x1965c4c: await_suspend
0x1965c4c: destroyed
0x1965c60: constructed(initial)
0x1965c60: await_resume
0x1965c60: destroyed
0x1965c80: constructed(final)
0x1965c80: await_ready
0x1965c80: await_suspend
consumed 10 values with sum 45
0x1965c80: destroyed
0x1965c60: destroyed
promise destroyed
I suggest, first modify the unit test to check that we do not create two
initial_suspend objects. Then to fix it :-)
Comment at: lib/CodeGen/CGCoroutine.cpp:595
+auto InitSuspend = S.getInitSuspendStmt();
CurCoro.Data->CurrentAwaitKind = AwaitKind::Init;
auto *InitialSuspend = ...
See:
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
Comment at: lib/CodeGen/CGCoroutine.cpp:605
auto Loc = S.getLocStart();
+ auto AwaitExpr =
+ cast(cast(InitSuspend)->getSubExpr());
auto *AwaitExpr =
Comment at: lib/CodeGen/CGCoroutine.cpp:608
+
+ SmallVector Stmts;
+ Stmts.push_back(AwaitExpr->getResumeExpr());
Consider:
```
std::array Stmts = {AwaitExpr->getResumeExpr(), S.getBody()};
```
instead
Repository:
rC Clang
https://reviews.llvm.org/D45860
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45860: [Coroutines] Catch exceptions in await_resume
GorNishanov added a comment.
Looks good. Make sure to run the tests in release and debug mode.
On my box, these three test failed in release mode.
Clang :: CodeGenCoroutines/coro-await-resume-eh.cpp
Clang :: CodeGenCoroutines/coro-promise-dtor.cpp
Clang :: CodeGenCoroutines/coro-unhandled-exception.cpp
There is a small difference in friendly names emission in release mode
Comment at: lib/CodeGen/CGCoroutine.cpp:220
CGF.EmitBlock(ReadyBlock);
+ CXXTryStmt *TryStmt = nullptr;
+ if (Coro.ExceptionHandler && Kind == AwaitKind::Init) {
I suggest to check whether await_resume expression is noexcept and omit
emission of all of the goo. Most of the time the await_ready for
initial_suspend will be noexcept and thus we would not need to emit extra stuff.
Repository:
rC Clang
https://reviews.llvm.org/D45860
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45860: [Coroutines] Catch exceptions in await_resume
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D45860 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48863: [Sema] Explain coroutine_traits template in diag
GorNishanov added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9053 "a coroutine">; +def note_coroutine_types_for_traits_here : Note< + "the coroutine traits class template is being instantiated using the return " I am wondering what is the use case here. Is it to guard against badly designed standard library? I played around a little and tried to see how I could get to trigger this error with a user-code, but, failed. If I did not specify enough parameters it will default to primary template defined in the Repository: rC Clang https://reviews.llvm.org/D48863 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36070: [coroutines] Evaluate the operand of void `co_return` expressions.
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D36070 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37115: [coroutines] Do not attempt to typo-correct when coroutine is looking for required members
GorNishanov created this revision.
When SemaCoroutine looks for await_resume, it means it. No need for helpful:
"Did you mean await_ready?" messages.
Fixes PR33477 and a couple of FIXMEs in test/SemaCXX/coroutines.cpp
https://reviews.llvm.org/D37115
Files:
lib/Sema/SemaCoroutine.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -500,8 +500,7 @@
struct bad_promise_2 {
coro get_return_object();
- // FIXME: We shouldn't offer a typo-correction here!
- suspend_always final_suspend(); // expected-note {{here}}
+ suspend_always final_suspend();
void unhandled_exception();
void return_void();
};
@@ -512,8 +511,7 @@
struct bad_promise_3 {
coro get_return_object();
- // FIXME: We shouldn't offer a typo-correction here!
- suspend_always initial_suspend(); // expected-note {{here}}
+ suspend_always initial_suspend();
void unhandled_exception();
void return_void();
};
@@ -1162,3 +1160,22 @@
template CoroMemberTag DepTestType::test_static_template(const char
*volatile &, unsigned);
} // namespace CoroHandleMemberFunctionTest
+
+struct missing_await_ready {
+ void await_suspend(std::experimental::coroutine_handle<>);
+ void await_resume();
+};
+struct missing_await_suspend {
+ bool await_ready();
+ void await_resume();
+};
+struct missing_await_resume {
+ bool await_ready();
+ void await_suspend(std::experimental::coroutine_handle<>);
+};
+
+void test_missing_awaitable_members() {
+ co_await missing_await_ready{}; // expected-error {{no member named
'await_ready' in 'missing_await_ready'}}
+ co_await missing_await_suspend{}; // expected-error {{no member named
'await_suspend' in 'missing_await_suspend'}}
+ co_await missing_await_resume{}; // expected-error {{no member named
'await_resume' in 'missing_await_resume'}}
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -360,6 +360,15 @@
if (Result.isInvalid())
return ExprError();
+ // We meant exactly what we asked for. No need for typo correction.
+ if (auto *TE = dyn_cast(Result.get())) {
+S.clearDelayedTypo(TE);
+S.Diag(Loc, diag::err_no_member)
+<< NameInfo.getName() << Base->getType()->getAsCXXRecordDecl()
+<< Base->getSourceRange();
+return ExprError();
+ }
+
return S.ActOnCallExpr(nullptr, Result.get(), Loc, Args, Loc, nullptr);
}
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -500,8 +500,7 @@
struct bad_promise_2 {
coro get_return_object();
- // FIXME: We shouldn't offer a typo-correction here!
- suspend_always final_suspend(); // expected-note {{here}}
+ suspend_always final_suspend();
void unhandled_exception();
void return_void();
};
@@ -512,8 +511,7 @@
struct bad_promise_3 {
coro get_return_object();
- // FIXME: We shouldn't offer a typo-correction here!
- suspend_always initial_suspend(); // expected-note {{here}}
+ suspend_always initial_suspend();
void unhandled_exception();
void return_void();
};
@@ -1162,3 +1160,22 @@
template CoroMemberTag DepTestType::test_static_template(const char *volatile &, unsigned);
} // namespace CoroHandleMemberFunctionTest
+
+struct missing_await_ready {
+ void await_suspend(std::experimental::coroutine_handle<>);
+ void await_resume();
+};
+struct missing_await_suspend {
+ bool await_ready();
+ void await_resume();
+};
+struct missing_await_resume {
+ bool await_ready();
+ void await_suspend(std::experimental::coroutine_handle<>);
+};
+
+void test_missing_awaitable_members() {
+ co_await missing_await_ready{}; // expected-error {{no member named 'await_ready' in 'missing_await_ready'}}
+ co_await missing_await_suspend{}; // expected-error {{no member named 'await_suspend' in 'missing_await_suspend'}}
+ co_await missing_await_resume{}; // expected-error {{no member named 'await_resume' in 'missing_await_resume'}}
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -360,6 +360,15 @@
if (Result.isInvalid())
return ExprError();
+ // We meant exactly what we asked for. No need for typo correction.
+ if (auto *TE = dyn_cast(Result.get())) {
+S.clearDelayedTypo(TE);
+S.Diag(Loc, diag::err_no_member)
+<< NameInfo.getName() << Base->getType()->getAsCXXRecordDecl()
+<< Base->getSourceRange();
+return ExprError();
+ }
+
return S.ActOnCallExpr(nullptr, Result.get(), Loc, Args, Loc, nullptr);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
[PATCH] D37131: [coroutines] Support coroutine-handle returning await-suspend (i.e symmetric control transfer)
GorNishanov created this revision.
If await_suspend returns a coroutine_handle, as in the example below:
coroutine_handle<> await_suspend(coroutine_handle<> h) {
coro.promise().waiter = h;
return coro;
}
suspensionExpression processing will resume the coroutine pointed at by that
handle.
Related LLVM change https://reviews.llvm.org/rL311751 makes resume calls of
this kind `musttail` at any optimization level.
This enables unlimited symmetric control transfer from coroutine to coroutine
without blowing up the stack.
https://reviews.llvm.org/D37131
Files:
lib/CodeGen/CGCoroutine.cpp
lib/Sema/SemaCoroutine.cpp
test/CodeGenCoroutines/coro-await.cpp
Index: test/CodeGenCoroutines/coro-await.cpp
===
--- test/CodeGenCoroutines/coro-await.cpp
+++ test/CodeGenCoroutines/coro-await.cpp
@@ -12,6 +12,7 @@
struct coroutine_handle {
void *ptr;
static coroutine_handle from_address(void *);
+ void *address();
};
template
@@ -326,3 +327,20 @@
// CHECK-NEXT: store %struct.RefTag* %[[RES3]], %struct.RefTag** %[[ZVAR]],
RefTag& z = co_yield 42;
}
+
+struct TailCallAwait {
+ bool await_ready();
+ std::experimental::coroutine_handle<> await_suspend(std::experimental::coroutine_handle<>);
+ void await_resume();
+};
+
+// CHECK-LABEL: @TestTailcall(
+extern "C" void TestTailcall() {
+ co_await TailCallAwait{};
+
+ // CHECK: %[[RESULT:.+]] = call i8* @_ZN13TailCallAwait13await_suspendENSt12experimental16coroutine_handleIvEE(%struct.TailCallAwait*
+ // CHECK: %[[COERCE:.+]] = getelementptr inbounds %"struct.std::experimental::coroutine_handle", %"struct.std::experimental::coroutine_handle"* %[[TMP:.+]], i32 0, i32 0
+ // CHECK: store i8* %[[RESULT]], i8** %[[COERCE]]
+ // CHECK: %[[ADDR:.+]] = call i8* @_ZNSt12experimental16coroutine_handleIvE7addressEv(%"struct.std::experimental::coroutine_handle"* %[[TMP]])
+ // CHECK: call void @llvm.coro.resume(i8* %[[ADDR]])
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -363,6 +363,32 @@
return S.ActOnCallExpr(nullptr, Result.get(), Loc, Args, Loc, nullptr);
}
+// See if return type is coroutine-handle and if so, invoke builtin coro-resume
+// on its address. This is to enable experimental support for coroutine-handle
+// returning await_suspend that results in a guranteed tail call to the target
+// coroutine.
+static Expr *maybeTailCall(Sema &S, QualType RetType, Expr *E,
+ SourceLocation Loc) {
+ if (RetType->isReferenceType())
+return nullptr;
+ Type const *T = RetType.getTypePtr();
+ if (!T->isClassType() && !T->isStructureType())
+return nullptr;
+
+ // FIXME: Add convertability check to coroutine_handle<>. Possibly via
+ // EvaluateBinaryTypeTrait(BTT_IsConvertible, ...) which is at the moment
+ // a private function in SemaExprCXX.cpp
+
+ ExprResult AddressExpr = buildMemberCall(S, E, Loc, "address", None);
+ if (AddressExpr.isInvalid())
+return nullptr;
+
+ Expr *JustAddress = AddressExpr.get();
+ // FIXME: Check that the type of AddressExpr is void*
+ return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume,
+ JustAddress);
+}
+
/// Build calls to await_ready, await_suspend, and await_resume for a co_await
/// expression.
static ReadySuspendResumeResult buildCoawaitCalls(Sema &S, VarDecl *CoroPromise,
@@ -412,16 +438,21 @@
// - await-suspend is the expression e.await_suspend(h), which shall be
// a prvalue of type void or bool.
QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
-// non-class prvalues always have cv-unqualified types
-QualType AdjRetType = RetType.getUnqualifiedType();
-if (RetType->isReferenceType() ||
-(AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
- S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
- diag::err_await_suspend_invalid_return_type)
- << RetType;
- S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required)
- << AwaitSuspend->getDirectCallee();
- Calls.IsInvalid = true;
+// Experimental support for coroutine_handle returning await_suspend.
+if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc))
+ Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
+else {
+ // non-class prvalues always have cv-unqualified types
+ QualType AdjRetType = RetType.getUnqualifiedType();
+ if (RetType->isReferenceType() ||
+ (AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
+S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
+ diag::err_await_suspend_invalid_return_type)
+<< RetType;
+S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required)
+<< AwaitSuspend->getDirectCallee();
[PATCH] D37131: [coroutines] Support coroutine-handle returning await-suspend (i.e symmetric control transfer)
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D37131 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37131: [coroutines] Support coroutine-handle returning await-suspend (i.e symmetric control transfer)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL311762: [coroutines] Support coroutine-handle returning
await-suspend (i.e symmetric… (authored by GorNishanov).
Changed prior to commit:
https://reviews.llvm.org/D37131?vs=112658&id=112659#toc
Repository:
rL LLVM
https://reviews.llvm.org/D37131
Files:
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-await.cpp
Index: cfe/trunk/test/CodeGenCoroutines/coro-await.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-await.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-await.cpp
@@ -12,6 +12,7 @@
struct coroutine_handle {
void *ptr;
static coroutine_handle from_address(void *);
+ void *address();
};
template
@@ -326,3 +327,20 @@
// CHECK-NEXT: store %struct.RefTag* %[[RES3]], %struct.RefTag** %[[ZVAR]],
RefTag& z = co_yield 42;
}
+
+struct TailCallAwait {
+ bool await_ready();
+ std::experimental::coroutine_handle<> await_suspend(std::experimental::coroutine_handle<>);
+ void await_resume();
+};
+
+// CHECK-LABEL: @TestTailcall(
+extern "C" void TestTailcall() {
+ co_await TailCallAwait{};
+
+ // CHECK: %[[RESULT:.+]] = call i8* @_ZN13TailCallAwait13await_suspendENSt12experimental16coroutine_handleIvEE(%struct.TailCallAwait*
+ // CHECK: %[[COERCE:.+]] = getelementptr inbounds %"struct.std::experimental::coroutine_handle", %"struct.std::experimental::coroutine_handle"* %[[TMP:.+]], i32 0, i32 0
+ // CHECK: store i8* %[[RESULT]], i8** %[[COERCE]]
+ // CHECK: %[[ADDR:.+]] = call i8* @_ZNSt12experimental16coroutine_handleIvE7addressEv(%"struct.std::experimental::coroutine_handle"* %[[TMP]])
+ // CHECK: call void @llvm.coro.resume(i8* %[[ADDR]])
+}
Index: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
===
--- cfe/trunk/lib/CodeGen/CGCoroutine.cpp
+++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp
@@ -181,10 +181,8 @@
auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});
auto *SuspendRet = CGF.EmitScalarExpr(S.getSuspendExpr());
- if (SuspendRet != nullptr) {
+ if (SuspendRet != nullptr && SuspendRet->getType()->isIntegerTy(1)) {
// Veto suspension if requested by bool returning await_suspend.
-assert(SuspendRet->getType()->isIntegerTy(1) &&
- "Sema should have already checked that it is void or bool");
BasicBlock *RealSuspendBlock =
CGF.createBasicBlock(Prefix + Twine(".suspend.bool"));
CGF.Builder.CreateCondBr(SuspendRet, RealSuspendBlock, ReadyBlock);
Index: cfe/trunk/lib/Sema/SemaCoroutine.cpp
===
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp
@@ -363,6 +363,32 @@
return S.ActOnCallExpr(nullptr, Result.get(), Loc, Args, Loc, nullptr);
}
+// See if return type is coroutine-handle and if so, invoke builtin coro-resume
+// on its address. This is to enable experimental support for coroutine-handle
+// returning await_suspend that results in a guranteed tail call to the target
+// coroutine.
+static Expr *maybeTailCall(Sema &S, QualType RetType, Expr *E,
+ SourceLocation Loc) {
+ if (RetType->isReferenceType())
+return nullptr;
+ Type const *T = RetType.getTypePtr();
+ if (!T->isClassType() && !T->isStructureType())
+return nullptr;
+
+ // FIXME: Add convertability check to coroutine_handle<>. Possibly via
+ // EvaluateBinaryTypeTrait(BTT_IsConvertible, ...) which is at the moment
+ // a private function in SemaExprCXX.cpp
+
+ ExprResult AddressExpr = buildMemberCall(S, E, Loc, "address", None);
+ if (AddressExpr.isInvalid())
+return nullptr;
+
+ Expr *JustAddress = AddressExpr.get();
+ // FIXME: Check that the type of AddressExpr is void*
+ return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume,
+ JustAddress);
+}
+
/// Build calls to await_ready, await_suspend, and await_resume for a co_await
/// expression.
static ReadySuspendResumeResult buildCoawaitCalls(Sema &S, VarDecl *CoroPromise,
@@ -412,16 +438,21 @@
// - await-suspend is the expression e.await_suspend(h), which shall be
// a prvalue of type void or bool.
QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
-// non-class prvalues always have cv-unqualified types
-QualType AdjRetType = RetType.getUnqualifiedType();
-if (RetType->isReferenceType() ||
-(AdjRetType != S.Context.BoolTy && AdjRetType != S.Context.VoidTy)) {
- S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
- diag::err_await_suspend_invalid_return_type)
- << RetType;
- S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required)
- << AwaitSuspend->getDirectCallee();
- Calls.IsInvalid = true;
+// Experimental support for coroutine_handle returning
[PATCH] D43927: [Coroutines] Schedule coro-split before asan
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang https://reviews.llvm.org/D43927 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37115: [coroutines] Do not attempt to typo-correct when coroutine is looking for required members
GorNishanov closed this revision. GorNishanov added a comment. Fixed: git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@328663 91177308-0d34-0410-b5e6-96231b3b80d8 https://reviews.llvm.org/D37115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44552: [Coroutines] Find custom allocators in class scope
GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.
LGTM with some stylistic suggestions
Comment at: lib/Sema/SemaExprCXX.cpp:2351
+return true;
+ else
+LookupQualifiedName(R, Context.getTranslationUnitDecl());
drop else?
```
if (R.empty()) {
if (NewScope == AFS_Class)
return true;
LookupQualifiedName(R, Context.getTranslationUnitDecl());
}
```
Comment at: lib/Sema/SemaExprCXX.cpp:2402
+ return true;
+else {
+ DeclareGlobalNewDelete();
drop else? so that it will read:
```
if (FoundDelete.empty()) {
if (DeleteScope == AFS_Class)
return true;
DeclareGlobalNewDelete();
LookupQualifiedName(FoundDelete, Context.getTranslationUnitDecl());
}
```
Repository:
rC Clang
https://reviews.llvm.org/D44552
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45120: [coroutines] Add __builtin_coro_noop => llvm.coro.noop
GorNishanov created this revision. GorNishanov added reviewers: EricWF, modocache, lewissbaker. A recent addition to Coroutines TS (https://wg21.link/p0913) adds a pre-defined coroutine noop_coroutine that does nothing. To implement this feature, we implemented an llvm.coro.noop intrinsic that returns a coroutine handle to a coroutine that does nothing when resumed or destroyed. This patch adds a builtin __builtin_coro_noop() that maps to llvm.coro.noop intrinsic. Related llvm change: https://reviews.llvm.org/D45114 https://reviews.llvm.org/D45120 Files: include/clang/Basic/Builtins.def lib/CodeGen/CGBuiltin.cpp test/CodeGenCoroutines/coro-builtins.c Index: test/CodeGenCoroutines/coro-builtins.c === --- test/CodeGenCoroutines/coro-builtins.c +++ test/CodeGenCoroutines/coro-builtins.c @@ -17,6 +17,9 @@ // CHECK-NEXT: call i1 @llvm.coro.alloc(token %[[COROID]]) __builtin_coro_alloc(); + // CHECK-NEXT: call i8* @llvm.coro.noop() + __builtin_coro_noop(); + // CHECK-NEXT: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64() // CHECK-NEXT: %[[MEM:.+]] = call i8* @myAlloc(i64 %[[SIZE]]) // CHECK-NEXT: %[[FRAME:.+]] = call i8* @llvm.coro.begin(token %[[COROID]], i8* %[[MEM]]) Index: lib/CodeGen/CGBuiltin.cpp === --- lib/CodeGen/CGBuiltin.cpp +++ lib/CodeGen/CGBuiltin.cpp @@ -2796,6 +2796,8 @@ return EmitCoroutineIntrinsic(E, Intrinsic::coro_resume); case Builtin::BI__builtin_coro_frame: return EmitCoroutineIntrinsic(E, Intrinsic::coro_frame); + case Builtin::BI__builtin_coro_noop: +return EmitCoroutineIntrinsic(E, Intrinsic::coro_noop); case Builtin::BI__builtin_coro_free: return EmitCoroutineIntrinsic(E, Intrinsic::coro_free); case Builtin::BI__builtin_coro_destroy: Index: include/clang/Basic/Builtins.def === --- include/clang/Basic/Builtins.def +++ include/clang/Basic/Builtins.def @@ -1391,6 +1391,7 @@ BUILTIN(__builtin_coro_size, "z", "n") BUILTIN(__builtin_coro_frame, "v*", "n") +BUILTIN(__builtin_coro_noop, "v*", "n") BUILTIN(__builtin_coro_free, "v*v*", "n") BUILTIN(__builtin_coro_id, "v*Iiv*v*v*", "n") Index: test/CodeGenCoroutines/coro-builtins.c === --- test/CodeGenCoroutines/coro-builtins.c +++ test/CodeGenCoroutines/coro-builtins.c @@ -17,6 +17,9 @@ // CHECK-NEXT: call i1 @llvm.coro.alloc(token %[[COROID]]) __builtin_coro_alloc(); + // CHECK-NEXT: call i8* @llvm.coro.noop() + __builtin_coro_noop(); + // CHECK-NEXT: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64() // CHECK-NEXT: %[[MEM:.+]] = call i8* @myAlloc(i64 %[[SIZE]]) // CHECK-NEXT: %[[FRAME:.+]] = call i8* @llvm.coro.begin(token %[[COROID]], i8* %[[MEM]]) Index: lib/CodeGen/CGBuiltin.cpp === --- lib/CodeGen/CGBuiltin.cpp +++ lib/CodeGen/CGBuiltin.cpp @@ -2796,6 +2796,8 @@ return EmitCoroutineIntrinsic(E, Intrinsic::coro_resume); case Builtin::BI__builtin_coro_frame: return EmitCoroutineIntrinsic(E, Intrinsic::coro_frame); + case Builtin::BI__builtin_coro_noop: +return EmitCoroutineIntrinsic(E, Intrinsic::coro_noop); case Builtin::BI__builtin_coro_free: return EmitCoroutineIntrinsic(E, Intrinsic::coro_free); case Builtin::BI__builtin_coro_destroy: Index: include/clang/Basic/Builtins.def === --- include/clang/Basic/Builtins.def +++ include/clang/Basic/Builtins.def @@ -1391,6 +1391,7 @@ BUILTIN(__builtin_coro_size, "z", "n") BUILTIN(__builtin_coro_frame, "v*", "n") +BUILTIN(__builtin_coro_noop, "v*", "n") BUILTIN(__builtin_coro_free, "v*v*", "n") BUILTIN(__builtin_coro_id, "v*Iiv*v*v*", "n") ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45121: [coroutines] Add noop_coroutine to
GorNishanov created this revision.
GorNishanov added reviewers: EricWF, lewissbaker, modocache.
GorNishanov added dependencies: D45114: [coroutines] Add support for
llvm.coro.noop intrinsics, D45120: [coroutines] Add __builtin_coro_noop =>
llvm.coro.noop.
A recent addition to Coroutines TS (https://wg21.link/p0913) adds a pre-defined
coroutine noop_coroutine that does nothing.
This patch implements require library types in
Related clang and llvm patches:
https://reviews.llvm.org/D45114
https://reviews.llvm.org/D45120
https://reviews.llvm.org/D45121
Files:
include/experimental/coroutine
test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
Index: test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
===
--- /dev/null
+++ test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
@@ -0,0 +1,63 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11
+
+//
+// struct noop_coroutine_promise;
+// using noop_coroutine_handle = coroutine_handle;
+// noop_coroutine_handle noop_coroutine() noexcept;
+
+#include
+#include
+#include
+
+namespace coro = std::experimental;
+
+static_assert(std::is_same, coro::noop_coroutine_handle>::value, "");
+static_assert(std::is_same::value, "");
+
+// template <> struct coroutine_handle : coroutine_handle<>
+// {
+// // 18.11.2.7 noop observers
+// constexpr explicit operator bool() const noexcept;
+// constexpr bool done() const noexcept;
+
+// // 18.11.2.8 noop resumption
+// constexpr void operator()() const noexcept;
+// constexpr void resume() const noexcept;
+// constexpr void destroy() const noexcept;
+
+// // 18.11.2.9 noop promise access
+// noop_coroutine_promise& promise() const noexcept;
+
+// // 18.11.2.10 noop address
+// constexpr void* address() const noexcept;
+
+int main()
+{
+ auto h = coro::noop_coroutine();
+ coro::coroutine_handle<> base = h;
+
+ assert(h);
+ assert(base);
+
+ assert(!h.done());
+ assert(!base.done());
+
+ h.resume();
+ h.destroy();
+ h();
+
+ h.promise();
+ assert(h.address() == base.address());
+ assert(h.address() != nullptr);
+}
+
Index: include/experimental/coroutine
===
--- include/experimental/coroutine
+++ include/experimental/coroutine
@@ -259,6 +259,43 @@
}
};
+struct noop_coroutine_promise {};
+
+template <>
+class _LIBCPP_TEMPLATE_VIS coroutine_handle
+: public coroutine_handle<> {
+ using _Base = coroutine_handle<>;
+ using _Promise = noop_coroutine_promise;
+public:
+
+_LIBCPP_INLINE_VISIBILITY
+_Promise& promise() const {
+return *reinterpret_cast<_Promise*>(
+__builtin_coro_promise(this->__handle_, __alignof(_Promise), false));
+}
+
+constexpr explicit operator bool() const noexcept { return true; }
+constexpr bool done() const noexcept { return false; }
+
+constexpr void operator()() const noexcept {}
+constexpr void resume() const noexcept {}
+constexpr void destroy() const noexcept {}
+
+private:
+friend coroutine_handle noop_coroutine() _NOEXCEPT;
+
+coroutine_handle() {
+ this->__handle_ = __builtin_coro_noop();
+}
+};
+
+using noop_coroutine_handle = coroutine_handle;
+
+inline _LIBCPP_ALWAYS_INLINE
+noop_coroutine_handle noop_coroutine() _NOEXCEPT {
+ return {};
+}
+
struct _LIBCPP_TYPE_VIS suspend_never {
_LIBCPP_ALWAYS_INLINE
bool await_ready() const _NOEXCEPT { return true; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45120: [coroutines] Add __builtin_coro_noop => llvm.coro.noop
GorNishanov closed this revision. GorNishanov added a comment. Committed: https://reviews.llvm.org/rC328993 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@328993 91177308-0d34-0410-b5e6-96231b3b80d8 https://reviews.llvm.org/D45120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45121: [coroutines] Add noop_coroutine to
GorNishanov added a comment. @EricWF , gentle ping. Super tini-tiny change. Last piece missing to be post-Jax 2018 compilant https://reviews.llvm.org/D45121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45121: [coroutines] Add noop_coroutine to
GorNishanov added a comment. In https://reviews.llvm.org/D45121#1056408, @lewissbaker wrote: > The `coroutine_handle` type does not have a > `from_address()` or a `from_promise()` static functions in the same way that > the `coroutine_handle` implementation does. > Is this intentional or an oversight in the TS wording? > > They don't seem hugely useful, so I'm not that worried. > If you know that you have a `coroutine_handle` then > you can just use `noop_coroutine()` to get the handle instead. This is intentional. The only way to get a noop coroutine is to ask for it via `noop_coroutine()` https://reviews.llvm.org/D45121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45121: [coroutines] Add noop_coroutine to
GorNishanov updated this revision to Diff 140903.
GorNishanov added a comment.
incorporated review feedback
https://reviews.llvm.org/D45121
Files:
include/experimental/coroutine
test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
Index: test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
===
--- /dev/null
+++ test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
@@ -0,0 +1,64 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11
+// XFAIL: clang-5, clang-6
+//
+// struct noop_coroutine_promise;
+// using noop_coroutine_handle = coroutine_handle;
+// noop_coroutine_handle noop_coroutine() noexcept;
+
+#include
+#include
+#include
+
+namespace coro = std::experimental;
+
+static_assert(std::is_same, coro::noop_coroutine_handle>::value, "");
+static_assert(std::is_same::value, "");
+
+// template <> struct coroutine_handle : coroutine_handle<>
+// {
+// // 18.11.2.7 noop observers
+// constexpr explicit operator bool() const noexcept;
+// constexpr bool done() const noexcept;
+
+// // 18.11.2.8 noop resumption
+// constexpr void operator()() const noexcept;
+// constexpr void resume() const noexcept;
+// constexpr void destroy() const noexcept;
+
+// // 18.11.2.9 noop promise access
+// noop_coroutine_promise& promise() const noexcept;
+
+// // 18.11.2.10 noop address
+// constexpr void* address() const noexcept;
+
+int main()
+{
+ auto h = coro::noop_coroutine();
+ coro::coroutine_handle<> base = h;
+
+ assert(h);
+ assert(base);
+
+ assert(!h.done());
+ assert(!base.done());
+
+ h.resume();
+ h.destroy();
+ h();
+
+ h.promise();
+ assert(h.address() == base.address());
+ assert(h.address() != nullptr);
+ assert(coro::coroutine_handle<>::from_address(h.address()) == base);
+}
+
Index: include/experimental/coroutine
===
--- include/experimental/coroutine
+++ include/experimental/coroutine
@@ -259,6 +259,45 @@
}
};
+#if __has_builtin(__builtin_coro_noop)
+struct noop_coroutine_promise {};
+
+template <>
+class _LIBCPP_TEMPLATE_VIS coroutine_handle
+: public coroutine_handle<> {
+ using _Base = coroutine_handle<>;
+ using _Promise = noop_coroutine_promise;
+public:
+
+_LIBCPP_INLINE_VISIBILITY
+_Promise& promise() const {
+return *reinterpret_cast<_Promise*>(
+__builtin_coro_promise(this->__handle_, __alignof(_Promise), false));
+}
+
+constexpr explicit operator bool() const noexcept { return true; }
+constexpr bool done() const noexcept { return false; }
+
+constexpr void operator()() const noexcept {}
+constexpr void resume() const noexcept {}
+constexpr void destroy() const noexcept {}
+
+private:
+friend coroutine_handle noop_coroutine() _NOEXCEPT;
+
+coroutine_handle() {
+ this->__handle_ = __builtin_coro_noop();
+}
+};
+
+using noop_coroutine_handle = coroutine_handle;
+
+inline _LIBCPP_INLINE_VISIBILITY
+noop_coroutine_handle noop_coroutine() _NOEXCEPT {
+ return {};
+}
+#endif // __has_builtin(__builtin_coro_noop)
+
struct _LIBCPP_TYPE_VIS suspend_never {
_LIBCPP_ALWAYS_INLINE
bool await_ready() const _NOEXCEPT { return true; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45121: [coroutines] Add noop_coroutine to
GorNishanov marked 2 inline comments as done.
GorNishanov added inline comments.
Comment at: include/experimental/coroutine:294
+
+inline _LIBCPP_ALWAYS_INLINE
+noop_coroutine_handle noop_coroutine() _NOEXCEPT {
lewissbaker wrote:
> EricWF wrote:
> > This should just be `_LIBCPP_INLINE_VISIBILITY`. We try not to use
> > `_LIBCPP_ALWAYS_INLINE` in new code.
> Should the same change be applied to the other usages of
> `_LIBCPP_ALWAYS_INLINE` in this file?
> Should some of them be marked `constexpr` to be consistent with
> `noop_coroutine_handle` member functions above?
Those were added by @EricWF, so from my perspective they are immutable.
Comment at: include/experimental/coroutine:288
+
+coroutine_handle() {
+ this->__handle_ = __builtin_coro_noop();
EricWF wrote:
> Can `__builtin_coro_noop` produce a constant expression?
No. llvm generates this value, so from clang perspective, it is not a constant.
At llvm level it is a private per TU constant, so invocations of
noop_coroutine() in different TUs linked into the same program will return you
different values.
https://reviews.llvm.org/D45121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45121: [coroutines] Add noop_coroutine to
GorNishanov updated this revision to Diff 140983.
GorNishanov marked 2 inline comments as done.
GorNishanov added a comment.
- static_cast instead of reintrepret_cast in promise()
- 2 spaces indent in added code (the rest of the file stayed as is)
- added static_assert to check for done-ness and capacity
- constexpr => _LIBCPP_CONSTEXPR
- noexcept => _NOEXCEPT
https://reviews.llvm.org/D45121
Files:
include/experimental/coroutine
test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
Index: test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
===
--- /dev/null
+++ test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
@@ -0,0 +1,66 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11
+// XFAIL: clang-5, clang-6
+//
+// struct noop_coroutine_promise;
+// using noop_coroutine_handle = coroutine_handle;
+// noop_coroutine_handle noop_coroutine() noexcept;
+
+#include
+#include
+#include
+
+namespace coro = std::experimental::coroutines_v1;
+
+static_assert(std::is_same, coro::noop_coroutine_handle>::value, "");
+static_assert(std::is_same::value, "");
+
+// template <> struct coroutine_handle : coroutine_handle<>
+// {
+// // 18.11.2.7 noop observers
+// constexpr explicit operator bool() const noexcept;
+// constexpr bool done() const noexcept;
+
+// // 18.11.2.8 noop resumption
+// constexpr void operator()() const noexcept;
+// constexpr void resume() const noexcept;
+// constexpr void destroy() const noexcept;
+
+// // 18.11.2.9 noop promise access
+// noop_coroutine_promise& promise() const noexcept;
+
+// // 18.11.2.10 noop address
+// constexpr void* address() const noexcept;
+
+int main()
+{
+ auto h = coro::noop_coroutine();
+ coro::coroutine_handle<> base = h;
+
+ assert(h);
+ assert(base);
+
+ assert(!h.done());
+ assert(!base.done());
+
+ h.resume();
+ h.destroy();
+ h();
+ static_assert(h.done() == false, "");
+ static_assert(h, "");
+
+ h.promise();
+ assert(h.address() == base.address());
+ assert(h.address() != nullptr);
+ assert(coro::coroutine_handle<>::from_address(h.address()) == base);
+}
+
Index: include/experimental/coroutine
===
--- include/experimental/coroutine
+++ include/experimental/coroutine
@@ -213,7 +213,7 @@
_LIBCPP_INLINE_VISIBILITY
_Promise& promise() const {
-return *reinterpret_cast<_Promise*>(
+return *static_cast<_Promise*>(
__builtin_coro_promise(this->__handle_, __alignof(_Promise), false));
}
@@ -259,6 +259,45 @@
}
};
+#if __has_builtin(__builtin_coro_noop)
+struct noop_coroutine_promise {};
+
+template <>
+class _LIBCPP_TEMPLATE_VIS coroutine_handle
+: public coroutine_handle<> {
+ using _Base = coroutine_handle<>;
+ using _Promise = noop_coroutine_promise;
+public:
+
+ _LIBCPP_INLINE_VISIBILITY
+ _Promise& promise() const {
+return *static_cast<_Promise*>(
+ __builtin_coro_promise(this->__handle_, __alignof(_Promise), false));
+ }
+
+ _LIBCPP_CONSTEXPR explicit operator bool() const _NOEXCEPT { return true; }
+ _LIBCPP_CONSTEXPR bool done() const _NOEXCEPT { return false; }
+
+ _LIBCPP_CONSTEXPR void operator()() const _NOEXCEPT {}
+ _LIBCPP_CONSTEXPR void resume() const _NOEXCEPT {}
+ _LIBCPP_CONSTEXPR void destroy() const _NOEXCEPT {}
+
+private:
+ friend coroutine_handle noop_coroutine() _NOEXCEPT;
+
+ coroutine_handle() _NOEXCEPT {
+this->__handle_ = __builtin_coro_noop();
+ }
+};
+
+using noop_coroutine_handle = coroutine_handle;
+
+inline _LIBCPP_INLINE_VISIBILITY
+noop_coroutine_handle noop_coroutine() _NOEXCEPT {
+ return {};
+}
+#endif // __has_builtin(__builtin_coro_noop)
+
struct _LIBCPP_TYPE_VIS suspend_never {
_LIBCPP_ALWAYS_INLINE
bool await_ready() const _NOEXCEPT { return true; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45121: [coroutines] Add noop_coroutine to
GorNishanov marked 11 inline comments as done.
GorNishanov added inline comments.
Comment at: include/experimental/coroutine:275
+return *reinterpret_cast<_Promise*>(
+__builtin_coro_promise(this->__handle_, __alignof(_Promise),
false));
+}
CaseyCarter wrote:
> Is `this->non_static_member_of_non_dependent_base_class` idiomatic in libc++?
> I typically reserve `this->` for forcing lookup into dependent bases. (and on
> 217.)
I am keeping it consistent with other uses in the file.
Comment at: include/experimental/coroutine:288
+
+coroutine_handle() {
+ this->__handle_ = __builtin_coro_noop();
CaseyCarter wrote:
> CaseyCarter wrote:
> > GorNishanov wrote:
> > > EricWF wrote:
> > > > Can `__builtin_coro_noop` produce a constant expression?
> > > No. llvm generates this value, so from clang perspective, it is not a
> > > constant.
> > > At llvm level it is a private per TU constant, so invocations of
> > > noop_coroutine() in different TUs linked into the same program will
> > > return you different values.
> > If this class type is not literal - since there's no `constexpr`
> > constructor - applying `constexpr` to the member functions on 278-283 is at
> > best misleading and at worst ill-formed NDR due to
> > http://eel.is/c++draft/dcl.constexpr#5.
> This constructor should be `_NOEXCEPT`, since it's invoked by
> `noop_coroutine` which is `_NOEXCEPT`.
Issue for Rapperswil? These constexprs were approved by LEWG/LWG in
Jacksonville 2018
https://reviews.llvm.org/D45121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45121: [coroutines] Add noop_coroutine to
GorNishanov marked an inline comment as done.
GorNishanov added inline comments.
Comment at: include/experimental/coroutine:294
+
+inline _LIBCPP_ALWAYS_INLINE
+noop_coroutine_handle noop_coroutine() _NOEXCEPT {
EricWF wrote:
> GorNishanov wrote:
> > lewissbaker wrote:
> > > EricWF wrote:
> > > > This should just be `_LIBCPP_INLINE_VISIBILITY`. We try not to use
> > > > `_LIBCPP_ALWAYS_INLINE` in new code.
> > > Should the same change be applied to the other usages of
> > > `_LIBCPP_ALWAYS_INLINE` in this file?
> > > Should some of them be marked `constexpr` to be consistent with
> > > `noop_coroutine_handle` member functions above?
> > Those were added by @EricWF, so from my perspective they are immutable.
> I'm not sure what I was thinking when I implemented these things with
> `_LIBCPP_ALWAYS_INLINE`.
@EricWF would you like me to do wholesale search-and-replace in coroutine
header and make everything _LIBCPP_INLINE_VISIBILITY. Though, strategic use of
always_inline in coroutine_handle and related classes can allow heap allocation
elision to work even in -O0
https://reviews.llvm.org/D45121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45121: [coroutines] Add noop_coroutine to
GorNishanov updated this revision to Diff 141010.
GorNishanov added a comment.
- s/_LIBCPP_ALWAYS_INLINE/_LIBCPP_INLINE_VISIBILITY throughout
- Added _LIBCPP_INLINE_VISIBILITY to noop_coroutine constructor
@EricWF , good to go?
https://reviews.llvm.org/D45121
Files:
include/experimental/coroutine
test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
Index: test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
===
--- /dev/null
+++ test/std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.noop/noop_coroutine.pass.cpp
@@ -0,0 +1,66 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11
+// XFAIL: clang-5, clang-6
+//
+// struct noop_coroutine_promise;
+// using noop_coroutine_handle = coroutine_handle;
+// noop_coroutine_handle noop_coroutine() noexcept;
+
+#include
+#include
+#include
+
+namespace coro = std::experimental::coroutines_v1;
+
+static_assert(std::is_same, coro::noop_coroutine_handle>::value, "");
+static_assert(std::is_same::value, "");
+
+// template <> struct coroutine_handle : coroutine_handle<>
+// {
+// // 18.11.2.7 noop observers
+// constexpr explicit operator bool() const noexcept;
+// constexpr bool done() const noexcept;
+
+// // 18.11.2.8 noop resumption
+// constexpr void operator()() const noexcept;
+// constexpr void resume() const noexcept;
+// constexpr void destroy() const noexcept;
+
+// // 18.11.2.9 noop promise access
+// noop_coroutine_promise& promise() const noexcept;
+
+// // 18.11.2.10 noop address
+// constexpr void* address() const noexcept;
+
+int main()
+{
+ auto h = coro::noop_coroutine();
+ coro::coroutine_handle<> base = h;
+
+ assert(h);
+ assert(base);
+
+ assert(!h.done());
+ assert(!base.done());
+
+ h.resume();
+ h.destroy();
+ h();
+ static_assert(h.done() == false, "");
+ static_assert(h, "");
+
+ h.promise();
+ assert(h.address() == base.address());
+ assert(h.address() != nullptr);
+ assert(coro::coroutine_handle<>::from_address(h.address()) == base);
+}
+
Index: include/experimental/coroutine
===
--- include/experimental/coroutine
+++ include/experimental/coroutine
@@ -93,60 +93,60 @@
template <>
class _LIBCPP_TEMPLATE_VIS coroutine_handle {
public:
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
_LIBCPP_CONSTEXPR coroutine_handle() _NOEXCEPT : __handle_(nullptr) {}
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
_LIBCPP_CONSTEXPR coroutine_handle(nullptr_t) _NOEXCEPT : __handle_(nullptr) {}
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
coroutine_handle& operator=(nullptr_t) _NOEXCEPT {
__handle_ = nullptr;
return *this;
}
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
_LIBCPP_CONSTEXPR void* address() const _NOEXCEPT { return __handle_; }
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
_LIBCPP_CONSTEXPR explicit operator bool() const _NOEXCEPT { return __handle_; }
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
void operator()() { resume(); }
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
void resume() {
_LIBCPP_ASSERT(__is_suspended(),
"resume() can only be called on suspended coroutines");
_LIBCPP_ASSERT(!done(),
"resume() has undefined behavior when the coroutine is done");
__builtin_coro_resume(__handle_);
}
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
void destroy() {
_LIBCPP_ASSERT(__is_suspended(),
"destroy() can only be called on suspended coroutines");
__builtin_coro_destroy(__handle_);
}
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
bool done() const {
_LIBCPP_ASSERT(__is_suspended(),
"done() can only be called on suspended coroutines");
return __builtin_coro_done(__handle_);
}
public:
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
static coroutine_handle from_address(void* __addr) _NOEXCEPT {
coroutine_handle __tmp;
__tmp.__handle_ = __addr;
return __tmp;
}
// FIXME: Should from_address(nullptr) be allowed?
-_LIBCPP_ALWAYS_INLINE
+_LIBCPP_INLINE_VISIBILITY
static coroutine_handle from_address(nullptr_t) _NOEXCEPT {
return coroutine_handle(nullptr);
}
