[PATCH] D115007: Dump swift5 reflection section data into dsym bundle generated binary with dsymutil

[Shubham Sandeep Rastogi via Phabricator via llvm-commits]

rastogishubham marked an inline comment as done.
rastogishubham added inline comments.


================
Comment at: llvm/include/llvm/MC/MCObjectFileInfo.h:448
+  void initMachOMCObjectFileInfo(const Triple &T,
+                                 StringRef Swift5ReflectionSegmentName = {});
   void initELFMCObjectFileInfo(const Triple &T, bool Large);
----------------
JDevlieghere wrote:
> rastogishubham wrote:
> > aprantl wrote:
> > > rastogishubham wrote:
> > > > JDevlieghere wrote:
> > > > > Similar as above, this seems really out of place.
> > > > I don't think this makes sense, I do not think the swift5reflectionsegmentname should be part of the Triple, so it has to be it's own argument.
> > > > Similar as above, this seems really out of place.
> > > 
> > > @JDevlieghere can you clarify what you mean? We need to make the reflection segment name configurable. What is it that you are suggesting?
> > @JDevlieghere Hi Jonas, can you please clarify what you mean? As Adrian said we need to make the segment name configurable, and I don't think it is valid to make that part of the Triple.
> > 
> > Thanks!
> I meant that I think it's weird that generic MC interfaces need to know about Swift section names. I understand the need to pass the information, I just don't think it should be part of the interface like this. If I follow the code correctly, the Swift 5 reflection segment name is stored in the MCContext. Instead of passing this the `Swift5ReflectionSegmentName` in here, can it query the `*Ctx` for the name? That would address my concern. 
Oh I don't know why I didn't realize that! Yes the ctx is part of the same MCObjectFileInfo object, so instead of passing it as an argument, it should just be something we should query from within the method itself!

Thank you Jonas, really appreciate your help here!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115007/new/

https://reviews.llvm.org/D115007