[PATCH] D26057: [coroutines] Add CoawaitDependentExpr AST node and use it to properly build await_transform.

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 31 05:17:54 PDT 2016

EricWF marked 10 inline comments as done.
EricWF added inline comments.

Comment at: lib/Sema/SemaCoroutine.cpp:237-244
+static UnresolvedSet<16> lookupOperatorCoawaitCall(Sema &SemaRef, Scope *S,
+                                                   SourceLocation Loc,
+                                                   Expr *E) {
   UnresolvedSet<16> Functions;
   SemaRef.LookupOverloadedOperatorName(OO_Coawait, S, E->getType(), QualType(),
+  return Functions;
ABataev wrote:
> Maybe it is better to add an argument `UnresolvedSetImpl &OpCandidates`?
This seems like the best place to specify the concrete type now that everything else uses `UnresolvedSetImpl`.

Comment at: lib/Sema/SemaCoroutine.cpp:301
+                                   SourceLocation Loc, StringRef Name,
+                                   MutableArrayRef<Expr *> Args) {
+  assert(Coroutine->CoroutinePromise && "no promise for coroutine");
ABataev wrote:
> Why do you need `MutableArrayRef<Epr *>`, when `ArrayRef<Expr *>` is enough? Also `buildMemberCall` must be changed
`Sema::ActOnCallExpr` requires the `MutableArrayRef`, so I don't see how this can be changed.

Comment at: lib/Sema/SemaCoroutine.cpp:331
+Sema::BuildCoawaitDependentExpr(SourceLocation Loc, Expr *E,
+                                const UnresolvedSet<16> &Candidates) {
ABataev wrote:
> s/BuildCoawaitDependentExpr/buildCoawaitDependentExpr/g
Why? Every similar function uses a leading capital.


More information about the cfe-commits mailing list