[PATCH] Add new warning to Clang to detect when all code paths in a function has a call back to the function.

[Arthur O'Dwyer]

On Wed, Oct 9, 2013 at 3:57 PM, Richard Trieu <rtrieu at google.com> wrote:
> On Wed, Oct 9, 2013 at 2:53 PM, Arthur O'Dwyer <arthur.j.odwyer at gmail.com>
> wrote:
>>
>> Well, I do think that you should run this diagnostic over a few real
>> codebases
>
> Already done.  My comments with the patch reflect the results of running
> this warning over Google code.
[...]
>  (B) where this diagnostic gave false positives on non-template code,
> Nope, didn't find any.

Didn't you find some related to infinite loops? Or else why would you
put in the special case to disable the warning on

    void foo() { while (true) { ... foo(); ... } }

?

>  (C) where this diagnostic gave false positives on template code, and
> // Same example as before.
> template <int value>
> int sum() {
>   return value + sum<value/2>;

Yeah, but this example is syntactically incorrect *and* even if the
syntax errors are fixed it's just a verbose and
buggy-on-negative-numbers substitute for __builtin_popcount(x-y) *and*
I've given you the trivial fix — so I discount this example. :)

> template<int First, int Last>
> void DoStuff() {
>   if (First + 1 == Last) {
>     DoSomethingHere();  // This gets removed during <0, 0> instantiation in
> the CFG, I think.
>   } else {
>     DoStuff<First, (First + Last)/2>();
>     DoStuff<(First + Last)/2, Last>();
>   }
> }
> DoStuff<0, 1>()

That's a good example; it's tricky to rewrite in such a way that no
dead code gets instantiated.
You could rewrite it as

template<int First, int Last>
void DoStuff() {
    if (First >= Last) {
        static_assert(First < Last, "Invalid range in DoStuff<First,Last>");
    } else if (First+1 == Last) {
        DoSomethingElse();
    } else {
        const int Middle = (First+Last)/2;
        DoStuff<First, (First==Middle) ? First+1 : Middle>();  // good
old ternary operator
        DoStuff<Middle, Last>();
    }
}

but I agree that that's ugly.


>  (D) where this diagnostic found bugs in template code?
> struct Value { int num; };
> template<typename T>
> struct Wrapper {
>   int num() { return num(); }  // should be V.num;
>   T V;
> };
> Wrapper<Value> Foo;

Another good example. Given the choice between "warn on both DoStuff
and Wrapper" and "warn on neither DoStuff nor Wrapper", I'd vastly
prefer the former.

–Arthur