[PATCH] D126509: [Objcopy][Wasm] Allow selecting known sections by name

[Derek Schuff via Phabricator via llvm-commits]

dschuff added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/wasm/only-keep-debug.test:17
 # CHECK:      Sections:
+# KEEPTYPE:       Type: TYPE
 # CHECK:        - Type: CUSTOM
----------------
aheejin wrote:
> dschuff wrote:
> > aheejin wrote:
> > > dschuff wrote:
> > > > aheejin wrote:
> > > > > aheejin wrote:
> > > > > > 
> > > > > Not something important, but gentle ping?
> > > > oh, oops, sorry I missed that. Will fix.
> > > ```
> > > # CHECK-NOT:    - Type: TYPE
> > > ```
> > > is still missing. Not important though.
> > oops, I completely misread your comment, sorry about that 😓
> > Yes, I think having that check is a good idea.
> > It's a little weird because the FileCheck line that includes KEEPTYPE also includes CHECK as a prefix. So in that case KEEPTYPE line requires that line, and then the CHECK-NOT isn't needed. but it does still work.
> > Maybe even better would be to add `--implicit-check-not=TYPE` to ensure that the type section isn't anywhere in the file. Although that would be overkill since the line where this CHECK is, is the only place it could appear.
> > I'll just add the CHECK-NOT where you suggested.
> Oh, I didn't notice `KEEPTYPE` has also `CHECK` in the command. Then does my `CHECK-NOT` line work? I guess not but you said it works... Anyway nevermind if it doesn't work.
It does work; I think it just checks that there isn't another instance of TYPE after it matches the one that's there. So that's not really necessary (in the sense that having the type section be duplicated seems an unlikely failure mode) but doesn't seem harmful either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126509