[cfe-dev] discriminating explicit boolean expressions from implicit boolean expressions in the AST

[David Blaikie]

On Tue, Mar 31, 2015 at 10:06 AM, Richard <legalize at xmission.com> wrote:

>
> In article <
> CAENS6EsAsF0NYiZkb2x_cbLm2HWH_m73Jd5rkAGGNkZnCLv1tw at mail.gmail.com>,
>     David Blaikie <dblaikie at gmail.com> writes:
>
> > On Wed, Mar 25, 2015 at 3:24 PM, Richard <legalize at xmission.com> wrote:
>
> > > AFAIK, all the smart pointer classes have bool conversion operators.
> > >
> >
> > They do, but they're explicit and my point was "return bool(sp);" is less
> > good than "return sp != nullptr;" - so without knowing the
> domain-specific
> > bool test it's probably hard to pick/suggest the best fix.
>
> Exactly.
>
> It is hard to know the best idiom without coding in a giant table that
> has to be updated all the time.
>
> In the case of a return statement, you don't need to do anything.
>
> If they wrote 'if (sp != nullptr) return true; else return false;',
> then the tool would replace this with 'return sp != nullptr;'.
>
> If they wrote 'if (sp) return true; else return false;', the tool
> would replace this with 'return sp;'.
>

This ^ transformation is most likely not valid for any modern user defined
type (because they'll have an explicit operator bool, not an implicit one):

$ cat sp.cpp
#include <memory>

bool f1(std::unique_ptr<int> u) {
  if (u)
    return true;
  return false;
}

bool f2(std::unique_ptr<int> u) {
  return u;
}
$ clang++-tot sp.cpp -fsyntax-only  -std=c++11
sp.cpp:10:10: error: no viable conversion from 'std::unique_ptr<int>' to
'bool'
  return u;
         ^
1 error generated.


>
> In both cases, the tool assumes that you wrote one or the other based
> on your preferences of what is readable and it shouldn't be changed.
>
> It's only in the case of a ternary operator embedded within a larger
> expression that you have to do something.  I don't want the tool to subtly
> change the type of the expression from bool to something else that is
> implicitly convertible to bool but might be interpreted as something
> other than bool in the larger context.
>
> However, in the case of a ternary operation, something like:
>
>         foo ^= (p ? true : false);
>
> changing that to:
>
>         foo ^= p;
>
> can introduce errors depending on the type of foo and p.  In these
> cases, at the very least the tool should do:
>
>         foo ^= static_cast<bool>(p);
>
> in order to leave the interpretation of the expression unchanged.
>
> At a later date, more heuristics can be added so that the expression
> is something more idiomatic than static_cast, but that isn't an issue
> of correctness, it's an issue of style and hence subject to all kinds
> of varying opinions.
>
> At this point I am only concerned about correctness.
>

Sure, but your transformation is stylistic in nature - so it's not just
important to preserve semantics, but also to improve readability. Perhaps
it's worth not offering a fixit hint in these cases, until good quality
ones can be provided? I'm not sure.

- David


> --
> "The Direct3D Graphics Pipeline" free book <
> http://tinyurl.com/d3d-pipeline>
>      The Computer Graphics Museum <http://ComputerGraphicsMuseum.org>
>          The Terminals Wiki <http://terminals.classiccmp.org>
>   Legalize Adulthood! (my blog) <http://LegalizeAdulthood.wordpress.com>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150331/60f6b19e/attachment.html>