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

Zachary Turner via cfe-dev cfe-dev at lists.llvm.org
Mon Sep 17 11:18:26 PDT 2018


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/396a3af6/attachment.html>


More information about the cfe-dev mailing list