On Wed, Oct 9, 2013 at 12:35 PM, David Blaikie <[email protected]> wrote:
> > > > On Wed, Oct 9, 2013 at 12:27 PM, Richard Trieu <[email protected]> wrote: > >> >> >> >> On Tue, Oct 8, 2013 at 9:09 PM, Arthur O'Dwyer <[email protected] >> > wrote: >> >>> Really cool! I don't understand the template example, though... >>> >>> // sum<0>() is instantiated, does recursively call itself, but never >>> runs. >>> template <int value> int sum() { return value + sum<value/2>(); } >>> // <== notice that your test is actually missing the (), so you've >>> got a bogus syntax error in there >>> template<> int sum<0>() { return 1; } >>> >> >> 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. >> > > 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. > > 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. > > Here is the code in question, template parameters fixed now: template <int value> int sum() { return value + sum<value/2>; } template<> int sum<1>() { return 1; } template<int x, int y> int calculate_value() { if (x != y) return sum<x - y>(); else return 0; } int value = calculate_value<1,1>(); 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? > >>> You imply that Clang would get confused and think that sum<0> calls >>> itself... but how can that be, since sum<0> simply returns 1? Or, if >>> you mean that Clang would mistakenly try to instantiate the regular >>> ("un-specialized"?) version of sum<i> with i=0... how can *that* be, >>> since you could just as well make it >>> >>> template <int value> int sum() { static_assert( i != 0, "" ); >>> return value + sum<value/2>(); } >>> >>> It would be great if the compiler could only choose to run on good code. >> In practice, we need to look out for edgecases. >> >> >>> ? Clang shouldn't be trying to static-analyze instantiations that >>> aren't real. :P >>> I'd like to see this patch fixed so that it works properly on >>> templates, because that strikes me as exactly the case where this sort >>> of warning would be most useful. We want to be able to distinguish the >>> cases >>> >>> template <int value> int sum() { return value + sum<value/2>(); } >>> // no warning >>> template<> int sum<0>() { return 1; } >>> void f() { sum<4>(); } >>> >>> and >>> >>> template <int value> int sum() { return value + sum<value/2>(); } >>> // yes warning >>> void f() { sum<4>(); } >>> >>> >>> Also, FWIW, I see no reason to special-case infinite loops. >>> >>> void f() { while (true) f(); } >>> >>> is buggy enough to deserve a diagnostic, IMO. (Such a construct >>> probably never happens in practice, but when it *does* happen, that >>> one time in a million, and an engineer spends three hours tracking it >>> down, he'll probably be annoyed that Clang specifically considered >>> pointing out the problem and then silently swallowed it.) >>> >> >> 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. >> >>> >>> my $.02, >>> –Arthur >>> >>> >>> On Tue, Oct 8, 2013 at 8:13 PM, Richard Trieu <[email protected]> wrote: >>> > 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. >>> > >>> > 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. >>> > >>> > 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. >>> > >>> > http://llvm-reviews.chandlerc.com/D1864 >>> > >>> > Files: >>> > test/Analysis/inlining/test-always-inline-size-option.c >>> > test/Analysis/misc-ps-region-store.cpp >>> > test/Analysis/cxx11-crashes.cpp >>> > test/FixIt/typo.m >>> > test/FixIt/fixit.c >>> > test/Sema/unused-expr-system-header.c >>> > test/Sema/warn-unused-function.c >>> > test/Sema/attr-deprecated.c >>> > test/Parser/cxx-using-declaration.cpp >>> > test/Parser/expressions.c >>> > test/CodeGen/functions.c >>> > test/SemaCXX/statements.cpp >>> > test/SemaCXX/warn-bool-conversion.cpp >>> > test/SemaCXX/warn-infinite-recursion.cpp >>> > test/SemaCXX/MicrosoftCompatibility.cpp >>> > test/Lexer/gnu-flags.c >>> > include/clang/Basic/DiagnosticSemaKinds.td >>> > include/clang/Basic/DiagnosticGroups.td >>> > lib/Sema/AnalysisBasedWarnings.cpp >>> > lib/Lex/Pragma.cpp >>> > >>> > _______________________________________________ >>> > cfe-commits mailing list >>> > [email protected] >>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> > >>> >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
