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

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 8 13:27:33 PDT 2016


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?

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/a2a7b14c/attachment.html>


More information about the llvm-commits mailing list