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

[David Blaikie]

On Tue, Mar 31, 2015 at 1:01 PM, Richard <legalize at xmission.com> wrote:

>
> In article <
> CAENS6EuerX1Fvf0NEo7fYvn3pR1RJHQxahGA+-C8ztZkgRi5yQ at mail.gmail.com>,
>     David Blaikie <dblaikie at gmail.com> writes:
>
> > On Tue, Mar 31, 2015 at 10:06 AM, Richard <legalize at xmission.com> wrote:
> > > 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.
>
> Good point.  So I guess I need to handle these cases and it looks like
> I can't avoid some type introspection in order to figure out what to
> do.
>
> > > 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.
>
> I think some type introspection can be done to deal with builtin types
> and standard library types.
>
> Yes, the transformation is stylistic, but it is also human invoked.
>

clang-tidy isn't necessarily used for transformation in particular - I
think of it more as a linter tool: suggesting problems, sometimes being
able to provide likely solutions. So I wouldn't necessarily worry about
providing fixit hints in all cases, especially as the fixit gets harder to
provide with high quality, etc.


> The user has asked for this transformation to be performed, so I'm ok
> with transforming it in such a way that it leaves correctness intact
> albeit possibly with an ugly static_cast<> lurking in there if I can't
> figure out something better.


I don't fundamentally object to a fixit hint including static_cast<bool>()
or bool(), I'm just not sure it's necessarily better than not suggesting
one & leaving it up to the user in cases where it's not obvious *shrug*


> (Personally I don't know why people are
> against the 'functional cast' bool(e), particularly when that's the
> wording in the standard.)
>

I tend to treat c-style casts with more care because I know they can do
more problematic transformations (I suppose when the target type is bool
this isn't an issue? I'm not sure - I'd have to go back & read the spec
about the sequence of conversions c-style casts can perform)

- 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/49be74e1/attachment.html>