[PATCH] D131375: [llvm-ranlib] Support more than one input file

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/docs/CommandGuide/llvm-ranlib.rst:15
 :program:`llvm-ranlib` is an alias for the :doc:`llvm-ar <llvm-ar>` tool that
-generates an index for an archive. It can be used as a replacement for GNU's
+generates an index for one or more archive. It can be used as a replacement for GNU's
 :program:`ranlib` tool.
----------------



================
Comment at: llvm/test/tools/llvm-ranlib/error-opening-permission.test:17
+# RUN: cmp a.a b.a
+## The others (c.a and d.a) do not a symbol table.
+# RUN: chmod 700 c.a
----------------



================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:1434
   }
-  if (!ArchiveSpecified) {
+  for (int i = 1; i < argc; ++i) {
+    StringRef arg(argv[i]);
----------------
Nit: I'd probably add a blank line between the two for loops.


================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:1435-1438
+    StringRef arg(argv[i]);
+    if (arg.startswith("-"))
+      continue;
+    ArchiveName = arg.str();
----------------
It seems to me like you should generate the list of input files during the first loop, and then iterate over that rather than iterating over the full set of argv options twice? This will remove the risk of issues where the `startswith("-")`/`consume_front("-")` logic changes for some reason, which could result in the two getting out-of-sync.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131375