Hi Richard, I just updated the patch to sync with some recent changes. Now no change will be made on the test instantiation-depth-subst.cpp.
Thanks for reviewing! Wei From: Pan, Wei Sent: Friday, September 21, 2012 4:43 PM To: Richard Smith Cc: [email protected] Subject: RE: [cfe-dev] Clang crash on infinite template instantiation Hello Richard, Does the attached patch look good for this bug? All your suggestions were applied. Thanks, Wei From: [email protected]<mailto:[email protected]> [mailto:[email protected]] On Behalf Of Richard Smith Sent: Friday, September 07, 2012 4:26 PM To: Pan, Wei Cc: [email protected]<mailto:[email protected]> Subject: Re: [cfe-dev] Clang crash on infinite template instantiation > --- a/include/clang/Sema/Sema.h > +++ b/include/clang/Sema/Sema.h For your future consideration: Convention on these lists is to use -p0 patches instead of -p1 patches. You can configure git with '[diff] noprefix = true' for this. Also, patches should generally be cfe-commits@, not cfe-dev@. The patch generally looks fine. A few minor things: > @@ -1939,6 +1939,10 @@ public: > BEF_end > }; > > + /// IsBuildingRecoveryCallExpr - True if Sema is building a recovery call Please use \brief instead of repeating the variable name. > + /// expression. > + bool IsBuildingRecoveryCallExpr; > + > ForRangeStatus BuildForRangeBeginEndCall(Scope *S, SourceLocation Loc, Please move this to the top of the class, with the other member variables. > + // template <typename T> auto foo(T t) -> decltpye(foo(t)) {} > + // template <typename T> auto foo(T t) -> decltpye(foo(&t)) {} Typo "decltpye". > +++ b/test/SemaTemplate/instantiation-depth-subst.cpp > @@ -1,9 +1,6 @@ > // RUN: %clang_cc1 -std=c++11 -verify %s -ftemplate-depth 2 > > // PR9793 > -template<typename T> auto f(T t) -> decltype(f(t)); // \ > -// expected-error {{recursive template instantiation exceeded maximum depth > of 2}} \ > -// expected-note 3 {{while substituting}} \ > -// expected-note {{candidate}} > +template<typename T> auto f(T t) -> decltype(f(t)); // expected-note > {{candidate template ignored}} > > int k = f(0); // expected-error {{no matching function for call to 'f'}} This test is no longer testing what it was intended to test (that we have a depth limit for pure substitution). Please instead change the test as follows (so 'f' can be found within its own return type via ADL): -int k = f(0); +struct S {}; +int k = f(S()); On Fri, Sep 7, 2012 at 9:39 AM, Pan, Wei <[email protected]<mailto:[email protected]>> wrote: Hi Richard, Disabling BuildRecoveryCallExpr seems correct to solving this problem. Do you think this patch is correct? Thanks! Wei From: [email protected]<mailto:[email protected]> [mailto:[email protected]<mailto:[email protected]>] On Behalf Of Richard Smith Sent: Thursday, September 06, 2012 6:26 PM To: Pan, Wei Cc: [email protected]<mailto:[email protected]> Subject: Re: [cfe-dev] Clang crash on infinite template instantiation On Thu, Sep 6, 2012 at 1:50 PM, Pan, Wei <[email protected]<mailto:[email protected]>> wrote: Hello Clang Dev, Recently I looked into the clang bug 12053, template <typename T> auto foo(T t) -> decltype(foo(t)) {} http://llvm.org/bugs/show_bug.cgi?id=12053 which crashes clang (trunk) (and gcc 4.6 too). As far as I know, Clang does realize that there is no candidate available while resolving "decltype(foo(t))", however BuildRecoveryCallExpr will find template foo (in DiagnoseTwoPhaseLookup) and try to instantiate it again. This leads to the crash. I am wondering if the following is the right way to fix this. The basic idea is: Before starting instantiating a function template, we check if the same function template instantiation *with the same template arguments* is already in-progress. If yes, then clang is not making any progress and should lead an infinite loop. We treat it as an SFINAE. The attached patch will fix the clang crashing on the above test and other similar tests like template <typename T> auto foo(T t) -> decltype(bar(t)) {} template <typename T> auto bar(T t) -> decltype(foo(t)) {} int x = foo(0); This is not a final patch since this change will affect two tests (only these two) instantiation-depth-subst.cpp and instantiation-depth-subst-2.cpp. Any thoughts? I generally like the idea of checking for cyclic function template instantiations, but I'm hesitant about this approach -- scanning through the instantiation stack each time introduces overhead which grows quadratically in the instantiation depth, which could be too slow for deep instantiation stacks. Also, this approach doesn't solve the entire problem. For instance, in this case, there is no cycle: template<typename T> auto foo(T t) -> decltype(foo(&t)) {} Perhaps we can solve this more directly, by just disabling BuildRecoveryCallExpr when we're already in the middle of recovery.
pr12053.patch
Description: pr12053.patch
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
