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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits