From: Eric Fiselier Date: Mon, 17 Apr 2017 22:06:13 +0000 (+0000) Subject: [coroutines] Fix rebuilding of implicit and dependent coroutine statements. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1772bdb2438378c9da3ce2eff55c81f1804d6df1;p=clang [coroutines] Fix rebuilding of implicit and dependent coroutine statements. Summary: Certain implicitly generated coroutine statements, such as the calls to 'return_value()' or `return_void()` or `get_return_object_on_allocation_failure()`, cannot be built until the promise type is no longer dependent. This means they are not built until after the coroutine body statement has been transformed. This patch fixes an issue where these statements would never be built for coroutine templates. It also fixes a small issue where diagnostics about `get_return_object_on_allocation_failure()` were incorrectly suppressed. Reviewers: rsmith, majnemer, GorNishanov, aaron.ballman Reviewed By: GorNishanov Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D31487 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@300504 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 9b2cfe495c..e2f2cbf6e6 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -8854,6 +8854,11 @@ def err_coroutine_invalid_func_context : Error< def err_implied_coroutine_type_not_found : Error< "%0 type was not found; include before defining " "a coroutine">; +def err_implicit_coroutine_std_nothrow_type_not_found : Error< + "std::nothrow was not found; include before defining a coroutine which " + "uses get_return_object_on_allocation_failure()">; +def err_malformed_std_nothrow : Error< + "std::nothrow must be a valid variable declaration">; def err_malformed_std_coroutine_handle : Error< "std::experimental::coroutine_handle must be a class template">; def err_coroutine_handle_missing_member : Error< @@ -8873,7 +8878,7 @@ def err_coroutine_promise_return_ill_formed : Error< "%0 declares both 'return_value' and 'return_void'">; def note_coroutine_promise_implicit_await_transform_required_here : Note< "call to 'await_transform' implicitly required by 'co_await' here">; -def note_coroutine_promise_call_implicitly_required : Note< +def note_coroutine_promise_suspend_implicitly_required : Note< "call to '%select{initial_suspend|final_suspend}0' implicitly " "required by the %select{initial suspend point|final suspend point}0">; def err_coroutine_promise_unhandled_exception_required : Error< @@ -8883,6 +8888,11 @@ def warn_coroutine_promise_unhandled_exception_required_with_exceptions : Warnin InGroup; def err_coroutine_promise_get_return_object_on_allocation_failure : Error< "%0: 'get_return_object_on_allocation_failure()' must be a static member function">; +def err_coroutine_promise_new_requires_nothrow : Error< + "%0 is required to have a non-throwing noexcept specification when the promise " + "type declares 'get_return_object_on_allocation_failure()'">; +def note_coroutine_promise_call_implicitly_required : Note< + "call to %0 implicitly required by coroutine function here">; } let CategoryName = "Documentation Issue" in { diff --git a/lib/Sema/SemaCoroutine.cpp b/lib/Sema/SemaCoroutine.cpp index 4a55e51495..f891365efd 100644 --- a/lib/Sema/SemaCoroutine.cpp +++ b/lib/Sema/SemaCoroutine.cpp @@ -454,7 +454,7 @@ static bool actOnCoroutineBodyStart(Sema &S, Scope *SC, SourceLocation KWLoc, /*IsImplicit*/ true); Suspend = S.ActOnFinishFullExpr(Suspend.get()); if (Suspend.isInvalid()) { - S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required) + S.Diag(Loc, diag::note_coroutine_promise_suspend_implicitly_required) << ((Name == "initial_suspend") ? 0 : 1); S.Diag(KWLoc, diag::note_declared_coroutine_here) << Keyword; return StmtError(); @@ -660,6 +660,39 @@ StmtResult Sema::BuildCoreturnStmt(SourceLocation Loc, Expr *E, return Res; } +/// Look up the std::nothrow object. +static Expr *buildStdNoThrowDeclRef(Sema &S, SourceLocation Loc) { + NamespaceDecl *Std = S.getStdNamespace(); + assert(Std && "Should already be diagnosed"); + + LookupResult Result(S, &S.PP.getIdentifierTable().get("nothrow"), Loc, + Sema::LookupOrdinaryName); + if (!S.LookupQualifiedName(Result, Std)) { + // FIXME: should have been included already. + // If we require it to include then this diagnostic is no longer + // needed. + S.Diag(Loc, diag::err_implicit_coroutine_std_nothrow_type_not_found); + return nullptr; + } + + // FIXME: Mark the variable as ODR used. This currently does not work + // likely due to the scope at in which this function is called. + auto *VD = Result.getAsSingle(); + if (!VD) { + Result.suppressDiagnostics(); + // We found something weird. Complain about the first thing we found. + NamedDecl *Found = *Result.begin(); + S.Diag(Found->getLocation(), diag::err_malformed_std_nothrow); + return nullptr; + } + + ExprResult DR = S.BuildDeclRefExpr(VD, VD->getType(), VK_LValue, Loc); + if (DR.isInvalid()) + return nullptr; + + return DR.get(); +} + // Find an appropriate delete for the promise. static FunctionDecl *findDeleteForPromise(Sema &S, SourceLocation Loc, QualType PromiseType) { @@ -847,23 +880,51 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() { if (S.RequireCompleteType(Loc, PromiseType, diag::err_incomplete_type)) return false; - // FIXME: Add nothrow_t placement arg for global alloc - // if ReturnStmtOnAllocFailure != nullptr. + const bool RequiresNoThrowAlloc = ReturnStmtOnAllocFailure != nullptr; + // FIXME: Add support for stateful allocators. FunctionDecl *OperatorNew = nullptr; FunctionDecl *OperatorDelete = nullptr; FunctionDecl *UnusedResult = nullptr; bool PassAlignment = false; + MultiExprArg PlacementArgs = None; S.FindAllocationFunctions(Loc, SourceRange(), /*UseGlobal*/ false, PromiseType, - /*isArray*/ false, PassAlignment, - /*PlacementArgs*/ None, OperatorNew, UnusedResult); + /*isArray*/ false, PassAlignment, PlacementArgs, + OperatorNew, UnusedResult); + + bool IsGlobalOverload = + OperatorNew && !isa(OperatorNew->getDeclContext()); + // If we didn't find a class-local new declaration and non-throwing new + // was is required then we need to lookup the non-throwing global operator + // instead. + if (RequiresNoThrowAlloc && (!OperatorNew || IsGlobalOverload)) { + auto *StdNoThrow = buildStdNoThrowDeclRef(S, Loc); + if (!StdNoThrow) + return false; + PlacementArgs = MultiExprArg(StdNoThrow); + OperatorNew = nullptr; + S.FindAllocationFunctions(Loc, SourceRange(), + /*UseGlobal*/ true, PromiseType, + /*isArray*/ false, PassAlignment, PlacementArgs, + OperatorNew, UnusedResult); + } - OperatorDelete = findDeleteForPromise(S, Loc, PromiseType); + if (OperatorNew && RequiresNoThrowAlloc) { + const auto *FT = OperatorNew->getType()->getAs(); + if (!FT->isNothrow(S.Context, /*ResultIfDependent*/ false)) { + S.Diag(OperatorNew->getLocation(), + diag::err_coroutine_promise_new_requires_nothrow) + << OperatorNew; + S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required) + << OperatorNew; + return false; + } + } - if (!OperatorDelete || !OperatorNew) + if ((OperatorDelete = findDeleteForPromise(S, Loc, PromiseType)) == nullptr) return false; Expr *FramePtr = @@ -879,8 +940,13 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() { if (NewRef.isInvalid()) return false; + SmallVector NewArgs{FrameSize}; + for (auto Arg : PlacementArgs) + NewArgs.push_back(Arg); + ExprResult NewExpr = - S.ActOnCallExpr(S.getCurScope(), NewRef.get(), Loc, FrameSize, Loc); + S.ActOnCallExpr(S.getCurScope(), NewRef.get(), Loc, NewArgs, Loc); + NewExpr = S.ActOnFinishFullExpr(NewExpr.get()); if (NewExpr.isInvalid()) return false; @@ -906,6 +972,7 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() { ExprResult DeleteExpr = S.ActOnCallExpr(S.getCurScope(), DeleteRef.get(), Loc, DeleteArgs, Loc); + DeleteExpr = S.ActOnFinishFullExpr(DeleteExpr.get()); if (DeleteExpr.isInvalid()) return false; diff --git a/test/CodeGenCoroutines/coro-alloc.cpp b/test/CodeGenCoroutines/coro-alloc.cpp index f0a600eabe..003095498b 100644 --- a/test/CodeGenCoroutines/coro-alloc.cpp +++ b/test/CodeGenCoroutines/coro-alloc.cpp @@ -19,8 +19,19 @@ struct coroutine_handle { coroutine_handle(coroutine_handle) {} }; -} -} +} // end namespace experimental + +struct nothrow_t {}; +constexpr nothrow_t nothrow = {}; + +} // end namespace std + +// Required when get_return_object_on_allocation_failure() is defined by +// the promise. +using SizeT = decltype(sizeof(int)); +void* operator new(SizeT __sz, const std::nothrow_t&) noexcept; +void operator delete(void* __p, const std::nothrow_t&) noexcept; + struct suspend_always { bool await_ready() { return false; } @@ -145,7 +156,7 @@ struct std::experimental::coroutine_traits { extern "C" int f4(promise_on_alloc_failure_tag) { // CHECK: %[[ID:.+]] = call token @llvm.coro.id(i32 16 // CHECK: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64() - // CHECK: %[[MEM:.+]] = call i8* @_Znwm(i64 %[[SIZE]]) + // CHECK: %[[MEM:.+]] = call i8* @_ZnwmRKSt9nothrow_t(i64 %[[SIZE]], %"struct.std::nothrow_t"* dereferenceable(1) @_ZStL7nothrow) // CHECK: %[[OK:.+]] = icmp ne i8* %[[MEM]], null // CHECK: br i1 %[[OK]], label %[[OKBB:.+]], label %[[ERRBB:.+]] diff --git a/test/SemaCXX/coroutines.cpp b/test/SemaCXX/coroutines.cpp index e6eaedef4c..856110333d 100644 --- a/test/SemaCXX/coroutines.cpp +++ b/test/SemaCXX/coroutines.cpp @@ -654,6 +654,18 @@ float badly_specialized_coro_handle() { // expected-error {{std::experimental::c co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}} } +namespace std { + struct nothrow_t {}; + constexpr nothrow_t nothrow = {}; +} + +using SizeT = decltype(sizeof(int)); + +void* operator new(SizeT __sz, const std::nothrow_t&) noexcept; +void operator delete(void* __p, const std::nothrow_t&) noexcept; + + + struct promise_on_alloc_failure_tag {}; template<> @@ -694,3 +706,43 @@ coro dependent_private_alloc_failure_handler(T) { } template coro dependent_private_alloc_failure_handler(bad_promise_11); // expected-note@-1 {{requested here}} + +struct bad_promise_12 { + coro get_return_object(); + suspend_always initial_suspend(); + suspend_always final_suspend(); + void unhandled_exception(); + void return_void(); + static coro get_return_object_on_allocation_failure(); + + static void* operator new(SizeT); + // expected-error@-1 2 {{'operator new' is required to have a non-throwing noexcept specification when the promise type declares 'get_return_object_on_allocation_failure()'}} +}; +coro throwing_in_class_new() { // expected-note {{call to 'operator new' implicitly required by coroutine function here}} + co_return; +} + +template +coro dependent_throwing_in_class_new(T) { // expected-note {{call to 'operator new' implicitly required by coroutine function here}} + co_return; +} +template coro dependent_throwing_in_class_new(bad_promise_12); // expected-note {{requested here}} + + +struct good_promise_13 { + coro get_return_object(); + suspend_always initial_suspend(); + suspend_always final_suspend(); + void unhandled_exception(); + void return_void(); + static coro get_return_object_on_allocation_failure(); +}; +coro uses_nothrow_new() { + co_return; +} + +template +coro dependent_uses_nothrow_new(T) { + co_return; +} +template coro dependent_uses_nothrow_new(good_promise_13);