faisalv marked 14 inline comments as done.
faisalv added a comment.
Thanks for the review! An updated patch that should address your concerns will
follow in a few mins.
I'm still not entirely sure I follow your comment about the intialized-entity:
'directly pass the fact that the capture in question is a capture of *this'.
================
Comment at: lib/Sema/SemaExprCXX.cpp:841-849
@@ -840,11 +840,11 @@
}
if (ThisTy.isNull()) {
if (isGenericLambdaCallOperatorSpecialization(CurContext) &&
CurContext->getParent()->getParent()->isRecord()) {
// This is a generic lambda call operator that is being instantiated
// within a default initializer - so use the enclosing class as 'this'.
// There is no enclosing member function to retrieve the 'this' pointer
// from.
QualType ClassTy = Context.getTypeDeclType(
cast<CXXRecordDecl>(CurContext->getParent()->getParent()));
// There are no cv-qualifiers for 'this' within default initializers,
----------------
rsmith wrote:
> (Only somewhat related to the current change...) This looks wrong. If we're
> in a lambda within a lambda within a default member initializer, we need to
> recurse up more parents to find the right context. Looks like we should be
> walking up to the parent of the closure type, checking whether that is itself
> a lambda, and if so, recursing, until we reach a class or a function that
> isn't a lambda call operator. And we should accumulate the constness of
> `*this` on the way.
Thanks for taking a look at this. Agreed - I'll submit this as a separate fix
- added a fixme.
================
Comment at: lib/Sema/SemaExprCXX.cpp:919
@@ +918,3 @@
+ InitializedEntity Entity = InitializedEntity::InitializeLambdaCapture(
+ &S.Context.Idents.get("*this"), CaptureThisTy, Loc);
+ InitializationKind InitKind = InitializationKind::CreateDirect(Loc, Loc,
Loc);
----------------
rsmith wrote:
> Please don't invent a fake identifier `*this` here. Instead, directly pass
> the fact that the capture in question is a capture of `*this`.
OK - I don't invent a fake identifier and instead pass in nullptr - but to be
honest, I'm not sure what you mean by 'directly pass the fact that the capture
in question is a capture of *this'
================
Comment at: lib/Sema/SemaExprCXX.cpp:982
@@ -950,3 +981,3 @@
// For lambda expressions, build a field and an initializing expression.
- ThisExpr = captureThis(Context, LSI->Lambda, ThisTy, Loc);
+ ThisExpr = captureThis(*this, Context, LSI->Lambda, ThisTy, Loc, ByCopy);
else if (CapturedRegionScopeInfo *RSI
----------------
rsmith wrote:
> This looks wrong: for all scopes other than the innermost one, if `*this` is
> not already captured (explicitly) then it should always be captured by
> reference.
You're absolutely right. Fixed. [=] { return [*this] { return m; }; }; will
now emit the right code.
http://reviews.llvm.org/D18139
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
