[PATCH] D76562: [llvm-objcopy] Support llvm-strip-11 as argv[0]

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 3 inline comments as done.
MaskRay added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:333
+  auto Is = [&](StringRef Tool) {
+    auto I = Stem.rfind_lower(Tool);
+    return I != StringRef::npos &&
----------------
abrachet wrote:
> Might as well just explicitly use `size_t`
The return type of `rfind_lower` probably should be `std::string::size_type`. Using `auto` will be width agnostic. (Although it is unlikely that size_t != std:string::size_type)


================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:335
+    return I != StringRef::npos &&
+           (I + Tool.size() == Stem.size() || !isAlnum(Stem[I + Tool.size()]));
+  };
----------------
alexshap wrote:
> What would you think about smth like this:
> 
>   const llvm::Regex StripRegex("^(llvm-)?strip(.*)$"); 
>   const llvm::Regex InstallNameToolRegex("^(llvm-)?install-name-tool(.*)$");
>   ...
>   if (StripRegex.match(Tool))
>     ...
>   else if (InstallNameToolRegex.match(Tool))
>     ...
> 
The current code is to make it similar to D71302 llvm-ar

We need to support something like `x86_64-unknown-linux-gnu-strip`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76562