[PATCH] D11487: llvm-objdump --section, -j section filtering

Colin LeMahieu colinl at codeaurora.org
Wed Jul 29 12:15:36 PDT 2015


colinl added a comment.

Merged MachO DumpSections in to FilterSections with 243556.  Also simplifying the filtering logic as suggested.  Leaving open though if more changes are wanted.


================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:204
@@ +203,3 @@
+  void ScanPredicate() {
+    while (Iterator != End && Predicate(*Iterator)) {
+      ++Iterator;
----------------
filcab wrote:
> We're *not* keeping the ones where the predicate returns true?
> I'd prefer to call it `FilterOut`, or reverse this condition.
I could see how that could be better, let me take a look at which way would make more sense.

================
Comment at: tools/llvm-objdump/llvm-objdump.h:28
@@ -27,3 +27,3 @@
 extern cl::list<std::string> MAttrs;
-extern cl::list<std::string> DumpSections;
-extern cl::opt<bool> Disassemble;
+extern cl::list<std::string> Sections;
+extern cl::opt<bool> Disassemble;
----------------
filcab wrote:
> Please split this in two commits: Renaming (and moving between .cpp files) `DumpSections` to `Sections`, and the the functional change.
Sorry I didn't address this before submitting, that was unintentional I have to get better at phabricator.

================
Comment at: tools/llvm-objdump/llvm-objdump.h:29
@@ -30,1 +28,3 @@
+extern cl::list<std::string> Sections;
+extern cl::opt<bool> Disassemble;
 extern cl::opt<bool> DisassembleAll;
----------------
filcab wrote:
> Are you changing whitespace in line 29? Adding `\r`? (The diff on phabricator looks weird)
> If you're changing WS, please split that to another commit. If you're adding `\r`, please don't. :-)
I think I have SVN configured to make sure it only submits proper LF.  This should just be an artifact of the diff upload.


Repository:
  rL LLVM

http://reviews.llvm.org/D11487







More information about the llvm-commits mailing list