[PATCH] D11188: [LLD] New ELF implementation

Rafael Ávila de Espíndola rafael.espindola at gmail.com
Wed Jul 22 16:21:48 PDT 2015


rafael added inline comments.

================
Comment at: ELF/CMakeLists.txt:17
@@ +16,3 @@
+  Core
+  LTO
+  MC
----------------
Please delete the LTO dependency. I added it to CMakeList to fix the build, but it should be removed and the code changed to avoid the dependency.

================
Comment at: ELF/Chunks.cpp:79
@@ +78,3 @@
+// Prints "Discarded <symbol>" for all external function symbols.
+template <class ELFT> void SectionChunk<ELFT>::printDiscardedMessage() {
+  auto Obj = File->getObj();
----------------
You don't need this on the first patch.

================
Comment at: ELF/Chunks.cpp:84
@@ +83,3 @@
+template <class ELFT>
+CommonChunk<ELFT>::CommonChunk(const Elf_Sym *S)
+    : Sym(S) {
----------------
This test doesn't need common symbols. Delete this. It should be added in a followup patch. This is also copying over COFF specific code and comments.

================
Comment at: ELF/Chunks.h:90
@@ +89,3 @@
+
+  // Used by the garbage collector.
+  virtual void mark() {}
----------------
Delete the garbage collector. It should be added in another patch and the terminology for ELF is different anyway.

================
Comment at: ELF/DriverUtils.cpp:111
@@ +110,3 @@
+ErrorOr<std::vector<const char *>>
+ArgParser::replaceResponseFiles(std::vector<const char *> Argv) {
+  SmallVector<const char *, 256> Tokens(Argv.data(), Argv.data() + Argv.size());
----------------
Delete this. It should be added when we add a test that uses response files.

================
Comment at: ELF/InputFiles.cpp:46
@@ +45,3 @@
+  auto ArchiveOrErr = Archive::create(MB);
+  if (auto EC = ArchiveOrErr.getError())
+    return EC;
----------------
Handle the error in here instead of returning it.

================
Comment at: test/elf2/basic.test:7
@@ +6,3 @@
+  .globl _start;
+  .type _start, @function;
+_start:
----------------
The type is not needed.


http://reviews.llvm.org/D11188







More information about the llvm-commits mailing list