[cfe-dev] Should functions returning bool return true or false on success?

[Zachary Turner via cfe-dev]

Error is nice when there is more than one possible way in which something
can fail, but it's overkill when all you care about is yes or no, and it
also forces you to check the value, which hinders readability in some
situations.

We can use Optional<T> to indicate success/failure if the return type is
non-void, but when it's void the only options are bool or Error, and Error
is not always ideal for the aforementioned reason.

Furthermore, the code Nico linked to is in LLVMDemangle, which does not
have a dependency on libSupport, so it can't use Error or Optional anyway.

On Mon, Sep 17, 2018 at 11:18 AM Zachary Turner <zturner at google.com> wrote:

> I was one of the people that commented on that review, so you already know
> my opinion, but sicne it hasn't been stated in any kind of official thread
> and some people might not click the link I will restate it here.
>
> I feel very strongly that true means success.  I think *every* existing
> occurrence of true on error should be fixed (not necessarily immediately or
> with high priority).  I would push back on any attempt to add a new
> function that returns true on failure.
>
> implicit integer-to-boolean conversions are bad, and so despite the fact
> that people often write stuff like if (!strcmp(x, y)) we should not be
> basing decisions about good usage on the fact that bad usage is common.
>
> Functions that return bools are frequently named with verbs.  It's beyond
> confusing to have code such as:
>
> if (withdrawMoney(Account, Amount))
>   reportAnError();
>
> Types like llvm::Error are an exception and while it can still be
> confusing, some of this confusion is alleviated by the fact that you have
> to actually assign the return value to something or else your program will
> fail with an assertion.  So, for example, if the above function returned an
> llvm::Error, you'd assert that the error was unhandled, and you'd have to
> re-write it:
>
> if (auto Err = withdrawMoney(Account, Amount))
>   report(Err);
>
>
> On Mon, Sep 17, 2018 at 10:57 AM Nico Weber <thakis at chromium.org> wrote:
>
>> Hi,
>>
>> in https://reviews.llvm.org/D52143 there's some uncertainty if LLVM code
>> prefers
>>
>>   if (!Function())
>>     // Call to function failed, deal with it
>>
>> or
>>
>>   if (Function())
>>     // Call to function failed, deal with it
>>
>> (Note that this is about functions returning bool, not int.)
>>
>> Folks on that review feel that returning true on success is probably what
>> we want, but it's not documented anywhere and we do have both forms in the
>> codebase.
>>
>> True on success seems more common:
>> http://llvm-cs.pcc.me.uk/?q=true+on+success
>> http://llvm-cs.pcc.me.uk/?q=true+on+error
>>
>> Does anyone have a pointer to previous on-list discussion on this? If
>> not, this thread could be the place where we sort this out once and for all
>> :-)
>>
>> Apologies for the bike-sheddy topic.
>>
>> Nico
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180917/8565af7d/attachment.html>