[lld] r247014 - Revert "[elf2] Add 32S and 64 relocations (needed for musl)."

[Sean Silva via llvm-commits]

On Tue, Sep 8, 2015 at 12:00 PM, Rafael Espíndola <
llvm-commits at lists.llvm.org> wrote:

> On 8 September 2015 at 14:50, Michael Spencer <bigcheesegs at gmail.com>
> wrote:
> > On Tue, Sep 8, 2015 at 6:52 AM, Rafael Espindola via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >> Author: rafael
> >> Date: Tue Sep  8 08:52:31 2015
> >> New Revision: 247014
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=247014&view=rev
> >> Log:
> >> Revert "[elf2] Add 32S and 64 relocations (needed for musl)."
> >>
> >> This reverts commit r246902. It had uncessary use of yaml.
> >
> > This is not a valid reason to revert someone's commit. This passes the
> > tests, and does not block forward progress of other developers. The
> > correct way to handle this is to send an email and request the changes
> > you would like.
>
> Not in this case. Many times before the feedback was for not using
> YAML. There is nothing new about it in this patch.
>

But the reason has always been verbosity. Here, we can test every new
relocation by just adding 3 lines:

      - Offset:          0x000000000000xxxx
        Symbol:          _start
        Type:            R_xxxxxxxxxxxxx

And update CHECK lines.

That is very simple, and just as short as any llvm-mc based test. Best of
all, it actually has a pattern that is very easy to follow; the equivalent
assembly is non-obvious for many (most?) relocations.

-- Sean Silva


> Also, "I don't know it" is *never* a good reason to implement a patch
> one way or the other.
>
> And last but not least, in the past you have taken more than a week to
> reply to post commit reviews as simple as "reuse a variable".
>
> Cheers,
> Rafael
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150908/92ebda97/attachment.html>