[PATCH] D73820: [llvm-strip][WebAssembly] Support strip flags

Sam Clegg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 2 12:04:57 PST 2020


sbc100 added a comment.

Mostly lgtm with some comments.



================
Comment at: llvm/test/tools/llvm-objcopy/wasm/basic-keep.test:7
+# CHECK: Sections:
+# CHECK-NEXT: - Type: TYPE
+# CHECK:      - Type: CUSTOM
----------------
So strip-all doesn't apply to the TYPE section?


================
Comment at: llvm/test/tools/llvm-objcopy/wasm/basic-keep.test:11
+# CHECK-NEXT:   Payload: DEADBEEF
+# CHECK-NOT: Section
+
----------------
What is this NOT checking for?


================
Comment at: llvm/test/tools/llvm-objcopy/wasm/basic-only-section.test:11
+# CHECK-NEXT:   Tools:
+# CHECK-NOT: Section
+
----------------
ditto above, what is this checking for?


================
Comment at: llvm/test/tools/llvm-objcopy/wasm/strip-debug.test:1
+## Test that debug sections are stripped with --strip-debug
+# RUN: yaml2obj %s -o %t
----------------
Can you do a followup that renames all the `.test` files in this directory to `.yaml` .. its more descriptive and enables syntax highlighting .


================
Comment at: llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp:97
+      // Otherwise defer to RemovePred.
+      return RemovePred(Sec);
+    };
----------------
Interesting technique of chaining the predicates together like this using lamda.    Might be a little too clever?   Do other object backends use this too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73820





More information about the llvm-commits mailing list