[PATCH] D31011: recommend using llvm-ar when finding undefined references and empty archives

Bob Haarman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 16 16:19:04 PDT 2017


inglorion added inline comments.


================
Comment at: ELF/InputFiles.cpp:559-565
+  bool NoSymbols = true;
+  for (const Archive::Symbol &Sym : File->symbols()) {
     Symtab<ELFT>::X->addLazyArchive(this, Sym);
+    NoSymbols = false;
+  }
+
+  if (NoSymbols) Config->ArchiveWithoutSymbolsSeen = true;
----------------
davide wrote:
> Why do you need two different flags?
By "two different flags" do you mean NoSymbols and ArchiveWithoutSymbolsSeen? I'll get rid of that by following ruiu's suggestion.


================
Comment at: ELF/InputFiles.cpp:565
+
+  if (NoSymbols) Config->ArchiveWithoutSymbolsSeen = true;
 }
----------------
davide wrote:
> ruiu wrote:
> > Format.
> > 
> > Let's eliminate the local varaible. You could replace `NoSymbols` with `File->symbols().empty()`. `File->symbols()` is cheap so you don't need to save time here.
> should live on two different lines, no? (in general, clang-format).
W.r.t. "two different lines": that's what I thought, too, but clang-format actually changed it to be one line. Shall I just go ahead and change it back to two lines?


================
Comment at: ELF/Relocations.cpp:621
+              " This can happen when creating archives with a version"
+              " of ar that does not understand the object files in"
+              " the archive. For example, if you are using LLVM"
----------------
davide wrote:
> I'd say "the bitcode files".
Hmm, yeah. I added the message because I ran into trouble with bitcode files, but it really is a more general situation, which is why I went with "object files" in the message. Is it ok to leave that as-is or shall I reword the message to something like "This can happen when creating archives with bitcode files (such as created by -flto) using a version of ar that does not know how to index those"?


https://reviews.llvm.org/D31011





More information about the llvm-commits mailing list