[PATCH] D19783: Fix cv-qualification of '*this' captures (and nasty bug PR27507 introduced by commit 263921 "Implement Lambda Capture of *this by Value as [=, *this]")

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: lib/Sema/SemaExprCXX.cpp:898
@@ +897,3 @@
+  // end of the TU) we need to be able to examine its enclosing lambdas and so
+  // we use the DeclContext to get a hold of the ClosureClass and query it for
+  // capture information.  The reason we don't just resort to always using the
----------------
No camel case for ClosureClass.

================
Comment at: lib/Sema/SemaExprCXX.cpp:910
@@ +909,3 @@
+      assert(IsFirstIteration);
+      assert(CurLSI->CallOperator->getParent()->getParent() == CurDC);
+      CurDC = CurLSI->CallOperator;
----------------
Please add a comment explaining this, I have no idea what special case you're checking for here.

================
Comment at: lib/Sema/SemaExprCXX.cpp:952
@@ +951,3 @@
+    while (Closure &&
+           IsThisCaptured(Closure, IsByCopyCapture, IsConstCapture)) {
+      if (IsByCopyCapture) {
----------------
Should you really stop here if this is not captured? There could still be a surrounding lambda with a mutable *this capture.

================
Comment at: lib/Sema/SemaExprCXX.cpp:1062
@@ -964,1 +1061,3 @@
+  Expr *This =
+      new (Context) CXXThisExpr(Loc, AdjustedThisTy, /*isImplicit*/ true);
   if (ByCopy) {
----------------
faisalv wrote:
> Hmm - I wonder if instead of using AdjustedThisTy, I should always use 'ThisTy' since it is being used in the initializer expression, and should probably inherit its cv qualifiers from an enclosing lambda? correct?
Yes, don't use AdjustedThisTy here. You can test for this bug by giving the class both a (const T&) and a (T&)=delete copy ctor.


http://reviews.llvm.org/D19783