llvm dev - Sep 2013 - [LLVMdev] LLD: Returning true on success

Hi LLD developers,

I'm about to make a change to invert the return value of Driver::parse() to
return true on success. Currently it returns false on success.

In many other functions, we return true to indicate success and false to
indicate failure. The inconsistency is confusing, and fixing it should
improve code readability.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130924/12d1cdec/attachment.html>

On Tue, Sep 24, 2013 at 12:33 PM, Rui Ueyama <ruiu at google.com> wrote:
> Hi LLD developers,
>
> I'm about to make a change to invert the return value of
Driver::parse()
> to return true on success. Currently it returns false on success.
>
> In many other functions, we return true to indicate success and false to
> indicate failure. The inconsistency is confusing, and fixing it should
> improve code readability.
>
>
Note that some places in LLVM use false to indicate success, not sure how
widespread this is. Personally I think that { if (doSomething()) } means if
doSomething succeeded, and thus agree with you. However, I think this is
something that needs to be consistent across all of LLVM and should be in
the coding standard.

- Michael Spencer
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130924/c0a994ef/attachment.html>

On Sep 24, 2013, at 12:40 PM, Michael Spencer <bigcheesegs at gmail.com>
wrote:
> On Tue, Sep 24, 2013 at 12:33 PM, Rui Ueyama <ruiu at google.com>
wrote:
> Hi LLD developers,
> 
> I'm about to make a change to invert the return value of
Driver::parse() to return true on success. Currently it returns false on
success.
> 
> In many other functions, we return true to indicate success and false to
indicate failure. The inconsistency is confusing, and fixing it should improve
code readability.
> 
> 
> Note that some places in LLVM use false to indicate success, not sure how
widespread this is. Personally I think that { if (doSomething()) } means if
doSomething succeeded, and thus agree with you. However, I think this is
something that needs to be consistent across all of LLVM and should be in the
coding standard.
StringRef::getAsInteger() is an example documented to return true if there was
an error parsing the string.

Mapping success/error onto a bool will always be ambiguous.  Is there some
better pattern?

-Nick


-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130924/7197328f/attachment.html>

On Tue, Sep 24, 2013 at 2:40 PM, Michael Spencer <bigcheesegs at
gmail.com>wrote:
> On Tue, Sep 24, 2013 at 12:33 PM, Rui Ueyama <ruiu at google.com>
wrote:
>
>> Hi LLD developers,
>>
>> I'm about to make a change to invert the return value of
Driver::parse()
>> to return true on success. Currently it returns false on success.
>>
>> In many other functions, we return true to indicate success and false
to
>> indicate failure. The inconsistency is confusing, and fixing it should
>> improve code readability.
>>
>>
> Note that some places in LLVM use false to indicate success, not sure how
> widespread this is. Personally I think that { if (doSomething()) } means if
> doSomething succeeded, and thus agree with you. However, I think this is
> something that needs to be consistent across all of LLVM and should be in
> the coding standard.
>
Yes, this comes up again and again.

Chris has argued for true-on-error before as it fits with the classical
pattern of integer or enum error codes, but recently has indicated a
willingness to shift this stance.

What makes sense to me is to give 3 possible choices:

1) bool return, true indicates success.

2) an enum with a success enumerator which == 0, and non-success error
codes != 0.

3) an error class with an operator bool where true indicates the class does
contain an error.


It is only #3 and #1 that then are in any real contradiction at that point,
and it seems easy to understand and explain due to the object being an
error object.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130924/83d1de07/attachment.html>

On 9/24/2013 2:33 PM, Rui Ueyama wrote:
> Hi LLD developers,
>
> I'm about to make a change to invert the return value of 
> Driver::parse() to return true on success. Currently it returns false 
> on success.
You have to be a bit careful here, as the return from parse is used as a 
exit code from lld.

The Unix shell interprets 0 (false as success) as 1(true as failure).

It has to be consistent throughout for this reason.

As Bigcheese mentioned,  if its in the Coding standard this would really 
help.

Thanks

Shankar Easwaran
>
> In many other functions, we return true to indicate success and false 
> to indicate failure. The inconsistency is confusing, and fixing it 
> should improve code readability.
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the
Linux Foundation

It looks like although it's not mentioned in the coding standard, nobody is
opposing to invert it for this case, so I'll do it. I agree with Michael.
Returning true on success seems natural to me. However, I believe the most
important thing is to maintain consistency of boolean return value at
least within LLD whether it should be true or false.


On Tue, Sep 24, 2013 at 1:51 PM, Shankar Easwaran
<shankare at codeaurora.org>wrote:
> On 9/24/2013 2:33 PM, Rui Ueyama wrote:
>
>> Hi LLD developers,
>>
>> I'm about to make a change to invert the return value of
Driver::parse()
>> to return true on success. Currently it returns false on success.
>>
> You have to be a bit careful here, as the return from parse is used as a
> exit code from lld.
>
> The Unix shell interprets 0 (false as success) as 1(true as failure).
>
> It has to be consistent throughout for this reason.
>
> As Bigcheese mentioned,  if its in the Coding standard this would really
> help.
>
> Thanks
>
> Shankar Easwaran
>
>
>> In many other functions, we return true to indicate success and false
to
>> indicate failure. The inconsistency is confusing, and fixing it should
>> improve code readability.
>>
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
> by the Linux Foundation
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130924/e8df7fa2/attachment.html>