[PATCH] D144866: [clang] Fix aggregate initialization inside lambda constexpr

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.


================
Comment at: clang/lib/AST/ExprConstant.cpp:8763
     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.
----------------
Fznamznon wrote:
> shafik wrote:
> > shafik wrote:
> > > Fznamznon wrote:
> > > > shafik wrote:
> > > > > 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.
> > > > 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.
> > > I will dig into that discrepancy. 
> > If you move the lambda to be a global, it works: https://godbolt.org/z/84ax7518o which is what the original example has.
> 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.
> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144866/new/

https://reviews.llvm.org/D144866