[llvm-commits] [PATCH] [lld] patch for ELF Writer to get it to 'Hello world'

[Shankar Easwaran]

Hi Sean,

Yes, I understand your point from the LLVM development style.

Let me split the patch into 5 parts

1) Changes to DefinedAtom
2) ReaderELF changes
3) WriterELF changes
4) Relocation Changes
5) Symbol Resolution changes

AFAIK, I wont be able to split up the WriterELF patch, because all the 
changes in WriterELF are very tightly connected, ESP, ELFLayout, 
Section, Segment.

I will incorporate Michael/your comments and send it over for review one 
at  a time.

Thanks

Shankar Easwaran

On 12/17/2012 10:24 PM, Sean Silva wrote:
>>    I didnt want to make the patch more bigger by adding / changing existing
>>    test cases.
> You have this backward: a patch introducing this much new
> functionality means that it *must* have a huge battery of tests (but
> it shouldn't be this big in the first place).
>
>>    This whole patch is needed to get 'Hello world' to work. I can split it up,
>>    but will not get to the complete functionality by adding one at a time.
> Just because a particular goal is tantalizingly close doesn't mean to
> abandon proper incremental development and testing as is expected by
> the LLVM community. A more incremental approach is easier to review
> and would help to develop proper test cases.
>
> This patch violates two core tenets of LLVM development style:
> incremental development and strong testing. Please refresh your memory
> about <http://llvm.org/docs/DeveloperPolicy.html#incremental-development>
> and <http://llvm.org/docs/DeveloperPolicy.html#test-cases>.
>
> -- Sean Silva
>
> On Mon, Dec 17, 2012 at 7:59 PM, Shankar Kalpathi Easwaran
> <shankarke at gmail.com> wrote:
>>    Hi Michael, Sean
>>
>>    I am fixing the test cases, and I am planning to add a few to the list of
>>    testcases.
>>
>>    I didnt want to make the patch more bigger by adding / changing existing
>>    test cases.
>>
>>    Will do the changes after the review.
>>
>>    Sean,
>>
>>    This whole patch is needed to get 'Hello world' to work. I can split it up,
>>    but will not get to the complete functionality by adding one at a time.
>>
>>    Thanks
>>
>>    Shankar Easwaran
>>
>> http://llvm-reviews.chandlerc.com/D222
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation