[llvm] r187819 - Change private functions of LTOCodeGenerator from ret-false-on-succ to ret-true-on-succ.

Eric Christopher echristo at gmail.com
Tue Aug 6 15:55:57 PDT 2013


On Tue, Aug 6, 2013 at 3:51 PM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
>
> On 8/6/13 3:40 PM, Eric Christopher wrote:
>>
>> On Tue, Aug 6, 2013 at 3:37 PM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
>>>
>>>
>>>> To whom?
>>>
>>> To everybody who feel more natural with return-true-on-succ.
>>
>> Then why aren't you proposing a standard for the project and changing
>> everything?
>
> As far as I can tell from the community's feedback. Most people take for
> granted
> return-true-on-succ is more natural. Now that it is widely accepted, why
> mess around
> proposing such std?
>

Because it's more clear that way as to what should happen.

> One thing not clear is that how to deal with existing return-false-on-succ
> code.
> My take (as embodied by this change) is to modify the code whenever and
> wherever
> one feel comfortable.  Seems like you are not happy with such approach.  How
> about
> you come up some guidelines and codify it, such that we can follow without
> ambiguity.
>

Because I'm not the one who brought it up in the first place - and as
you obviously don't remember I agreed with you that it was confusing.

>
>>>
>>>> The code that you'd be changing hasn't been committed yet.
>>>>
>>> I don't think it is better to commit confusing integrated code first,
>>> then
>>> get rid of confusing part.
>>
>> This isn't what I'm saying at all. I'm saying that you should change
>> the code that you're writing to match the code that you are
>> integrating with since your code hasn't been approved or committed
>> yet. Then, in the future, if we want to change the meaning of the APIs
>> all of it can change.
>
> As I reiterate in my previous mail, I don't won't to change the API -- Apple
> linker
> directly call these APIs, there is no way to change it; on the other hand,
> changing API is dangerous.

I know exactly what ld64 does here and changing the C API is
unacceptable anyhow unless we version and deprecate. Which probably
makes more sense to do if you have a bunch of changes and extensions
coming.

At any rate the review feedback that I do want you to follow is:

Write documentation for each of these functions that specifies what
the return value is and what it means in each case.
If you care enough to change it then you care enough to document it.

-eric



More information about the llvm-commits mailing list