[PATCH] D24060: [lld][ELF] Add support for -b binary

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/Driver.cpp:500
@@ +499,3 @@
+    case OPT_script:
+      addFile(Arg->getValue(), true);
+      break;
----------------
I'd directly call `readLinkerScript` if OPT_script. It should allows you to remove the second parameter from `addFile`.

================
Comment at: ELF/Driver.cpp:506
@@ +505,3 @@
+    case OPT_format: {
+      StringRef Val = Arg->getValue();
+      if (Val == "elf" || Val == "default") {
----------------
We use `S` as a local variable name in other places for a return value of `getValue`.

================
Comment at: ELF/Driver.cpp:515
@@ +514,3 @@
+      }
+      error("unknown " + Arg->getSpelling() + " format: " + Arg->getValue() +
+            " (supported formats: elf, default, binary)");
----------------
I think `getSpelling` always return `-b`, so I'd just print out `"unknown -b format: "`.

================
Comment at: ELF/InputFiles.cpp:714
@@ +713,3 @@
+std::unique_ptr<InputFile> BinaryFile::createELF() {
+  SimpleELFWriter<ELFT> ELF(ET_REL, Config->EMachine);
+  auto DataSec = ELF.addSection(".data");
----------------
Can you move all these details into SimpleELFWriter so that we can just do something like

  std::vector<uint8_t> Buf = createELFFromBinary(MB);
  return createELFFile<ObjectFile>(MemoryBufferRef(Buf, MB.getBufferIdentifier()));


================
Comment at: ELF/SimpleELFWriter.h:1
@@ +1,2 @@
+//===- SimpleELFWriter.h ----------------------------------------*- C++ -*-===//
+//
----------------
`Writer` is I think a confusing name because it sounds like an alternative Writer. It is actually rather a part of readers. I'd probably name this BinaryFile.h.


https://reviews.llvm.org/D24060