[PATCH] D83002: [llvm-libtool-darwin] Add support for -static option

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

(No time to look at tests today, sorry)



================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:65
+  Expected<NewArchiveMember> NMOrErr =
+      NewArchiveMember::getFile(FileName, /*deterministic=*/true);
+
----------------
I'd expect `deterministic` to be capitalized in the function signature. If that is the case, this comment should match.


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:79
+  Members.push_back(std::move(*NMOrErr));
+
+  return Error::success();
----------------
You probably don't need this blank line.


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:85
+  std::vector<NewArchiveMember> NewMembers;
+  for (const auto &Member : InputFiles)
+    if (Error E = addMember(NewMembers, Member))
----------------
Let's not use `auto` here, as it's not immediately clear what the type of `Member` is.


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:90-93
+                             /*symtab=*/true,
+                             /*kind=*/object::Archive::K_DARWIN,
+                             /*deterministic=*/true,
+                             /*thin=*/false))
----------------
Same comment as above re. capitalization in the comments.


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:100
   InitLLVM X(Argc, Argv);
+  ToolName = Argv[0];
   cl::ParseCommandLineOptions(Argc, Argv, "llvm-libtool\n");
----------------
What is `ToolName` used for?


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:107
+      WithColor::defaultErrorHandler(std::move(E));
+      return 1;
+    }
----------------
`return EXIT_FAILURE` maybe? What do other tools typically do?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83002