From f3d4e168dbc7e736ef535c673ee635a40feb2037 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Fri, 24 Dec 2021 11:48:15 +0800 Subject: [PATCH] [C++20] Conform coroutine's comments in clang (NFC-ish) The comments for coroutine in clang wrote for coroutine-TS. Now coroutine is merged into standard. Try to conform the comments. --- clang/lib/CodeGen/CGCoroutine.cpp | 4 ++ clang/lib/Sema/SemaCoroutine.cpp | 144 +++++++++++++++++++++++++------------- 2 files changed, 101 insertions(+), 47 deletions(-) diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp index ca071d3..2041d2a 100644 --- a/clang/lib/CodeGen/CGCoroutine.cpp +++ b/clang/lib/CodeGen/CGCoroutine.cpp @@ -597,6 +597,10 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) { CGM.getIntrinsic(llvm::Intrinsic::coro_begin), {CoroId, Phi}); CurCoro.Data->CoroBegin = CoroBegin; + // We need to emit `get_­return_­object` first. According to: + // [dcl.fct.def.coroutine]p7 + // The call to get_­return_­object is sequenced before the call to + // initial_­suspend and is invoked at most once. GetReturnObjectManager GroManager(*this, S); GroManager.EmitGroAlloca(); diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp index 27ba49f..336e0c5 100644 --- a/clang/lib/Sema/SemaCoroutine.cpp +++ b/clang/lib/Sema/SemaCoroutine.cpp @@ -237,9 +237,9 @@ static bool isValidCoroutineContext(Sema &S, SourceLocation Loc, // placeholder type shall not be a coroutine." if (FD->getReturnType()->isUndeducedType()) DiagInvalid(DiagAutoRet); - // [dcl.fct.def.coroutine]p1: "The parameter-declaration-clause of the - // coroutine shall not terminate with an ellipsis that is not part of a - // parameter-declaration." + // [dcl.fct.def.coroutine]p1 + // The parameter-declaration-clause of the coroutine shall not terminate with + // an ellipsis that is not part of a parameter-declaration. if (FD->isVariadic()) DiagInvalid(DiagVarargs); @@ -579,8 +579,12 @@ VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) { /*TopLevelOfInitList=*/false, /*TreatUnavailableAsInvalid=*/false); - // Attempt to initialize the promise type with the arguments. - // If that fails, fall back to the promise type's default constructor. + // [dcl.fct.def.coroutine]5.7 + // promise-constructor-arguments is determined as follows: overload + // resolution is performed on a promise constructor call created by + // assembling an argument list q_1 ... q_n . If a viable constructor is + // found ([over.match.viable]), then promise-constructor-arguments is ( q_1 + // , ..., q_n ), otherwise promise-constructor-arguments is empty. if (InitSeq) { ExprResult Result = InitSeq.Perform(*this, Entity, Kind, CtorArgExprs); if (Result.isInvalid()) { @@ -648,6 +652,10 @@ static void checkNoThrow(Sema &S, const Stmt *E, return; } if (ThrowingDecls.empty()) { + // [dcl.fct.def.coroutine]p15 + // The expression co_­await promise.final_­suspend() shall not be + // potentially-throwing ([except.spec]). + // // First time seeing an error, emit the error message. S.Diag(cast(S.CurContext)->getLocation(), diag::err_coroutine_promise_final_suspend_requires_nothrow); @@ -995,9 +1003,8 @@ static Expr *buildStdNoThrowDeclRef(Sema &S, SourceLocation Loc) { 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. + // is not requred to include , so we couldn't omit + // the check here. S.Diag(Loc, diag::err_implicit_coroutine_std_nothrow_type_not_found); return nullptr; } @@ -1029,9 +1036,21 @@ static FunctionDecl *findDeleteForPromise(Sema &S, SourceLocation Loc, auto *PointeeRD = PromiseType->getAsCXXRecordDecl(); assert(PointeeRD && "PromiseType must be a CxxRecordDecl type"); + // [dcl.fct.def.coroutine]p12 + // The deallocation function's name is looked up by searching for it in the + // scope of the promise type. If nothing is found, a search is performed in + // the global scope. if (S.FindDeallocationFunction(Loc, PointeeRD, DeleteName, OperatorDelete)) return nullptr; + // FIXME: We didn't implement following selection: + // [dcl.fct.def.coroutine]p12 + // If both a usual deallocation function with only a pointer parameter and a + // usual deallocation function with both a pointer parameter and a size + // parameter are found, then the selected deallocation function shall be the + // one with two parameters. Otherwise, the selected deallocation function + // shall be the function with one parameter. + if (!OperatorDelete) { // Look for a global declaration. const bool CanProvideSize = S.isCompleteType(Loc, PromiseType); @@ -1062,8 +1081,8 @@ void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) { return; } - // Coroutines [stmt.return]p1: - // A return statement shall not appear in a coroutine. + // [stmt.return.coroutine]p1: + // A coroutine shall not enclose a return statement ([stmt.return]). if (Fn->FirstReturnLoc.isValid()) { assert(Fn->FirstCoroutineStmtLoc.isValid() && "first coroutine location not set"); @@ -1164,12 +1183,15 @@ bool CoroutineStmtBuilder::makeReturnOnAllocFailure() { assert(!IsPromiseDependentType && "cannot make statement while the promise type is dependent"); - // [dcl.fct.def.coroutine]/8 - // The unqualified-id get_return_object_on_allocation_failure is looked up in - // the scope of class P by class member access lookup (3.4.5). ... - // If an allocation function returns nullptr, ... the coroutine return value - // is obtained by a call to ... get_return_object_on_allocation_failure(). - + // [dcl.fct.def.coroutine]p10 + // If a search for the name get_­return_­object_­on_­allocation_­failure in + // the scope of the promise type ([class.member.lookup]) finds any + // declarations, then the result of a call to an allocation function used to + // obtain storage for the coroutine state is assumed to return nullptr if it + // fails to obtain storage, ... If the allocation function returns nullptr, + // ... and the return value is obtained by a call to + // T::get_­return_­object_­on_­allocation_­failure(), where T is the + // promise type. DeclarationName DN = S.PP.getIdentifierInfo("get_return_object_on_allocation_failure"); LookupResult Found(S, DN, Loc, Sema::LookupMemberName); @@ -1215,12 +1237,11 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() { const bool RequiresNoThrowAlloc = ReturnStmtOnAllocFailure != nullptr; - // [dcl.fct.def.coroutine]/7 - // Lookup allocation functions using a parameter list composed of the - // requested size of the coroutine state being allocated, followed by - // the coroutine function's arguments. If a matching allocation function - // exists, use it. Otherwise, use an allocation function that just takes - // the requested size. + // According to [dcl.fct.def.coroutine]p9, Lookup allocation functions using a + // parameter list composed of the requested size of the coroutine state being + // allocated, followed by the coroutine function's arguments. If a matching + // allocation function exists, use it. Otherwise, use an allocation function + // that just takes the requested size. FunctionDecl *OperatorNew = nullptr; FunctionDecl *OperatorDelete = nullptr; @@ -1228,21 +1249,32 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() { bool PassAlignment = false; SmallVector PlacementArgs; - // [dcl.fct.def.coroutine]/7 - // "The allocation function’s name is looked up in the scope of P. - // [...] If the lookup finds an allocation function in the scope of P, - // overload resolution is performed on a function call created by assembling - // an argument list. The first argument is the amount of space requested, - // and has type std::size_t. The lvalues p1 ... pn are the succeeding - // arguments." + // [dcl.fct.def.coroutine]p9 + // An implementation may need to allocate additional storage for a + // coroutine. + // This storage is known as the coroutine state and is obtained by calling a + // non-array allocation function ([basic.stc.dynamic.allocation]). The + // allocation function's name is looked up by searching for it in the scope of + // the promise type. + // - If any declarations are found, overload resolution is performed on a + // function call created by assembling an argument list. 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 as: // - // [dcl.fct.def.coroutine]/3 - // "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. Let p1 ... pn be lvalues denoting those objects." + // [dcl.fct.def.coroutine]p3 + // The promise type of a coroutine is `std::coroutine_traits` + // , where R is the return type of the function, and `P1, ..., Pn` are the + // sequence of types of the non-object function parameters, preceded by the + // type of the object parameter ([dcl.fct]) if the coroutine is a non-static + // member function. [dcl.fct.def.coroutine]p4 In the following, p_i is an + // lvalue of type P_i, where p1 denotes the object parameter and p_i+1 denotes + // the i-th non-object function parameter for a non-static member function, + // and p_i denotes the i-th function parameter otherwise. For a non-static + // member function, q_1 is an lvalue that denotes *this; any other q_i is an + // lvalue that denotes the parameter copy corresponding to p_i. if (auto *MD = dyn_cast(&FD)) { if (MD->isInstance() && !isLambdaCallOperator(MD)) { ExprResult ThisExpr = S.ActOnCXXThis(Loc); @@ -1273,10 +1305,10 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() { /*isArray*/ false, PassAlignment, PlacementArgs, OperatorNew, UnusedResult, /*Diagnose*/ false); - // [dcl.fct.def.coroutine]/7 - // "If no matching function is found, overload resolution is performed again - // on a function call created by passing just the amount of space required as - // an argument of type std::size_t." + // [dcl.fct.def.coroutine]p9 + // If no viable function is found ([over.match.viable]), overload resolution + // is performed again on a function call created by passing just the amount of + // space required as an argument of type std::size_t. if (!OperatorNew && !PlacementArgs.empty()) { PlacementArgs.clear(); S.FindAllocationFunctions(Loc, SourceRange(), /*NewScope*/ Sema::AFS_Class, @@ -1285,10 +1317,11 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() { OperatorNew, UnusedResult, /*Diagnose*/ false); } - // [dcl.fct.def.coroutine]/7 - // "The allocation function’s name is looked up in the scope of P. If this - // lookup fails, the allocation function’s name is looked up in the global - // scope." + // [dcl.fct.def.coroutine]p9 + // The allocation function's name is looked up by searching for it in the + // scope of the promise type. + // - If any declarations are found, ... + // - Otherwise, a search is performed in the global scope. if (!OperatorNew) { S.FindAllocationFunctions(Loc, SourceRange(), /*NewScope*/ Sema::AFS_Global, /*DeleteScope*/ Sema::AFS_Both, PromiseType, @@ -1328,8 +1361,12 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() { } } - if ((OperatorDelete = findDeleteForPromise(S, Loc, PromiseType)) == nullptr) + if ((OperatorDelete = findDeleteForPromise(S, Loc, PromiseType)) == nullptr) { + // FIXME: We should add an error here. According to: + // [dcl.fct.def.coroutine]p12 + // If no usual deallocation function is found, the program is ill-formed. return false; + } Expr *FramePtr = S.BuildBuiltinCallExpr(Loc, Builtin::BI__builtin_coro_frame, {}); @@ -1368,7 +1405,11 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() { SmallVector DeleteArgs{CoroFree}; - // Check if we need to pass the size. + // [dcl.fct.def.coroutine]p12 + // The selected deallocation function shall be called with the address of + // the block of storage to be reclaimed as its first argument. If a + // deallocation function with a parameter of type std::size_t is + // used, the size of the block is passed as the corresponding argument. const auto *OpDeleteType = OpDeleteQualType.getTypePtr()->castAs(); if (OpDeleteType->getNumParams() > 1) @@ -1481,8 +1522,9 @@ bool CoroutineStmtBuilder::makeOnException() { } bool CoroutineStmtBuilder::makeReturnObject() { - // Build implicit 'p.get_return_object()' expression and form initialization - // of return type from it. + // [dcl.fct.def.coroutine]p7 + // The expression promise.get_­return_­object() is used to initialize the + // returned reference or prvalue result object of a call to a coroutine. ExprResult ReturnObject = buildPromiseCall(S, Fn.CoroutinePromise, Loc, "get_return_object", None); if (ReturnObject.isInvalid()) @@ -1620,6 +1662,12 @@ bool Sema::buildCoroutineParameterMoves(SourceLocation Loc) { if (!ScopeInfo->CoroutineParameterMoves.empty()) return false; + // [dcl.fct.def.coroutine]p13 + // When a coroutine is invoked, after initializing its parameters + // ([expr.call]), a copy is created for each coroutine parameter. For a + // parameter of type cv T, the copy is a variable of type cv T with + // automatic storage duration that is direct-initialized from an xvalue of + // type T referring to the parameter. for (auto *PD : FD->parameters()) { if (PD->getType()->isDependentType()) continue; @@ -1636,7 +1684,9 @@ bool Sema::buildCoroutineParameterMoves(SourceLocation Loc) { CExpr = castForMoving(*this, PDRefExpr.get()); else CExpr = PDRefExpr.get(); - + // [dcl.fct.def.coroutine]p13 + // The initialization and destruction of each parameter copy occurs in the + // context of the called coroutine. auto D = buildVarDecl(*this, Loc, PD->getType(), PD->getIdentifier()); AddInitializerToDecl(D, CExpr, /*DirectInit=*/true); -- 2.7.4