rjmccall added inline comments.
================
Comment at: lib/CodeGen/CGCoroutine.cpp:32
struct CGCoroData {
+ AwaitKind CurrentAwaitKind = AwaitKind::Init;
----------------
GorNishanov wrote:
> majnemer wrote:
> > Shouldn't this struct be in an anonymous namespace?
> It is forward declared in CodeGenFunction.h and CodeGenFunction class holds a
> unique_ptr to CGCoroData as a member.
> Here is the snippet from CodeGenFunction.h:
>
> ```
> // Holds coroutine data if the current function is a coroutine. We use a
> // wrapper to manage its lifetime, so that we don't have to define
> CGCoroData
> // in this header.
> struct CGCoroInfo {
> std::unique_ptr<CGCoroData> Data;
> CGCoroInfo();
> ~CGCoroInfo();
> };
> CGCoroInfo CurCoro;
> ```
If it's already forward-declared, you should check here that you're matching it
by doing:
struct CodeGen::CGCoroData {
...
};
instead of making namespace declarations.
================
Comment at: lib/CodeGen/CGCoroutine.cpp:42
+ unsigned YieldNum = 0;
unsigned CoreturnCount = 0;
----------------
Comments about what these mean would be useful.
================
Comment at: lib/CodeGen/CGCoroutine.cpp:85
+ unsigned No = 0;
+ StringRef AwaitKindStr = 0;
+ switch (Kind) {
----------------
majnemer wrote:
> I'd just let the default constructor do its thing.
Agreed. Assigning 0 to a StringRef reads very wrong.
================
Comment at: lib/CodeGen/CGCoroutine.cpp:203
+ break;
+ }
+ return nullptr;
----------------
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.
https://reviews.llvm.org/D30809
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
