<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Oct 9, 2013 at 12:35 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">
<div class="im">On Wed, Oct 9, 2013 at 12:27 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">
<div>On Tue, Oct 8, 2013 at 9:09 PM, Arthur O'Dwyer <span dir="ltr"><<a href="mailto:arthur.j.odwyer@gmail.com" target="_blank">arthur.j.odwyer@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Really cool! I don't understand the template example, though...<br>
<br>
// sum<0>() is instantiated, does recursively call itself, but never runs.<br>
template <int value> int sum() { return value + sum<value/2>(); }<br>
// <== notice that your test is actually missing the (), so you've<br>
got a bogus syntax error in there<br>
template<> int sum<0>() { return 1; }<br></blockquote><div><br></div></div><div>My mistake. That should be int sum<1>(). For value > 1, it will eventually call sum<1>(). But if sum<0>() is called, it just calls itself.</div>
</div></div></div></blockquote><div><br></div></div><div>If the analysis is applied to template specializations, then, you'd only get this warning if your code lead to the instantiation of sum<0> - in which case the warning is correct and useful.<br>
<br>I'm with Arthur - it's still not clear to me why this warning would need to avoid templates, as long as it ran on template specializations not template patterns.</div><div><div class="h5"><div> </div></div></div>
</div></div></div></blockquote><div>Here is the code in question, template parameters fixed now:<br></div><div><br></div><div>template <int value></div><div>int sum() {</div><div> return value + sum<value/2>;</div>
<div>}</div><div><br></div><div>template<></div><div>int sum<1>() { return 1; }</div><div><br></div><div>template<int x, int y></div><div>int calculate_value() {</div><div> if (x != y)</div><div> return sum<x - y>();</div>
<div> else</div><div> return 0;</div><div>}</div><div><br></div><div>int value = calculate_value<1,1>();</div><div><br></div><div>calculate_value<1,1>() causes the instantiation of sum<0>(), even though it is protected from being called by the conditional "if (x != y)". sum<0>() is the only self-recursive function here. Are you saying we should warn on that even though the programmer added the proper check to prevent it from being called?</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
You imply that Clang would get confused and think that sum<0> calls<br>
itself... but how can that be, since sum<0> simply returns 1? Or, if<br>
you mean that Clang would mistakenly try to instantiate the regular<br>
("un-specialized"?) version of sum<i> with i=0... how can *that* be,<br>
since you could just as well make it<br>
<br>
template <int value> int sum() { static_assert( i != 0, "" );<br>
return value + sum<value/2>(); }<br>
<br></blockquote></div><div>It would be great if the compiler could only choose to run on good code. In practice, we need to look out for edgecases.</div><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
? Clang shouldn't be trying to static-analyze instantiations that<br>
aren't real. :P<br>
I'd like to see this patch fixed so that it works properly on<br>
templates, because that strikes me as exactly the case where this sort<br>
of warning would be most useful. We want to be able to distinguish the<br>
cases<br>
<br>
template <int value> int sum() { return value + sum<value/2>(); }<br>
// no warning<br>
template<> int sum<0>() { return 1; }<br>
void f() { sum<4>(); }<br>
<br>
and<br>
<br>
template <int value> int sum() { return value + sum<value/2>(); }<br>
// yes warning<br>
void f() { sum<4>(); }<br>
<br>
<br>
Also, FWIW, I see no reason to special-case infinite loops.<br>
<br>
void f() { while (true) f(); }<br>
<br>
is buggy enough to deserve a diagnostic, IMO. (Such a construct<br>
probably never happens in practice, but when it *does* happen, that<br>
one time in a million, and an engineer spends three hours tracking it<br>
down, he'll probably be annoyed that Clang specifically considered<br>
pointing out the problem and then silently swallowed it.)<br></blockquote><div><br></div></div><div>Except that there's plenty of reasons to allow this. Probably the most common are programs that should never terminate and have a while(true) loop in their main function. It's possible to make a list of the well-known patterns to ignore [while(1), while(true), for(;;)] and warn on the rest.</div>
<div><div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
my $.02,<br>
–Arthur<br>
<div><div><br>
<br>
On Tue, Oct 8, 2013 at 8:13 PM, Richard Trieu <<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>> wrote:<br>
> Implement a warning to detect when a function will call itself recursively on every code path. If a program ever calls such a function, the function will attempt to call itself until it runs out of stack space.<br>
><br>
> This warning searches the CFG to determine if every codepath results in a self call. In addition to the test for this warning, several other tests needed to be fixed, and a pragma to prevent this warning where Clang really wants a stack overflow.<br>
><br>
> Testing with this warning has already caught several buggy functions. Common mistakes include: incorrect namespaces, wrapper classes not forwarding calls properly, similarly named member function and data member, and failing to call an overload of the same function. When run outside of template instantiations, all true positives. In template instantiations, only 25% true positive. Therefore, this warning is disabled in template instantiations. An example of such a false positive is in the test cases.<br>
><br>
> <a href="http://llvm-reviews.chandlerc.com/D1864" target="_blank">http://llvm-reviews.chandlerc.com/D1864</a><br>
><br>
> Files:<br>
> test/Analysis/inlining/test-always-inline-size-option.c<br>
> test/Analysis/misc-ps-region-store.cpp<br>
> test/Analysis/cxx11-crashes.cpp<br>
> test/FixIt/typo.m<br>
> test/FixIt/fixit.c<br>
> test/Sema/unused-expr-system-header.c<br>
> test/Sema/warn-unused-function.c<br>
> test/Sema/attr-deprecated.c<br>
> test/Parser/cxx-using-declaration.cpp<br>
> test/Parser/expressions.c<br>
> test/CodeGen/functions.c<br>
> test/SemaCXX/statements.cpp<br>
> test/SemaCXX/warn-bool-conversion.cpp<br>
> test/SemaCXX/warn-infinite-recursion.cpp<br>
> test/SemaCXX/MicrosoftCompatibility.cpp<br>
> test/Lexer/gnu-flags.c<br>
> include/clang/Basic/DiagnosticSemaKinds.td<br>
> include/clang/Basic/DiagnosticGroups.td<br>
> lib/Sema/AnalysisBasedWarnings.cpp<br>
> lib/Lex/Pragma.cpp<br>
><br>
</div></div>> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
><br>
</blockquote></div></div></div><br></div></div>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>