[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]

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.
On 18 May 2016 10:15 a.m., "Richard Smith via cfe-commits" <
cfe-commits at lists.llvm.org> wrote:

> 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
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160518/f511c19a/attachment-0001.html>