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

[Piotr Dziwinski via cfe-dev]

On 2015-10-13 22:33, Richard via cfe-dev wrote:
> [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.]
As you wish.

>> 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.

Yes, I was thinking along the same lines. It seems that these are the 
most common cases, where people rely on evaluating pointer as boolean. 
I'll provide a config option and conditionally exclude these cases.
>> 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 :).

I'm working on it now and I think I see how to approach this. I decided 
on writing something like a context-sensitive RecursiveASTVisitor, where 
I can keep track of variable declarations and their uses in different 
scopes. At the end, I just go over the collected data, and propose 
replacements as necessary. Sound complicated but it should do the job.
>> 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.

Thanks, it's nice to hear that my work is being appreciated. I think I 
will be able to make that check reliable enough for production use. Once 
I finish it, I'll test it myself by running it on Colobot codebase 
(100k+ LOC). This should prove to be a suitable field test for it.

Best regards,
Piotr Dziwinski