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

[Owen Reynolds via Phabricator via llvm-commits]

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


================
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:
> gbreynoo wrote:
> > 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.
> Is there a test case you could add that would have failed with the previous version of this patch?
Added to regular-to-thin-archive.test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123142