[PATCH] D56873: [llvm-objcopy] [COFF] Implement --only-section

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo marked 3 inline comments as done.
mstorsjo added inline comments.


================
Comment at: test/tools/llvm-objcopy/COFF/only-section.test:12
+
+SECTIONS: Sections:
+SECTIONS-NEXT: Idx Name
----------------
rupprecht wrote:
> nit: these tests will be a little easier to read if they are indented something like this:
> 
> ```
> SECTIONS:           xSections:
> SECTIONS-NEXT:      xIdx Name
> SECTIONS-DEBUG-NEXT:x  .debug_discardable
> SECTIONS-TEXT-NEXT: x  .text
> SECTIONS-NEXT-EMPTY:x
> ```
> (x's for clarity)
Ok, I can align them.


================
Comment at: test/tools/llvm-objcopy/COFF/only-section.test:21
+SYMBOLS-TEXT-NEXT: main
+SYMBOLS-EMPTY:
----------------
rupprecht wrote:
> I'm not too familiar with using `-EMPTY` in filecheck, but is this supposed to be `SYMBOLS-NEXT-EMPTY`?
`-EMPTY` essentially implies `-NEXT`, it requires the next line to either be empty, or there to be EOF here. I haven't intended to use them combined but apparently I did do that in the paragraph above this. I'll change that one for consistency with the rest (that's the only one I have with `NEXT-EMPTY` at all).


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

https://reviews.llvm.org/D56873