<p dir="ltr">I'd also like to know whether there are cases that this patch addresses but Taewook Oh's patch does not, as the other patch involves a lot less complexity.</p>
<div class="gmail_quote">On 18 May 2016 10:15 a.m., "Richard Smith via cfe-commits" <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">rsmith added inline comments.<br>
<br>
================<br>
Comment at: lib/Sema/SemaExprCXX.cpp:898<br>
@@ +897,3 @@<br>
+  // end of the TU) we need to be able to examine its enclosing lambdas and so<br>
+  // we use the DeclContext to get a hold of the ClosureClass and query it for<br>
+  // capture information.  The reason we don't just resort to always using the<br>
----------------<br>
No camel case for ClosureClass.<br>
<br>
================<br>
Comment at: lib/Sema/SemaExprCXX.cpp:910<br>
@@ +909,3 @@<br>
+      assert(IsFirstIteration);<br>
+      assert(CurLSI->CallOperator->getParent()->getParent() == CurDC);<br>
+      CurDC = CurLSI->CallOperator;<br>
----------------<br>
Please add a comment explaining this, I have no idea what special case you're checking for here.<br>
<br>
================<br>
Comment at: lib/Sema/SemaExprCXX.cpp:952<br>
@@ +951,3 @@<br>
+    while (Closure &&<br>
+           IsThisCaptured(Closure, IsByCopyCapture, IsConstCapture)) {<br>
+      if (IsByCopyCapture) {<br>
----------------<br>
Should you really stop here if this is not captured? There could still be a surrounding lambda with a mutable *this capture.<br>
<br>
================<br>
Comment at: lib/Sema/SemaExprCXX.cpp:1062<br>
@@ -964,1 +1061,3 @@<br>
+  Expr *This =<br>
+      new (Context) CXXThisExpr(Loc, AdjustedThisTy, /*isImplicit*/ true);<br>
   if (ByCopy) {<br>
----------------<br>
faisalv wrote:<br>
> 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?<br>
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.<br>
<br>
<br>
<a href="http://reviews.llvm.org/D19783" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19783</a><br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>