[clang] f3d4e16 - [C++20] Conform coroutine's comments in clang (NFC-ish)
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 23 20:42:46 PST 2021
Author: Chuanqi Xu
Date: 2021-12-24T12:41:44+08:00
New Revision: f3d4e168dbc7e736ef535c673ee635a40feb2037
URL: https://github.com/llvm/llvm-project/commit/f3d4e168dbc7e736ef535c673ee635a40feb2037
DIFF: https://github.com/llvm/llvm-project/commit/f3d4e168dbc7e736ef535c673ee635a40feb2037.diff
LOG: [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.
Added:
Modified:
clang/lib/CodeGen/CGCoroutine.cpp
clang/lib/Sema/SemaCoroutine.cpp
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index ca071d3d2e80f..2041d2a5b4c95 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 27ba49fb001c0..336e0c5f1d465 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<FunctionDecl>(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: <coroutine> should have been included already.
- // If we require it to include <new> then this diagnostic is no longer
- // needed.
+ // <coroutine> is not requred to include <new>, 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<Expr *, 1> 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<R, P1, ...,
+ // Pn>`
+ // , 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<CXXMethodDecl>(&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<Expr *, 2> 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<FunctionProtoType>();
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);
More information about the cfe-commits
mailing list