[PATCH] D57365: [llvm-readobj] Add a flag to dump just the section-to-segment mapping.

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

Aside from the unnecessary pure virtual base class method for `printSectionMapping`, I think this is good to go. I'd like @rupprecht or @grimar to take another look though.



================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:345
+                                   cl::boolOrDefault PrintSectionMapping) = 0;
+  virtual void printSectionMapping(const ELFFile<ELFT> *Obj) = 0;
   virtual void printHashHistogram(const ELFFile<ELFT> *Obj) = 0;
----------------
jhenderson wrote:
> Can you remove this?
Ping? I don't think you need this in the base class interface - it doesn't need to exist as the section mapping printing is a detail of the program header printing, so it probably should be removed.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:4540-4541
+    printProgramHeaders(Obj);
+  if (PrintSectionMapping == cl::BOU_TRUE)
+    printSectionMapping(Obj);
+}
----------------
mattd wrote:
> jhenderson wrote:
> > Given we don't currently support section mapping in LLVM style, does it make sense to have this here?
> We don't need it, I anticipate we will eventually provide a definition to `LLVMStyle<ELFT>::printSectionMapping`.  To me, the function feels incomplete if we take a parameter and do nothing with it.  Additionally, I fear the annoyance of compiler warning messages about unused formal parameters.... if anyone is building this with -Wunused-parameter.  
You can always comment out the name, but I'm okay leaving it in.


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

https://reviews.llvm.org/D57365