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

[Eric Christopher]

I didn't see if you ever added documentation for the functions you
changed. Did you?

-eric

On Mon, Aug 12, 2013 at 5:41 PM, Eric Christopher <echristo at gmail.com> wrote:
> 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
>>
>>