faisalv updated this revision to Diff 57733.
faisalv marked an inline comment as done.
faisalv added a comment.
This patch addresses all of Richard's comments - except one (on which I'm
awaiting some additional clarity on, before I make any changes).
http://reviews.llvm.org/D19783
Files:
include/clang/Sema/ScopeInfo.h
lib/Parse/ParseDeclCXX.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaExprCXX.cpp
test/SemaCXX/cxx1z-lambda-star-this.cpp
Index: test/SemaCXX/cxx1z-lambda-star-this.cpp
===================================================================
--- test/SemaCXX/cxx1z-lambda-star-this.cpp
+++ test/SemaCXX/cxx1z-lambda-star-this.cpp
@@ -3,6 +3,8 @@
// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fms-extensions %s -DMS_EXTENSIONS
// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fdelayed-template-parsing -fms-extensions %s -DMS_EXTENSIONS -DDELAYED_TEMPLATE_PARSING
+template<class, class> constexpr bool is_same = false;
+template<class T> constexpr bool is_same<T, T> = true;
namespace test_star_this {
namespace ns1 {
@@ -69,4 +71,137 @@
b.foo(); //expected-note{{in instantiation}}
} // end main
} // end ns4
+namespace ns5 {
+
+struct X {
+ double d = 3.14;
+ X(const volatile X&);
+ void foo() {
+
+ }
+
+ void foo() const { //expected-note{{const}}
+
+ auto L = [*this] () mutable {
+ static_assert(is_same<decltype(this), X*>);
+ ++d;
+ auto M = [this] {
+ static_assert(is_same<decltype(this), X*>);
+ ++d;
+ auto N = [] {
+ static_assert(is_same<decltype(this), X*>);
+ };
+ };
+ };
+
+ auto L1 = [*this] {
+ static_assert(is_same<decltype(this), const X*>);
+ auto M = [this] () mutable {
+ static_assert(is_same<decltype(this), const X*>);
+ auto N = [] {
+ static_assert(is_same<decltype(this), const X*>);
+ };
+ };
+ auto M2 = [*this] () mutable {
+ static_assert(is_same<decltype(this), X*>);
+ auto N = [] {
+ static_assert(is_same<decltype(this), X*>);
+ };
+ };
+ };
+ auto L2 = [this] () mutable {
+ static_assert(is_same<decltype(this), const X*>);
+ ++d; //expected-error{{cannot assign}}
+ };
+ auto GL = [*this] (auto a) mutable {
+ static_assert(is_same<decltype(this), X*>);
+ ++d;
+ auto M = [this] (auto b) {
+ static_assert(is_same<decltype(this), X*>);
+ ++d;
+ auto N = [] (auto c) {
+ static_assert(is_same<decltype(this), X*>);
+ };
+ N(3.14);
+ };
+ M("abc");
+ };
+ GL(3.14);
+
+ }
+ void foo() volatile const {
+ auto L = [this] () {
+ static_assert(is_same<decltype(this), const volatile X*>);
+ auto M = [*this] () mutable {
+ static_assert(is_same<decltype(this), X*>);
+ auto N = [this] {
+ static_assert(is_same<decltype(this), X*>);
+ auto M = [] {
+ static_assert(is_same<decltype(this), X*>);
+ };
+ };
+ auto N2 = [*this] {
+ static_assert(is_same<decltype(this), const X*>);
+ };
+ };
+ auto M2 = [*this] () {
+ static_assert(is_same<decltype(this), const X*>);
+ auto N = [this] {
+ static_assert(is_same<decltype(this), const X*>);
+ };
+ };
+ };
+ }
+
+};
+
+} //end ns5
+namespace ns6 {
+struct X {
+ double d;
+ auto foo() const {
+ auto L = [*this] () mutable {
+ auto M = [=] (auto a) {
+ auto N = [this] {
+ ++d;
+ static_assert(is_same<decltype(this), X*>);
+ auto O = [*this] {
+ static_assert(is_same<decltype(this), const X*>);
+ };
+ };
+ N();
+ static_assert(is_same<decltype(this), X*>);
+ };
+ return M;
+ };
+ return L;
+ }
+};
+
+int main() {
+ auto L = X{}.foo();
+ auto M = L();
+ M(3.14);
+}
+} // end ns6
+namespace ns7 {
+
+struct X {
+ double d;
+ X();
+ X(const X&);
+ X(X&) = delete;
+ auto foo() const {
+ //OK - the object used to initialize our capture is a const object and so prefers the non-deleted ctor.
+ const auto &&L = [*this] { };
+ }
+
+};
+int main() {
+ X x;
+ x.foo();
+}
+} // end ns7
+
} //end ns test_star_this
+
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -872,6 +872,92 @@
return false;
}
+static QualType adjustCVQualifiersForCXXThisWithinLambda(
+ ArrayRef<FunctionScopeInfo *> FunctionScopes, QualType ThisTy,
+ DeclContext *CurSemaContext, ASTContext &ASTCtx) {
+
+ QualType ClassType = ThisTy->getPointeeType();
+ LambdaScopeInfo *CurLSI = nullptr;
+ DeclContext *CurDC = CurSemaContext;
+
+ // Iterate through the stack of lambdas starting from the innermost lambda to
+ // the outermost lambda, checking if '*this' is ever captured by copy - since
+ // that could change the cv-qualifiers of the '*this' object.
+ // The object referred to by '*this' starts out with the cv-qualifiers of its
+ // member function. We then start with the innermost lambda and iterate
+ // outward checking to see if any lambda performs a by-copy capture of '*this'
+ // - and if so, any nested lambda must respect the 'constness' of that
+ // capturing lamdbda's call operator.
+ //
+
+ // The issue is that we cannot rely entirely on the FunctionScopeInfo stack
+ // since ScopeInfos are pushed on during parsing and treetransforming. But
+ // since a generic lambda's call operator can be instantiated anywhere (even
+ // 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 closure-class and query it for
+ // capture information. The reason we don't just resort to always using the
+ // DeclContext chain is that it is only mature for lambda expressions
+ // enclosing generic lambda's call operators that are being instantiated.
+
+ for (int I = FunctionScopes.size();
+ I-- && isa<LambdaScopeInfo>(FunctionScopes[I]);
+ CurDC = getLambdaAwareParentOfDeclContext(CurDC)) {
+ CurLSI = cast<LambdaScopeInfo>(FunctionScopes[I]);
+
+ if (!CurLSI->isCXXThisCaptured())
+ continue;
+
+ auto C = CurLSI->getCXXThisCapture();
+
+ if (C.isCopyCapture()) {
+ ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
+ if (CurLSI->CallOperator->isConst())
+ ClassType.addConst();
+ return ASTCtx.getPointerType(ClassType);
+ }
+ }
+ // We've run out of ScopeInfos but check if CurDC is a lambda (which can
+ // happen during instantiation of generic lambdas)
+ if (isLambdaCallOperator(CurDC)) {
+ assert(CurLSI);
+ assert(isGenericLambdaCallOperatorSpecialization(CurLSI->CallOperator));
+ assert(CurDC == getLambdaAwareParentOfDeclContext(CurLSI->CallOperator));
+
+ auto IsThisCaptured =
+ [](CXXRecordDecl *Closure, bool &IsByCopy, bool &IsConst) {
+ IsConst = false;
+ IsByCopy = false;
+ for (auto &&C : Closure->captures()) {
+ if (C.capturesThis()) {
+ if (C.getCaptureKind() == LCK_StarThis)
+ IsByCopy = true;
+ if (Closure->getLambdaCallOperator()->isConst())
+ IsConst = true;
+ return true;
+ }
+ }
+ return false;
+ };
+
+ bool IsByCopyCapture = false;
+ bool IsConstCapture = false;
+ CXXRecordDecl *Closure = cast<CXXRecordDecl>(CurDC->getParent());
+ while (Closure &&
+ IsThisCaptured(Closure, IsByCopyCapture, IsConstCapture)) {
+ if (IsByCopyCapture) {
+ ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
+ if (IsConstCapture)
+ ClassType.addConst();
+ return ASTCtx.getPointerType(ClassType);
+ }
+ Closure = isLambdaCallOperator(Closure->getParent())
+ ? cast<CXXRecordDecl>(Closure->getParent()->getParent())
+ : nullptr;
+ }
+ }
+ return ASTCtx.getPointerType(ClassType);
+}
+
QualType Sema::getCurrentThisType() {
DeclContext *DC = getFunctionLevelDeclContext();
QualType ThisTy = CXXThisTypeOverride;
@@ -902,20 +988,13 @@
ThisTy = Context.getPointerType(ClassTy);
}
}
- // Add const for '* this' capture if not mutable.
- if (isLambdaCallOperator(CurContext)) {
- LambdaScopeInfo *LSI = getCurLambda();
- assert(LSI);
- if (LSI->isCXXThisCaptured()) {
- auto C = LSI->getCXXThisCapture();
- QualType BaseType = ThisTy->getPointeeType();
- if ((C.isThisCapture() && C.isCopyCapture()) &&
- LSI->CallOperator->isConst() && !BaseType.isConstQualified()) {
- BaseType.addConst();
- ThisTy = Context.getPointerType(BaseType);
- }
- }
- }
+
+ // If we are within a lambda's call operator, the cv-qualifiers of 'this'
+ // might need to be adjusted if the lambda or any of its enclosing lambda's
+ // captures '*this' by copy.
+ if (!ThisTy.isNull() && isLambdaCallOperator(CurContext))
+ return adjustCVQualifiersForCXXThisWithinLambda(FunctionScopes, ThisTy,
+ CurContext, Context);
return ThisTy;
}
@@ -951,22 +1030,36 @@
static Expr *captureThis(Sema &S, ASTContext &Context, RecordDecl *RD,
QualType ThisTy, SourceLocation Loc,
const bool ByCopy) {
- QualType CaptureThisTy = ByCopy ? ThisTy->getPointeeType() : ThisTy;
- FieldDecl *Field
- = FieldDecl::Create(Context, RD, Loc, Loc, nullptr, CaptureThisTy,
- Context.getTrivialTypeSourceInfo(CaptureThisTy, Loc),
- nullptr, false, ICIS_NoInit);
+ QualType AdjustedThisTy = ThisTy;
+ // The type of the corresponding data member (not a 'this' pointer if 'by
+ // copy').
+ QualType CaptureThisFieldTy = ThisTy;
+ if (ByCopy) {
+ // If we are capturing the object referred to by '*this' by copy, ignore any
+ // cv qualifiers inherited from the type of the member function for the type
+ // of the closure-type's corresponding data member and any use of 'this'.
+ CaptureThisFieldTy = ThisTy->getPointeeType();
+ CaptureThisFieldTy.removeLocalCVRQualifiers(Qualifiers::CVRMask);
+ AdjustedThisTy = Context.getPointerType(CaptureThisFieldTy);
+ }
+
+ FieldDecl *Field = FieldDecl::Create(
+ Context, RD, Loc, Loc, nullptr, CaptureThisFieldTy,
+ Context.getTrivialTypeSourceInfo(CaptureThisFieldTy, Loc), nullptr, false,
+ ICIS_NoInit);
+
Field->setImplicit(true);
Field->setAccess(AS_private);
RD->addDecl(Field);
- Expr *This = new (Context) CXXThisExpr(Loc, ThisTy, /*isImplicit*/true);
+ Expr *This =
+ new (Context) CXXThisExpr(Loc, ThisTy, /*isImplicit*/ true);
if (ByCopy) {
Expr *StarThis = S.CreateBuiltinUnaryOp(Loc,
UO_Deref,
This).get();
InitializedEntity Entity = InitializedEntity::InitializeLambdaCapture(
- nullptr, CaptureThisTy, Loc);
+ nullptr, CaptureThisFieldTy, Loc);
InitializationKind InitKind = InitializationKind::CreateDirect(Loc, Loc, Loc);
InitializationSequence Init(S, Entity, InitKind, StarThis);
ExprResult ER = Init.Perform(S, Entity, InitKind, StarThis);
@@ -1065,12 +1158,12 @@
"*this) by copy");
// FIXME: We need to delay this marking in PotentiallyPotentiallyEvaluated
// contexts.
-
+ QualType ThisTy = getCurrentThisType();
for (unsigned idx = MaxFunctionScopesIndex; NumCapturingClosures;
--idx, --NumCapturingClosures) {
CapturingScopeInfo *CSI = cast<CapturingScopeInfo>(FunctionScopes[idx]);
Expr *ThisExpr = nullptr;
- QualType ThisTy = getCurrentThisType();
+
if (LambdaScopeInfo *LSI = dyn_cast<LambdaScopeInfo>(CSI)) {
// For lambda expressions, build a field and an initializing expression,
// and capture the *enclosing object* by copy only if this is the first
@@ -1085,7 +1178,7 @@
false/*ByCopy*/);
bool isNested = NumCapturingClosures > 1;
- CSI->addThisCapture(isNested, Loc, ThisTy, ThisExpr, ByCopy);
+ CSI->addThisCapture(isNested, Loc, ThisExpr, ByCopy);
}
return false;
}
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11110,7 +11110,7 @@
} else if (C.capturesThis()) {
LSI->addThisCapture(/*Nested*/ false, C.getLocation(),
- S.getCurrentThisType(), /*Expr*/ nullptr,
+ /*Expr*/ nullptr,
C.getCaptureKind() == LCK_StarThis);
} else {
LSI->addVLATypeCapture(C.getLocation(), I->getType());
Index: lib/Parse/ParseDeclCXX.cpp
===================================================================
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -3647,8 +3647,8 @@
return true;
case AttributeList::AT_WarnUnusedResult:
return !ScopeName && AttrName->getName().equals("nodiscard");
- case AttributeList::AT_Unused:
- return !ScopeName && AttrName->getName().equals("maybe_unused");
+ case AttributeList::AT_Unused:
+ return !ScopeName && AttrName->getName().equals("maybe_unused");
default:
return false;
}
Index: include/clang/Sema/ScopeInfo.h
===================================================================
--- include/clang/Sema/ScopeInfo.h
+++ include/clang/Sema/ScopeInfo.h
@@ -500,8 +500,11 @@
/// \brief Retrieve the capture type for this capture, which is effectively
/// the type of the non-static data member in the lambda/block structure
/// that would store this capture.
- QualType getCaptureType() const { return CaptureType; }
-
+ QualType getCaptureType() const {
+ assert(!isThisCapture());
+ return CaptureType;
+ }
+
Expr *getInitExpr() const {
assert(!isVLATypeCapture() && "no init expression for type capture");
return static_cast<Expr *>(InitExprAndCaptureKind.getPointer());
@@ -546,7 +549,10 @@
/*Cpy*/ nullptr));
}
- void addThisCapture(bool isNested, SourceLocation Loc, QualType CaptureType,
+ // Note, we do not need to add the type of 'this' since that is always
+ // retrievable from Sema::getCurrentThisType - and is also encoded within the
+ // type of the corresponding FieldDecl.
+ void addThisCapture(bool isNested, SourceLocation Loc,
Expr *Cpy, bool ByCopy);
/// \brief Determine whether the C++ 'this' is captured.
@@ -868,9 +874,9 @@
inline void
CapturingScopeInfo::addThisCapture(bool isNested, SourceLocation Loc,
- QualType CaptureType, Expr *Cpy,
+ Expr *Cpy,
const bool ByCopy) {
- Captures.push_back(Capture(Capture::ThisCapture, isNested, Loc, CaptureType,
+ Captures.push_back(Capture(Capture::ThisCapture, isNested, Loc, QualType(),
Cpy, ByCopy));
CXXThisCaptureIndex = Captures.size();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
