Details
Diff Detail
Event Timeline
test/tools/llvm-objcopy/ELF/strip-unneeded.test | ||
---|---|---|
29 | All the newly added tests produce results identical to some of the existing tests, removing all symbols. Wouldn't it be useful with a test that only removes some of the unneeded symbols but not all of them, to showcase the difference? |
test/tools/llvm-objcopy/ELF/strip-unneeded.test | ||
---|---|---|
29 | --strip-unneeded removes only bar, foobar and foobaz. File and section symbols are not removed, also foo is not removed because it's in comdat. This is checked by original test case (line 6). Additional tests check that
What kind of extra test do you feel is needed? |
test/tools/llvm-objcopy/ELF/strip-unneeded.test | ||
---|---|---|
29 | A test that removes only one of bar/foobar/foobaz but not all of them. |
Thanks, looks good to me now. I'll leave someone else to do the overall approval though.
tools/llvm-objcopy/ELF/ELFObjcopy.cpp | ||
---|---|---|
368–371 | nit: these checks should be inverted, i.e. first check if the command line flag has been set, and then call isUnneededSymbol() to see if it applies. |
All the newly added tests produce results identical to some of the existing tests, removing all symbols. Wouldn't it be useful with a test that only removes some of the unneeded symbols but not all of them, to showcase the difference?