EricWF updated this revision to Diff 95074. EricWF added a comment. This revision is now accepted and ready to land.
- Add call to `MarkDeclRefReferenced` even though it doesn't prevent the assertion. https://reviews.llvm.org/D31562 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/CoroutineBuilder.h lib/Sema/SemaCoroutine.cpp test/CodeGenCoroutines/coro-alloc.cpp test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp =================================================================== --- test/SemaCXX/coroutines.cpp +++ test/SemaCXX/coroutines.cpp @@ -654,6 +654,18 @@ 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,24 @@ } template coro<bad_promise_11> dependent_private_alloc_failure_handler(bad_promise_11); // expected-note@-1 {{requested here}} + +struct bad_promise_12 { + coro<bad_promise_12> get_return_object(); + suspend_always initial_suspend(); + suspend_always final_suspend(); + void unhandled_exception(); + void return_void(); + static coro<bad_promise_12> 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<bad_promise_12> throwing_in_class_new() { // expected-note {{call to 'operator new' implicitly required by coroutine function here}} + co_return; +} + +template <class T> +coro<T> dependent_throwing_in_class_new(T) { // expected-note {{call to 'operator new' implicitly required by coroutine function here}} + co_return; +} +template coro<bad_promise_12> dependent_throwing_in_class_new(bad_promise_12); // expected-note {{requested here}} Index: test/CodeGenCoroutines/coro-alloc.cpp =================================================================== --- test/CodeGenCoroutines/coro-alloc.cpp +++ test/CodeGenCoroutines/coro-alloc.cpp @@ -19,8 +19,19 @@ coroutine_handle(coroutine_handle<PromiseType>) {} }; -} -} +} // 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; } Index: lib/Sema/SemaCoroutine.cpp =================================================================== --- lib/Sema/SemaCoroutine.cpp +++ lib/Sema/SemaCoroutine.cpp @@ -454,7 +454,7 @@ /*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,43 @@ 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: <experimental/coroutine> should have been included already. + // If we require it to include <new> 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<VarDecl>(); + 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; + + // FIXME: This doesn't work. The assertion `MaybeODRUseExprs.empty()` in + // ActOnFinishFunctionBody still fires. + S.MarkDeclRefReferenced(cast<DeclRefExpr>(DR.get())); + + return DR.get(); +} + // Find an appropriate delete for the promise. static FunctionDecl *findDeleteForPromise(Sema &S, SourceLocation Loc, QualType PromiseType) { @@ -847,23 +884,51 @@ 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<CXXRecordDecl>(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<FunctionProtoType>(); + 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 +944,12 @@ if (NewRef.isInvalid()) return false; + SmallVector<Expr *, 2> 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); if (NewExpr.isInvalid()) return false; Index: lib/Sema/CoroutineBuilder.h =================================================================== --- /dev/null +++ lib/Sema/CoroutineBuilder.h @@ -0,0 +1,83 @@ +//===----- CoroutineBuilder.h - Coroutine Semantic checking -----*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +//===----------------------------------------------------------------------===// +// +// This file implements a semantic tree transformation that takes a given +// AST and rebuilds it, possibly transforming some nodes in the process. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_SEMA_COROUTINEBUILDER_H +#define LLVM_CLANG_LIB_SEMA_COROUTINEBUILDER_H + +#include "clang/AST/Decl.h" +#include "clang/AST/ExprCXX.h" +#include "clang/AST/StmtCXX.h" +#include "clang/Lex/Preprocessor.h" +#include "clang/Sema/SemaInternal.h" + +namespace clang { + +class CoroutineStmtBuilder : public CoroutineBodyStmt::CtorArgs { + Sema &S; + FunctionDecl &FD; + sema::FunctionScopeInfo &Fn; + bool IsValid = true; + SourceLocation Loc; + QualType RetType; + SmallVector<Stmt *, 4> ParamMovesVector; + const bool IsPromiseDependentType; + CXXRecordDecl *PromiseRecordDecl = nullptr; + +public: + CoroutineStmtBuilder(Sema &S, FunctionDecl &FD, sema::FunctionScopeInfo &Fn, + Stmt *Body) + : S(S), FD(FD), Fn(Fn), Loc(FD.getLocation()), + IsPromiseDependentType( + !Fn.CoroutinePromise || + Fn.CoroutinePromise->getType()->isDependentType()) { + this->Body = Body; + if (!IsPromiseDependentType) { + PromiseRecordDecl = Fn.CoroutinePromise->getType()->getAsCXXRecordDecl(); + assert(PromiseRecordDecl && "Type should have already been checked"); + } + this->IsValid = makePromiseStmt() && makeInitialAndFinalSuspend(); + } + + bool buildStatements() { + assert(this->IsValid && "coroutine already invalid"); + this->IsValid = makeReturnObject() && makeParamMoves(); + if (this->IsValid && !IsPromiseDependentType) + buildDependentStatements(); + return this->IsValid; + } + + bool buildDependentStatements() { + assert(this->IsValid && "coroutine already invalid"); + assert(!this->IsPromiseDependentType && + "coroutine cannot have a dependent promise type"); + this->IsValid = makeOnException() && makeOnFallthrough() && + makeReturnOnAllocFailure() && makeNewAndDeleteExpr(); + return this->IsValid; + } + + bool isInvalid() const { return !this->IsValid; } + +private: + bool makePromiseStmt(); + bool makeInitialAndFinalSuspend(); + bool makeNewAndDeleteExpr(); + bool makeOnFallthrough(); + bool makeOnException(); + bool makeReturnObject(); + bool makeReturnOnAllocFailure(); + bool makeParamMoves(); +}; + +} // end namespace clang + +#endif // LLVM_CLANG_LIB_SEMA_COROUTINEBUILDER_H Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -8854,6 +8854,11 @@ def err_implied_coroutine_type_not_found : Error< "%0 type was not found; include <experimental/coroutine> before defining " "a coroutine">; +def err_implicit_coroutine_std_nothrow_type_not_found : Error< + "std::nothrow was not found; include <new> 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 @@ "%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 @@ InGroup<Coroutine>; 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 {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits