Clang crash on infinite template instantiation

Clang Frontend
1

Hello Clang Dev,

Recently I looked into the clang bug 12053,

template 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 auto foo(T t) → decltype(bar(t)) {}

template 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?

Thanks!

Wei Pan

decl_type.patch (2.26 KB)

2

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 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.

3

Hi Richard,

Disabling BuildRecoveryCallExpr seems correct to solving this problem. Do you think this patch is correct?

Thanks!

Wei

a.patch (4.33 KB)

4

— 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 auto foo(T t) → decltpye(foo(t)) {}
  • // template 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 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 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());