[PATCH] D39207: [llvm-objcopy] Add support for dwarf fission

[James Henderson via Phabricator via llvm-commits]

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

LGTM. The change as it stands is fine, but I don't understand why you've made a couple of changes, that I don't remember seeing before (see inline comments). If there's a good reason for them, feel free to put the changes in as part of this, or to remove them if they're not supposed to be there.

In https://reviews.llvm.org/D39207#914752, @jakehehrlich wrote:

> Also, as more stripping options become available I think coming up with a system to handle the removal predicate would be nice. I might write that patch soon.


Sounds good.



================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:81
 
-using SectionPred = std::function<bool(const SectionBase &Sec)>;
+cl::list<std::string> ToRemove("remove-section",
+                               cl::desc("Remove a specific section"));
----------------
Why the removal of static here and in the following lines?


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:139
+
+typedef std::function<bool(const SectionBase &Sec)> SectionPred;
+
----------------
Why the change here?


Repository:
  rL LLVM

https://reviews.llvm.org/D39207