shafik added a comment.

Thank you for this patch, it looks good.



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


================
Comment at: clang/test/SemaCXX/lambda-expressions.cpp:673
+    int i;
+    int *p = &i;
+};
----------------
It would be helpful to add a comment about the implicit `this` access.


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

Reply via email to