[PATCH] D78342: [lld] Add archive file support to Mach-O backend

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/InputFiles.cpp:248-255
+std::vector<InputSection *> const ArchiveFile::getInputSections() {
+  std::vector<InputSection *> sections;
+  for (InputFile *input : resolvedInputs) {
+    auto inputSections = input->getInputSections();
+    sections.insert(sections.end(), inputSections.begin(), inputSections.end());
+  }
+  return sections;
----------------
smeenai wrote:
> Ktwu wrote:
> > int3 wrote:
> > > Kind of unfortunate that we have to return by value here, but I see that lld-ELF's `getInputSections` does something similar.
> > > 
> > > Also, did you mean to type this method as `std::vector<InputSection *> ArchiveFile::getInputSections() const {`? I *think* the current placement of the `const` is saying that the return value is `const` instead of saying the method is const...
> > re: const, yup, thanks!
> The ELF equivalent is `InputFiles::getSections`, and it returns an `ArrayRef`.
> 
> Returning by value wouldn't be an issue here – we're returning a local object, so return value optimization ([NRVO](https://en.cppreference.com/w/cpp/language/copy_elision)) kicks in. It's not ideal for `ObjFile::getInputSections` though, as I commented above.
Ah, sorry, I was looking at `getInputSections()` in OutputSections.cpp, but I see it's doing something different. And good point about the NRVO, forgot about that.

Maybe we could make this a `forEachSection(callback)` instead, though I do like the `objectFiles` idea too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78342