[PATCH] D30809: [coroutines] Add codegen for await and yield expressions

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov marked 4 inline comments as done.
GorNishanov added inline comments.


================
Comment at: lib/CodeGen/CGCoroutine.cpp:26
+enum class AwaitKind { Init, Normal, Yield, Final };
+char const *AwaitKindStr[] = {"init", "await", "yield", "final"};
+}
----------------
majnemer wrote:
> I'd move this into buildSuspendSuffixStr.
I prefer to keep AwaitKindStr here, so it is trivial to visually inspect that the order in the array matches the order of the enum.


================
Comment at: lib/CodeGen/CGCoroutine.cpp:85
+  unsigned No = 0;
+  StringRef AwaitKindStr = 0;
+  switch (Kind) {
----------------
rjmccall wrote:
> majnemer wrote:
> > I'd just let the default constructor do its thing.
> Agreed.  Assigning 0 to a StringRef reads very wrong.
I got rid of this one and now use a llvm::StringLiteral array to get names.


================
Comment at: lib/CodeGen/CGCoroutine.cpp:203
+    break;
+  }
+  return nullptr;
----------------
rjmccall wrote:
> This function should return an RValue and take an AggValueSlot, just like every other generically-typed expression emitter.  That way, you can just use EmitAnyExpr here instead of inlining two of three cases.
> 
> And you should implement it in CGExprComplex, of course, and add that as a test case.
Thank you! That was very helpful. The code shrunk and looks nicer now. Not sure, if ignoreResult is needed, but, I added it as well to match EmitAnyExpr.


https://reviews.llvm.org/D30809