[PATCH] D119074: [ELF] Parse archives as --start-lib object files

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I've had a chance to look through this. Assuming the user-visible behaviour changes are limited to what has been mentioned in the description I'm in favour of the change. I've only made one suggestion surrounding a comment. To check my understanding all files in archives are handled with createLazyFile which would make their symbols `LazyObject` ? If I'm wrong then I'll need to re-read the code to make sure I understand first.



================
Comment at: lld/ELF/Driver.cpp:233
+    archiveFiles.emplace_back(path, members.size());
 
     // All files within the archive get the same group ID to allow mutual
----------------
Is it worth a design decision comment here? I'm thinking of people new to LLD wondering where the traditional archive handling code is.

For example
```LLD chooses to handle archives and --start-lib and --end-lib using the same code-path. This scans all the ELF files in the archive rather than just the index file, with the benefit that the symbols are only loaded once. For many projects archives see high utilization rates so in these cases it is a net performance win.```


================
Comment at: lld/ELF/Symbols.h:412
+// LazyObject symbols represents symbols in object files between
+// --start-lib and --end-lib options.
 //
----------------
As I understand it, traditional archives are effectively being handled as if they've been expanded by `--start-lib` and `--end-lib` is it worth mentioning this here.
```
// LazyObject symbols represents symbols in object files between --start-lib and --end-lib options. LLD also handles traditional archives as if all the files in the archive where extracted while being surrounded by --start-lib and --end-lib.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119074