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

[Richard Smith via cfe-commits]

rsmith added inline comments.


================
Comment at: include/clang/AST/ExprCXX.h:4256
+/// is dependent.
+class CoawaitDependentExpr : public Expr {
+  SourceLocation KeywordLoc;
----------------
Rename -> `DependentCoawaitExpr`, to match our other `Dependent*Expr` classes.


================
Comment at: include/clang/AST/ExprCXX.h:4259
+  Stmt *Operand;
+  UnresolvedSet<16> CoawaitOperatorCandidates;
+
----------------
Would it make sense to store this as an `UnresolvedLookupExpr*` instead?


================
Comment at: lib/AST/ExprClassification.cpp:191
   case Expr::DesignatedInitUpdateExprClass:
+  // FIXME How should we classify co_await expressions while they're still
+  // dependent?
----------------
EricWF wrote:
> @rsmith: What's the correct way to classify this type?
See above:
```
    // Unresolved lookups and uncorrected typos get classified as lvalues.
    // FIXME: Is this wise? Should they get their own kind?
```
It shouldn't ever matter what value category we give to a type-dependent expression.


================
Comment at: lib/Sema/TreeTransform.h:6707-6708
+  // context or if the promise type has changed.
+  return getDerived().RebuildCoawaitDependentExpr(
+      E->getKeywordLoc(), Result.get(), E->getOperatorCandidates());
+}
----------------
This should ideally call into the same function we use for the non-dependent case.


https://reviews.llvm.org/D26057