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