GH60642: Fix ICE when checking a lambda defined in a concept definition

[erichkeane]

Summary

As reported in GH60642, we asserted when there was a lambda defined in a
template arguments inside of a concept, which caused us to not properly
set up the list of instantiation args. This patch ensures that the
'lambda context decl' correctly falls-through the template argument
instantiation, so that it is available when instantiating the lambda,
and thus, when setting up the lambda instantiation args list.

[erichkeane]

I don't really need reviewers here, I'm more putting this on phab to get the libcxx testing, once that succeeds, i intend to do this as Review-after-commit if there are no concerns (and also ask for this to go to 16.0, as it is a regression).

[shafik]

clang/lib/Sema/TreeTransform.h

4572

Not for this PR but I am looking at other place we use EnterExpressionEvaluationContext and it is not obvious to me when we should be using Sema::ReuseLambdaContextDecl or not. So might be worth looking at other uses as well.

[erichkeane]

clang/lib/Sema/TreeTransform.h

4572

Yeah, its unfortunately not particularly clear to me either. We should be re-using that context any time we're going "down" in code (that is, if we are potentially in a context where a lambda is defined inside of something its actual declcontext doesn't match), but should NOT reuse the context in a case where we are 'swapping' contexts to do something like instantiate a function required by the current context/etc.

[erichkeane]

Looks like I have ot abandon this now? Anyway, fix was committed, but I forgot the differential-revision line.

[philnik]

In D144192#4134887, @erichkeane wrote:

Looks like I have ot abandon this now? Anyway, fix was committed, but I forgot the differential-revision line.

You can also Close the revision instead of abandoning it to make it obvious that it has been landed.

[erichkeane]

In D144192#4134893, @philnik wrote:

In D144192#4134887, @erichkeane wrote:

Looks like I have ot abandon this now? Anyway, fix was committed, but I forgot the differential-revision line.

You can also Close the revision instead of abandoning it to make it obvious that it has been landed.

I didn't actually have that option in the "Add Action" section? I was looking for it, but it seems to have disappeared?

[philnik]

In D144192#4134894, @erichkeane wrote:

In D144192#4134893, @philnik wrote:

In D144192#4134887, @erichkeane wrote:

Looks like I have ot abandon this now? Anyway, fix was committed, but I forgot the differential-revision line.

You can also Close the revision instead of abandoning it to make it obvious that it has been landed.

I didn't actually have that option in the "Add Action" section? I was looking for it, but it seems to have disappeared?

I have it on my open revisions, maybe you just missed it?

[erichkeane]

In D144192#4134900, @philnik wrote:

In D144192#4134894, @erichkeane wrote:

In D144192#4134893, @philnik wrote:

In D144192#4134887, @erichkeane wrote:

Looks like I have ot abandon this now? Anyway, fix was committed, but I forgot the differential-revision line.

You can also Close the revision instead of abandoning it to make it obvious that it has been landed.

I didn't actually have that option in the "Add Action" section? I was looking for it, but it seems to have disappeared?

I have it on my open revisions, maybe you just missed it?

This is what I have for options on my open revisions (I went and grabbed a snap from a different review): https://imgur.com/a/WYD3uAV

So I don't see 'close' as an option?

[philnik]

In D144192#4134902, @erichkeane wrote:

In D144192#4134900, @philnik wrote:

In D144192#4134894, @erichkeane wrote:

In D144192#4134893, @philnik wrote:

In D144192#4134887, @erichkeane wrote:

Looks like I have ot abandon this now? Anyway, fix was committed, but I forgot the differential-revision line.

You can also Close the revision instead of abandoning it to make it obvious that it has been landed.

I didn't actually have that option in the "Add Action" section? I was looking for it, but it seems to have disappeared?

I have it on my open revisions, maybe you just missed it?

This is what I have for options on my open revisions (I went and grabbed a snap from a different review): https://imgur.com/a/WYD3uAV

So I don't see 'close' as an option?

Weird. I see a few more options:

clang/lib/Sema/TreeTransform.h

case TemplateArgument::TemplateExpansion:
     llvm_unreachable("Caller should expand pack expansions");
 
   case TemplateArgument::Expression: {
     // Template argument expressions are constant expressions.
     EnterExpressionEvaluationContext Unevaluated(
         getSema(),
         Uneval ? Sema::ExpressionEvaluationContext::Unevaluated
                : Sema::ExpressionEvaluationContext::ConstantEvaluated,
-        /*LambdaContextDecl=*/nullptr, /*ExprContext=*/
+        Sema::ReuseLambdaContextDecl, /*ExprContext=*/
         Sema::ExpressionEvaluationContextRecord::EK_TemplateArgument);
 
     Expr *InputExpr = Input.getSourceExpression();
     if (!InputExpr)
       InputExpr = Input.getArgument().getAsExpr();
 
     ExprResult E = getDerived().TransformExpr(InputExpr);
     E = SemaRef.ActOnConstantExpression(E);

clang/test/SemaTemplate/concepts-lambda.cpp

 // RUN: %clang_cc1 -std=c++20 -verify %s
 // RUN: %clang_cc1 -std=c++20 -verify %s -triple powerpc64-ibm-aix
-// expected-no-diagnostics
 
 namespace GH57945 {
   template<typename T>
     concept c = true;
 
   template<typename>
     auto f = []() requires c<void> {
     };
struct Foo {
   int i;
   double j;
   char k;
 };
 
 static_assert(ConstructibleWithN<Foo>);
 
 }
+
+// GH60642 reported an assert being hit, make sure we don't assert.
+namespace GH60642 {
+template<auto Q> concept C = requires { Q.template operator()<float>(); };
+template<class> concept D = true;
+static_assert(C<[]<D>{}>);  // ok
+template<class> concept E = C<[]<D>{}>;
+static_assert(E<int>);  // previously Asserted.
+
+// ensure we properly diagnose when "D" is false.
+namespace DIsFalse {
+template<auto Q> concept C = requires { Q.template operator()<float>(); };
+template<class> concept D = false;
+static_assert(C<[]<D>{}>);
+// expected-error@-1{{static assertion failed}}
+// expected-note@-2{{does not satisfy 'C'}}
+// expected-note@-5{{because 'Q.template operator()<float>()' would be invalid: no matching member function for call to 'operator()'}}
+template<class> concept E = C<[]<D>{}>;
+static_assert(E<int>);
+// expected-error@-1{{static assertion failed}}
+// expected-note@-2{{because 'int' does not satisfy 'E'}}
+// expected-note@-4{{does not satisfy 'C'}}
+// expected-note@-11{{because 'Q.template operator()<float>()' would be invalid: no matching member function for call to 'operator()'}}
+}
+}