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

[Shuxin Yang]

On 8/6/13 4:10 PM, David Blaikie wrote:
> On Tue, Aug 6, 2013 at 3:55 PM, Eric Christopher <echristo at gmail.com> wrote:
>> 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.
> To expound on this a little further (in agreement with Eric): There's
> no current predominant style & both true and false successes are used
> throughout the codebase. Switching them back & forth isn't a great
> idea in the absence of a style notation so that newcomers to the
> codebase know which style to follow, which ones to leave alone, and
> which ones to cleanup.
>
> The style guide also says to be consistent (pretty sure) - so without
> documentation either way, I'd say the style guide says to do what the
> existing code does, rather than to change it. It's not hard to add
> some wording in there to codify what you see as universal agreement
> (I'm not sure it's that, but at least people can discuss that in a
> style guide change).
>
This class has only two public functions with boolean return value, it 
is not huge deal
to change it back to return-on-true.  Otherwise, my added code, which 
has lots of
boolean functions, have to be return-on-false in order to make it 
consistent with
the existing code, as far as I can tell from community's feedback, I 
don't think this
is better decision.

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

83   // Compile the merged module into a *single* object file; the path 
to object
  84   // file is returned to the caller via argument "name". Return 
*FALSE* on
  85   // *SUCCESS*, true otherwise.
  86   //
  87   // NOTE that it is up to the linker to remove the intermediate 
object file.
  88   //  Do not try to remove the object file in LTOCodeGenerator's 
destructor
  89   //  as we don't who (LTOCodeGenerator or the obj file) will last 
longer.
  90   //
  91   bool compile_to_file(const char **name, std::string &errMsg);