[PATCH] D13055: [ELF2] Handle -m option

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/Driver.cpp:96
@@ +95,3 @@
+template <template <class> class T>
+std::unique_ptr<ELFFileBase> LinkerDriver::createELFFile(MemoryBufferRef MB) {
+  std::unique_ptr<ELFFileBase> File = ::createELFFile<T>(MB);
----------------
I'd avoid giving the same name even if they live in different namespaces. Name this createELFInputFile.

================
Comment at: ELF/Driver.cpp:107
@@ +106,3 @@
+  }
+  if (ElfKind != Config->ElfKind || EMachine != Config->EMachine) {
+    const ELFFileBase *First = Symtab.getFirstELF();
----------------
Flip this condition to return early.

================
Comment at: ELF/Driver.cpp:108-110
@@ +107,5 @@
+  if (ElfKind != Config->ElfKind || EMachine != Config->EMachine) {
+    const ELFFileBase *First = Symtab.getFirstELF();
+    error(Twine(MB.getBufferIdentifier() + " is incompatible with " +
+                (First ? First->getName() : "target architecture")));
+  }
----------------
Separate two conditions

  if (ELFFileBase *F = Symtab.getFirstELF())
    error(MB.getBufferIdentifier() + " is not compatible with" + F->getName());
  error(MB.getBufferIdentifier() + " is not compatible with target architecture");


================
Comment at: ELF/Driver.cpp:109-110
@@ +108,4 @@
+    const ELFFileBase *First = Symtab.getFirstELF();
+    error(Twine(MB.getBufferIdentifier() + " is incompatible with " +
+                (First ? First->getName() : "target architecture")));
+  }
----------------
Can you remove Twine()? The result type of the expression inside Twine() is std::string, and because std::string -> Twine implicit conversion is defined, I believe you don't need that.


http://reviews.llvm.org/D13055