[PATCH] D57248: [llvm-objcopy] Support -X|--discard-locals.

Wed Jan 30 01:57:21 PST 2019

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

I'm happy with this as is. LGTM.

================
Comment at: llvm/test/tools/llvm-objcopy/ELF/discard-locals.test:60-64
+    - Name:     .L.undefined
+    - Name:     .L.abs
+      Index:    SHN_ABS
+    - Name:     .L.common
+      Index:    SHN_COMMON
----------------
rupprecht wrote:
> jhenderson wrote:
> > These aren't local, so won't be discarded anyway (and therefore aren't testing anything). They should probably be in the local list...
> I don't think a local SHN_COMMON symbol makes any sense, so I just removed that case, but moved the others to local... although I'm not sure the others make sense to be local either.
I guess it depends what you mean by "make sense". At least in assembly, you can generate them (although you can't generate section symbols with names, I believe).

================
Comment at: llvm/tools/llvm-objcopy/StripOpts.td:61
+def discard_locals : Flag<["-", "--"], "discard-locals">,
+                     HelpText<"Remove compiler-generated local symbols, (e.g. "
+                              "symbols starting with .L)">;
----------------
Perhaps this should be "i.e." rather than "e.g."?

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

https://reviews.llvm.org/D57248