On Wed, Oct 9, 2013 at 3:57 PM, Richard Trieu <[email protected]> wrote:
> On Wed, Oct 9, 2013 at 2:53 PM, Arthur O'Dwyer <[email protected]>
> 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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
