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

[Mehdi AMINI via llvm-commits]

mehdi_amini added inline comments.

================
Comment at: lib/Transforms/Coroutines/CoroInternal.h:35
@@ +34,3 @@
+  Module& M;
+  LLVMContext& C;
+  FunctionType* const ResumeFnType;
----------------
GorNishanov wrote:
> mehdi_amini wrote:
> > `M` and `C` aren't great variable names. Also, is `C` used outside of the ctor? (The name does not help for grep...)
> I was following examples of similar classes in other components, such as in FunctionImportUtils.h, WholeProgramDevirt.cpp and many others.
> 
> But, I will be happy to change them to:
> 
>   LLVMContext &Context;
>   Module &TheModule; 
> 
> These also seem to be popular choices in the codebase.
> 
> Context is not used outside of the constructor in this patch. It will be in future patches. I can remove it for now.
Yeah, maybe it's me, but I keep single letter variable name for an iterator or other "short-lived" ones.

If you already know for sure you're gonna need soon the Context in multiple methods of this class, I don't mind you leaving it here for now.


Repository:
  rL LLVM

https://reviews.llvm.org/D22998