On Fri, Sep 21, 2012 at 1:42 PM, Pan, Wei <[email protected]> wrote: > Hello Richard,**** > > ** ** > > Does the attached patch look good for this bug? All your suggestions were > applied. >
I'm surprised that we now produce four (fatal!) errors for instantiation-depth-subst.cpp. We shouldn't be emitting diagnostics after the first fatal error. Do you know why it's happening? Please add a FIXME to that test for that. Please merge your new test file (test/SemaCXX/PR12053.cpp) into test/SemaCXX/trailing-return-0x.cpp (put the contents into a namespace PR12053 in that file). Then this LGTM. Do you need me to commit this for you? > > > Thanks,**** > > ** ** > > Wei**** > > ** ** > > *From:* [email protected] [mailto:[email protected]] *On Behalf Of *Richard > Smith > *Sent:* Friday, September 07, 2012 4:26 PM > > *To:* Pan, Wei > *Cc:* [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]> 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]] *On Behalf Of *Richard > Smith > *Sent:* Thursday, September 06, 2012 6:26 PM > *To:* Pan, Wei > *Cc:* [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]> 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.**** > > ** ** >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
