[PATCH] D80099: [llvm-objcopy][MachO] Add support for removing Swift symbols

Alexander Shaposhnikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 27 13:36:28 PDT 2020


alexshap added a comment.

In D80099#2058039 <https://reviews.llvm.org/D80099#2058039>, @jyknight wrote:

> > The current state doesn't appear to block it or prevent it in any way
>
> It does prevent it. This change introduces an argument, -T to the option syntax of the llvm-strip tool, which we'd be better off without. But, once we add it, users of llvm-strip will depend on it, and then we can't easily remove it later on. The only argument for the name -T is compatibility with cctools strip, which is unachievable within a single driver syntax, anyways.


@jyknight - alright, I wouldn't like to repeat the arguments already presented  and want to put this discussion into a more constructive course to avoid moving back and forth.

"It does not" for the following reasons. First, it's supported only for MachO (and by the way there are options supported only for ELF), so the only way users can depend on it is a legitimate one (the system strip supports this flag and various projects use it). Second, from users' perspective the fact that "strip" is only a driver for another tool (whether it's the existing driver, the existing driver with a new "flavor" or a new driver) is a detail of implementation (and it is hidden from them).  So if e.g. we introduce a new way to invoke llvm-strip (llvm-objcopy) it should not affect them / disrupt their workflow, thus from users' perspective it doesn't fundamentally alter this change. 
Also, please, note that the argument regarding binutils for MachO is invalid (see comments above).

> Now, if it wasn't for all the other incompatibilities with cctools' strip, I could buy the argument that it's worth adding -T, despite that it was a bad choice of name, and despite the potential for future conflicts. But, since compatibility is impossible, we have those downsides, and little upside to go along with it.
> 
>> introducing a new name would break the existing use-cases
> 
> I don't see how, since llvm-strip isn't a drop in replacement for cctools strip anyhow.

Let me remind (and it has already been mentioned above) it's not fully functional yet but it's moving forward, and there are projects which build with it, it's just one of the first steps,
introducing a new name would break it without providing much benefits at all.
Basically llvm-strip would exist on OSX but would be unusable and would make it harder even to test it / plug into a real world project and see what features are missing and what needs to be fixed. 
Based on the experience with ELF I'm pretty sure it's a long path and we will encounter and fix issues along the way, and llvm-objcopy (llvm-strip) for ELF didn't magically become compatible with binutils immediately.

That said, the problem with the discrepancies will have to be addressed at some point, nobody disagrees with it and, in particular, I'm open to discuss the approaches, should it be a new driver, a new "flavor" or something else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80099





More information about the llvm-commits mailing list