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

Eric Christopher echristo at gmail.com
Mon Aug 12 17:41:24 PDT 2013


On Mon, Aug 12, 2013 at 5:32 PM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
> I guess I answer your question in the mail I replied to David:-)
>
> As of today,  the LTOCodeGenerator is consistently "return-true-on-succ".  I
> updated the comments to reflect the change.
> (The comment was actually added by me).  This class has no document at all,
> only comments.  Part of the reason I guess
>  is that it is "internal" class. It is called by lto_xxx APIs.  I didn't
> change the semantics of these APIs. I definitely cannot
> change them -- otherwise, we are not able to compile with Apple-ld.
>
> I believe this class has no ambiguity. Of course it is my biased opinion. If
> you have specific advice as to how to improve the
> interface. I'm open to make change. It should not be a big deal I guess.
>

Aww. Missed that. Guess the functions could all use some documentation
in a block comment? :)

-eric

>
> On 8/12/13 5:04 PM, Eric Christopher wrote:
>>
>> Ping?
>>
>> -eric
>>
>> On Thu, Aug 8, 2013 at 1:07 PM, Eric Christopher <echristo at gmail.com>
>> wrote:
>>>>
>>>> As to consistency. Yes, consistency is important. That is why I need to
>>>> change
>>>> the interface.  On the other hand, maybe I were not clear before. What I
>>>> commit
>>>> in this revision only change the private functions. I'm working on the
>>>> two
>>>> public functions
>>>> as well.
>>>>
>>>> As to documentation, in yesterday's commit. I clearly high-light the
>>>> return-on-false
>>>>
>>> My complaint here is that your patch changed the return of numerous
>>> functions without seemingly needing a documentation change. My request
>>> was that either a) you should have changed the documentation, or b) if
>>> it doesn't exist add some documentation to all of the functions you
>>> just changed stating what the expected return is.
>>>
>>> Thanks.
>>>
>>> -eric
>
>



More information about the llvm-commits mailing list