[PATCH] D123142: [llvm-ar] Fix thin archive being wrongly converted to a full archive

[Owen Reynolds via Phabricator via llvm-commits]

gbreynoo marked 5 inline comments as done.
gbreynoo added inline comments.


================
Comment at: llvm/test/tools/llvm-ar/thin-to-full-archive.test:28
+# FULL: !<arch>
+# NOT-FULL: thin.a
+# FULL: a.txt
----------------
jhenderson wrote:
> This isn't used anywhere. Did you mean `FULL-NOT`? If so, I'd recommend instead adding `--implicit-check-not=thin.a` to the FileCheck command, since NOT matches are order-sensitive.
Thanks, I've made the change as suggested.


================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:1023-1024
     std::unique_ptr<object::Archive> Archive = std::move(ArchiveOrError.get());
-    if (Archive->isThin())
+    if (Thin && !Archive->isThin())
+      fail("cannot convert a regular archive to a thin one");
+
----------------
jhenderson wrote:
> I've not looked at the full code paths, but at an educated guess, moving this code here will cause this error to occur in other cases which don't add new members, such as `t` operations? Is that desirable (I'm inclined to say so, but at least want to flag up the possibility of an issue)?
Good catch, this has been changed to only check on write commands.


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

https://reviews.llvm.org/D123142