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

Michael Spencer via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 8 13:16:37 PDT 2016


On Thu, Sep 8, 2016 at 9:19 AM, Rui Ueyama <ruiu at google.com> wrote:

> ruiu added inline comments.
>
> ================
> Comment at: ELF/CMakeLists.txt:20
> @@ -19,2 +19,3 @@
>    ScriptParser.cpp
> +  SimpleELFWriter.cpp
>    Strings.cpp
> ----------------
> I still don't like this file name. `SimpleELFWriter` sounds like it is for
> -o binary instead of -b. Can you rename BinaryFile.cpp?
>

I'm fine with finding another name, but BinaryFile isn't at all what this
class is.

- Michael Spencer


>
> ================
> Comment at: ELF/InputFiles.cpp:736
> @@ +735,3 @@
> +template <class ELFT> std::unique_ptr<InputFile> BinaryFile::createELF() {
> +  SimpleELFWriter<ELFT> ELF(ET_REL, Config->EMachine);
> +  auto DataSec = ELF.addSection(".data");
> ----------------
> Please move these details to SimpleELFWriter.cpp.
>
>
> https://reviews.llvm.org/D24060
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160908/eea94ace/attachment.html>


More information about the llvm-commits mailing list