[PATCH] D27077: LTO: Port the new LTO API to ModuleSymbolTable.

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: llvm/include/llvm/LTO/LTO.h:104
   friend LTO;
-  InputFile() = default;
+  InputFile(MemoryBufferRef MBRef) : MBRef(MBRef) {}
 
----------------
pcc wrote:
> mehdi_amini wrote:
> > Why don't you take all the members here?
> > It seems strange to see the initialization and then `File->Mod = ...`
> We need to create the module using the LLVMContext owned by this class.
> 
> I suppose that given that we need to do that we might as well be consistent with the other members and let MBRef be default initialized as well.
OK, I missed this! But indeed I feel it is less confusing now.


================
Comment at: llvm/include/llvm/LTO/LTO.h:171
     StringRef getName() const { return Name; }
     StringRef getIRName() const {
+      if (isGV())
----------------
pcc wrote:
> mehdi_amini wrote:
> > Non relevant for this patch, but it'd be nice to document getName vs getIRName, I always forget when I look at this.
> It looks like getIRName is only used by the implementation, so I've removed it in r288302 and added documentation for getName.
It seems you need to rebase this patch?


https://reviews.llvm.org/D27077