[clang] Fix aggregate initialization inside lambda constexpr

[Fznamznon]

Summary

Constant evaluator only considered access to this pointer to be
possible if this poitners was captured. However this can also appear if
there was a default member initializer.

Fixes https://github.com/llvm/llvm-project/issues/60936

[shafik]

Thank you for this patch, it looks good.

clang/lib/AST/ExprConstant.cpp

8763–8766

It might be worth it to review all the examples here: https://eel.is/c++draft/expr.prim.lambda

and make sure the test we have actually covers all the scenarios. It looks like we capture most of them but I have not gone over them fully.

clang/test/SemaCXX/lambda-expressions.cpp

673

It would be helpful to add a comment about the implicit this access.

[shafik]

Also please add a release note before landing the fix, thank you.

[Fznamznon]

Apply suggestions and rebase.

[Fznamznon]

clang/lib/AST/ExprConstant.cpp

8763–8766

At least the ones about this capture seem to be working and covered. Not all of them have the same behavior as described though, for example on https://eel.is/c++draft/expr.prim.lambda#closure-6 clang complains about missing capture, though gcc agrees https://godbolt.org/z/hfxbEP5fW , so this is probably a bug in the example. Anyway I think this is a bit out of the scope of the patch.

[shafik]

LGTM, the modules failures is a previous known issue.

clang/lib/AST/ExprConstant.cpp

8763–8766

I will dig into that discrepancy.

[shafik]

clang/lib/AST/ExprConstant.cpp

8763–8766

If you move the lambda to be a global, it works: https://godbolt.org/z/84ax7518o which is what the original example has.

[Fznamznon]

Thanks for the review @shafik .

clang/lib/AST/ExprConstant.cpp

8763–8766

Yes, this is understandable. But in the example there is also an assert which doesn't do well in global scope.

BTW, in case this is interesting, another one that doesn't seem to be working with clang is https://eel.is/c++draft/expr.prim.lambda#capture-example-2
https://godbolt.org/z/dP71j3MxK.

[shafik]

clang/lib/AST/ExprConstant.cpp

8763–8766

Yes, this is understandable. But in the example there is also an assert which doesn't do well in global scope.

Oh interesting, I missed that one!

BTW, in case this is interesting, another one that doesn't seem to be working with clang is https://eel.is/c++draft/expr.prim.lambda#capture-example-2
https://godbolt.org/z/dP71j3MxK.

Another interesting one, if we change it to int y clang diagnoses it: https://godbolt.org/z/rjx71qj8j

I am not sure what the (y) means I think that should be ill-formed, seems like a clang bug. Let me look at it some more.

Thank you for doing this deeper look.

[shafik]

clang/lib/AST/ExprConstant.cpp

8763–8766

(y) is a type b/c it is a template parameter, duh!

I filed a bug report against clang: https://github.com/llvm/llvm-project/issues/61105

clang/docs/ReleaseNotes.rst

- Fix assert that fails when the expression causing the this pointer to be
   instantiation of the enclosing method causes the branch to be discarded.
 - Fix __VA_OPT__ implementation so that it treats the concatenation of a
   non-placemaker token and placemaker token as a non-placemaker token.
   (`#60268 <https://github.com/llvm/llvm-project/issues/60268>`_)
 - Fix crash when taking the address of a consteval lambda call operator.
   (`#57682 <https://github.com/llvm/llvm-project/issues/57682>`_)
 - Clang now support export declarations in the language linkage.
   (`#60405 <https://github.com/llvm/llvm-project/issues/60405>`_)
+- Fix aggregate initialization inside lambda constexpr.
+  (`#60936 <https://github.com/llvm/llvm-project/issues/60936>`_)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to Attribute Support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to C++ Support

clang/lib/AST/ExprConstant.cpp

bool VisitCXXThisExpr(const CXXThisExpr *E) {
     if (!Info.CurrentCall->This) {
       if (Info.getLangOpts().CPlusPlus11)
         Info.FFDiag(E, diag::note_constexpr_this) << E->isImplicit();
       else
         Info.FFDiag(E);
       return false;
     }
     Result = *Info.CurrentCall->This;
-    // If we are inside a lambda's call operator, the 'this' expression refers
-    // to the enclosing '*this' object (either by value or reference) which is
-    // either copied into the closure object's field that represents the '*this'
-    // or refers to '*this'.
+
     if (isLambdaCallOperator(Info.CurrentCall->Callee)) {
-      // Ensure we actually have captured 'this'. (an error will have
-      // been previously reported if not).
+      // Ensure we actually have captured 'this'. If something was wrong with
+      // 'this' capture, the error would have been previously reported.
+      // Otherwise we can be inside of a default initialization of an object
+      // declared by lambda's body, so no need to return false.
       if (!Info.CurrentCall->LambdaThisCaptureField)
-        return false;
+        return true;
 
+      // If we have captured 'this',  the 'this' expression refers
+      // to the enclosing '*this' object (either by value or reference) which is
+      // either copied into the closure object's field that represents the
+      // '*this' or refers to '*this'.
       // Update 'Result' to refer to the data member/field of the closure object
       // that represents the '*this' capture.
       if (!HandleLValueMember(Info, E, Result,
                              Info.CurrentCall->LambdaThisCaptureField))
         return false;
       // If we captured '*this' by reference, replace the field with its referent.
       if (Info.CurrentCall->LambdaThisCaptureField->getType()
               ->isPointerType()) {

clang/test/SemaCXX/lambda-expressions.cpp

[] { return i; }; // expected-error {{variable '' cannot be implicitly captured in a lambda with no capture-default specified}}
                     // expected-note@-2 2 {{default capture by}}
 }
 };
 
 namespace GH60518 {
 // Lambdas should not try to capture
 // function parameters that are used in enable_if
 struct StringLiteral {
 template <int N>
 StringLiteral(const char (&array)[N])
     __attribute__((enable_if(__builtin_strlen(array) == 2,
                               "invalid string literal")));
 };
 
 namespace cpp_attribute {
 struct StringLiteral {
 template <int N>
 StringLiteral(const char (&array)[N]) [[clang::annotate_type("test", array)]];
 };
 }
 
 void Func1() {
   [[maybe_unused]] auto y = [&](decltype(StringLiteral("xx"))) {};
   [[maybe_unused]] auto z = [&](decltype(cpp_attribute::StringLiteral("xx"))) {};
 }
 
 }
+
+#if __cplusplus > 201402L
+namespace GH60936 {
+struct S {
+  int i;
+  // `&i` in default initializer causes implicit `this` access.
+  int *p = &i;
+};
+
+static_assert([]() constexpr {
+  S r = S{2};
+  return r.p != nullptr;
+}());
+} // namespace GH60936
+#endif