[PATCH] D13286: [ELF2] Add --[no-]whole-archive command line switches

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/InputFiles.cpp:193-194
@@ -192,6 +192,4 @@
 
 void ArchiveFile::parse() {
-  ErrorOr<std::unique_ptr<Archive>> ArchiveOrErr = Archive::create(MB);
-  error(ArchiveOrErr, "Failed to parse archive");
-  File = std::move(*ArchiveOrErr);
+  openArchive();
 
----------------
The point is that we never want to call both parse() and getMembers() on the same ArchiveFile object. If you call getMembers() on an ArchiveFile, that's the end of that archive file, and we won't call any other member function after that.

Restore the original code here so that it sets this class' File pointer only when parse() is called. We don't have to set that pointer in getMembers().

================
Comment at: ELF/InputFiles.cpp:194
@@ -193,5 +193,3 @@
 void ArchiveFile::parse() {
-  ErrorOr<std::unique_ptr<Archive>> ArchiveOrErr = Archive::create(MB);
-  error(ArchiveOrErr, "Failed to parse archive");
-  File = std::move(*ArchiveOrErr);
+  openArchive();
 
----------------
I'd be easy to read if you write this way:

  File = open(MB);

assuming you modify the function as

  std::unique_ptr<Archive> open(MemoryBufferRef MB);

================
Comment at: ELF/InputFiles.cpp:236-238
@@ +235,5 @@
+void ArchiveFile::openArchive() {
+  if (File)
+    return;
+
+  ErrorOr<std::unique_ptr<Archive>> ArchiveOrErr = Archive::create(MB);
----------------
This guard should be removed. You can call either getMembers() or pares() but not both.

================
Comment at: ELF/SymbolTable.cpp:33
@@ -34,1 +32,3 @@
+    File.release();
     ArchiveFiles.emplace_back(AF);
+    if (Config->WholeArchive) {
----------------
Do not push archive file objects to the vector if WholeArchive is true. If --whole-archive is set, archive files are treated as if they did not exist.

================
Comment at: ELF/SymbolTable.cpp:35-36
@@ +34,4 @@
+    if (Config->WholeArchive) {
+      for (MemoryBufferRef &MBRef : AF->getMembers())
+        addFile(createELFFile<ObjectFile>(MBRef));
+    } else {
----------------
Early return after this for-loop.


http://reviews.llvm.org/D13286