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

[Michael Spencer via llvm-commits]

On Thu, Sep 8, 2016 at 1:27 PM, Rui Ueyama <ruiu at google.com> wrote:

> Doesn't BinaryFile make sense? We have InputFiles to handle input files,
> so BinaryFile sounds like it is to handle binary file inputs. Do you have
> other suggestions?
>

SimpleELFWriter isn't an input. It's a class that generates ELF files.

- Michael Spencer


>
> On Thu, Sep 8, 2016 at 1:16 PM, Michael Spencer <bigcheesegs at gmail.com>
> wrote:
>
>> 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/7e007415/attachment.html>