[LLVMdev] LLD: Returning true on success

[Sean Silva]

On Tue, Sep 24, 2013 at 4:17 PM, Nick Kledzik <kledzik at apple.com> wrote:

>
> 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.
>

I think it makes a lot of sense in this case. The idea is that you increase
indentation in the "error" case. Consider parsing 2 numbers and returning
their sum:

if (S1.getAsInteger(16, Result1)) {
// report failure
}
if (S2.getAsInteger(16, Result2)) {
// report failure
}
return Result1 + Result2;

vs.

if (S1.getAsIntegerTrueOnSuccess(16, Result1)) {
  if (S2.getAsIntegerTrueOnSuccess(16, Result2)) {
    return Result1 + Result2;
  } else {
  // report failure
  }
} else {
  // report failure
}

Of course, in the latter case you would just use ! to avoid gaining
indentation, but still, I think that if most uses of an API will require
immediately negating the return value, then maybe the sense should be
inverted.


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

Maybe something like:

enum {
  Success,
  Failure
};
or
enum {
  Failure,
  Success
};

-- Sean Silva


>
> -Nick
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130924/f98b5742/attachment.html>