[PATCH] D22998: [coroutines] Part 4a: Coroutine Devirtualization: Lower coro.resume and coro.destroy.

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/Transforms/Coroutines/CoroInstr.h:36
@@ +35,3 @@
+class LLVM_LIBRARY_VISIBILITY CoroSubFnInst : public IntrinsicInst {
+  enum { kFrame, kIndex };
+public:
----------------
GorNishanov wrote:
> majnemer wrote:
> > We don't use this naming style for enums, see [[ http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly | here ]]
> s/kFrame/Frame
> s/kIndex/Index ?
> 
> It is a private enum inside of the class, so it does not need a prefix according to the coding guidelines.
> 
> I did want to have it stand out a little to signify that it is a constant. I considered
> 
> s/kFrame/FRAME
> s/kIndex/INDEX ?
> 
> but then it maybe consumed with a macro.
I'd name it Frame and Index.

================
Comment at: lib/Transforms/Coroutines/Coroutines.cpp:82-83
@@ +81,4 @@
+
+Value *coro::LowererBase::makeSubFnCall(Value *Arg, int Index,
+  Instruction *InsertPt) {
+  auto IndexVal = ConstantInt::get(Type::getInt8Ty(C), Index);
----------------
majnemer wrote:
> This doesn't look clang-format'd.
Please clang-format this.


Repository:
  rL LLVM

https://reviews.llvm.org/D22998