[PATCH] D57842: [llvm-ar][libObject] Fix relative paths when nesting thin archives.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:238
+  if (Thin)
+    Out << computeRelativePath(ArcName, M.MemberName);
+  else
----------------
rupprecht wrote:
> rupprecht wrote:
> > ruiu wrote:
> > > So, M.MemberName is relative to ArcName if it is thin, and it is used as-is if not thin. It doesn't look like we really have to distinguish two here. Could you call `computeRelativePath` early in llvm-ar and set its result to MemberName? Then I think you can remove special handling of thin archive here.
> > Hmm, good point, that would be a lot simpler. I'll try it.
> That seems to have worked, thanks for the suggestion!
This function is now too trivial to be a function. I'd remove this function by replacing a call site with `OS << M.MemberName << "/\n";`.


================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:618
+      Thin ? Saver.save(computeRelativePath(ArchiveName, FileName))
+           : NMOrErr->MemberName = sys::path::filename(NMOrErr->MemberName);
   if (FlattenArchive &&
----------------
Remove the redundant assignment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57842