[PATCH] D57517: [llvm-objcopy] Allow using regex in name comparison

Alexander Shaposhnikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 4 00:02:45 PST 2019


alexshap added inline comments.


================
Comment at: tools/llvm-objcopy/CopyConfig.h:53
+template <class NameTy> class NameOrRegex {
+  NameTy Name;
+  // Regex is shared between mumltiple CopyConfig instances.
----------------
evgeny777 wrote:
> alexshap wrote:
> > alexshap wrote:
> > > tbh I have mixed feelings regarding this,
> > > 1. so basically string comparison is just a particular case of regex - maybe we can simply use Regex and don't need this class ?
> > > 2. khm, where is CopyConfig copied ? 
> > although my second thought is that probably it might make sense to have these two cases separate (performance, simplicity of the most popular use case).
> > However, maybe we can get rid of shared_ptr here ? (movable but non-copyable classes work just fine with the standard containers) i.e. use optional or (probably even better) - Regex has a default "empty" state (i.e. default constructed Regex is in this state), maybe we can just add a method "empty" (or safe bool conversion operator) to Regex 
> llvm-strip may use multiple CopyConfig instances which differ only in input/output file names.
> ```
>    for (const char *Filename : Positional) {
>       Config.InputFilename = Filename;
>       Config.OutputFilename = Filename;
>       DC.CopyConfigs.push_back(Config);
>     }
> ```
oh, i see, this is kind of unfortunate, but i see what you are saying.


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

https://reviews.llvm.org/D57517





More information about the llvm-commits mailing list