[PATCH] D92360: [lld/mac] Fix issues around thin archives

[Jez Ng via Phabricator via llvm-commits]

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

lgtm



================
Comment at: lld/MachO/Driver.cpp:306
   for (StringRef path : args::getLines(mbref))
-    addFile(path);
-}
-
-static void forceLoadArchive(StringRef path) {
-  if (Optional<MemoryBufferRef> buffer = readFile(path))
-    for (MemoryBufferRef member : getArchiveMembers(*buffer))
-      inputFiles.push_back(make<ObjFile>(member));
+    addFile(path, false);
 }
----------------
thakis wrote:
> int3 wrote:
> > int3 wrote:
> > > same for the rest of the calls
> > actually, I guess this would make for rather verbose code. How about making the parameter default to `false` instead?
> I think lld code likes to be explicit about arguments -- cf LinkerDriver::enqueuePath() in lld/COFF/Driver.cpp which looks very similar to this.
> 
> If you hate the bool I can invent a two-state enum for this so that it's addFile(path, kForceLoadArchive), addFile(path, kRegularFile). But given that there are just a few callers, all in one file, and COFF doesn't do it, I'm not sure it's worth it?
hm fine I guess this is okay as-is


================
Comment at: lld/MachO/Symbols.cpp:22
+  return std::string(symName);
+}
+std::string lld::toString(const Symbol &sym) {
----------------
thakis wrote:
> int3 wrote:
> > nit: can we have a blank line between functions? my vim habits depend on it :p
> Done. Off-topic: Do you use `{` / `}` or `[]` / `][`, `[[`, `]]`? I like the spirit behind the latter but they don't seem to work well for me for C++ code. And `{` / `}` movements are often to granular, so I usually navigate with ctrl-f / ctrl-b / search / marks.
heh yeah I use a lot of `{` / `}`, regardless of language


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

https://reviews.llvm.org/D92360