[cfe-dev] [clang-tidy] Some possible contributions

[Richard via cfe-dev]

[Please reply *only* to the list and do not include my email directly
in the To: or Cc: of your reply; otherwise I will not see your reply.
Thanks.]

In article <561A524F.209 at gmail.com>,
    Piotr Dziwinski via cfe-dev <cfe-dev at lists.llvm.org> writes:

> As I mentioned previously, my check for implicit bool casts, may not be 
> suited to some coding styles. Personally, I like to avoid implicit bool 
> casts whenever possible, as I find code that relies on them more 
> difficult to read. Sometimes this even leads to hidden bugs, or if not 
> bugs, then at least inconsitent code.

Yes, I would like this check.  I agree that sometimes implicit cast
to bool reveals problems in the code.  I think if you check for a few
common idioms that you can avoid too many false positives.

> I would like to hear, what you think about that. Should some of these 
> styles, e.g. relying on pointer to bool conversion in "if (pointer) {}", 
> be allowed? Maybe I should add config option to enable/disable my check 
> in such cases?

I did some snooping of these situations in my simplify-bool-expr check.

In my check, I simplified

  if (p) { return true; } else { return false; }

to

  return p != nullptr;

This turned the implicit cast to bool into an explicit comparison
resulting in a bool.

The only idioms that I think you need to allow for are:

  if (p) x; else y;
  if (!p) x; else y;
  p ? e : f;
  !p ? e : f;

where p is some pointer.  Maybe there are some other common idioms
that exploit implicit conversion to bool, but that's all I could think
of right now.

That list is pretty small, so it shouldn't be hard to handle.  I do
recommend the idea of adding a configuration option to switch the
behavior to warn even in these cases.  Some people do prefer the more
explicit check against nullptr.

> As regards old-style function check, or more broadly: localizing 
> variables, (http://reviews.llvm.org/D12473), it turned out to be more 
> difficult than I thought at first, and still have not made much 
> progress.

Totally agree.  If I could think of an easy way to implement it, I
would have tried it by now :).

> However, in the meantime, working on other pieces of code, I 
> have become more familiar with Clang codebase, so I think it will be 
> easier for me now. This is what I'll focus on from now on.

The implicit-conversion-to-bool check might be a good stepping stone
in working with the clang-tidy infrastructure.  I would like to see
this check.  Although it's easier for you to code, I'll be honest and
say that I'd get more value out of the localizing variables check.  If
that one could be highly reliable, I'd even have an automated process
that applied this on nightly builds every time the code changed.
-- 
"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>