[PATCH] D88674: [install-name-tool] Add --delete_all_rpaths to llvm-install-name-tool

Alexander Shaposhnikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 3 14:31:33 PDT 2020


alexshap added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/install-name-tool-delete-rpath.test:48
+# RUN: llvm-install-name-tool --delete_all_rpaths %t2
+# RUN: llvm-objdump -p %t2 | \
+# RUN:   FileCheck %s
----------------
the line break appears to be unnecessary


================
Comment at: llvm/tools/llvm-objcopy/InstallNameToolOpts.td:24
 
+def delete_all_rpaths: Flag<["-", "--"], "delete_all_rpaths">,
+              HelpText<"Delete all rpath directives">;
----------------
it would be good to have at least one test which checks -delete_all_rpaths (with single dash)


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:136
 
-  LoadCommandPred RemovePred = [&RPathsToRemove](const LoadCommand &LC) {
+  LoadCommandPred RemovePred = [&RPathsToRemove, &Config](const LoadCommand &LC) {
     if (LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_RPATH) {
----------------
you can apply clang-format -style=llvm to reformat the code (here and above)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88674



More information about the llvm-commits mailing list