r313487 - Fix the second half of PR34266: Don't implicitly capture '*this' if the members are found in a class unrelated to the enclosing class.
Faisal Vali via cfe-commits
cfe-commits at lists.llvm.org
Sun Sep 17 08:37:51 PDT 2017
Author: faisalv
Date: Sun Sep 17 08:37:51 2017
New Revision: 313487
URL: http://llvm.org/viewvc/llvm-project?rev=313487&view=rev
Log:
Fix the second half of PR34266: Don't implicitly capture '*this' if the members are found in a class unrelated to the enclosing class.
https://bugs.llvm.org/show_bug.cgi?id=34266
For e.g.
struct A {
void f(int);
static void f(char);
};
struct B {
auto foo() {
return [&] (auto a) {
A::f(a); // this should not cause a capture of '*this'
};
}
};
The patch does the following:
1) It moves the check to attempt an implicit capture of '*this' by reference into the more logical location of when the call is actually built within ActOnCallExpr (as opposed to when the unresolved-member-lookup node is created).
- Reminder: A capture of '*this' by value has to always be an explicit capture.
2) It additionally checks whether the naming class of the UnresolvedMemberExpr ('A' in the example above) is related to the enclosing class ('B' above).
P.S. If you have access to ISO-C++'s CWG reflector, see this thread for some potentially related discussion: http://lists.isocpp.org/core/2017/08/2851.php
Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprMember.cpp
cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=313487&r1=313486&r2=313487&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Sun Sep 17 08:37:51 2017
@@ -5125,6 +5125,87 @@ static void checkDirectCallValidity(Sema
}
}
+static bool enclosingClassIsRelatedToClassInWhichMembersWereFound(
+ const UnresolvedMemberExpr *const UME, Sema &S) {
+
+ const auto GetFunctionLevelDCIfCXXClass =
+ [](Sema &S) -> const CXXRecordDecl * {
+ const DeclContext *const DC = S.getFunctionLevelDeclContext();
+ if (!DC || !DC->getParent())
+ return nullptr;
+
+ // If the call to some member function was made from within a member
+ // function body 'M' return return 'M's parent.
+ if (const auto *MD = dyn_cast<CXXMethodDecl>(DC))
+ return MD->getParent()->getCanonicalDecl();
+ // else the call was made from within a default member initializer of a
+ // class, so return the class.
+ if (const auto *RD = dyn_cast<CXXRecordDecl>(DC))
+ return RD->getCanonicalDecl();
+ return nullptr;
+ };
+ // If our DeclContext is neither a member function nor a class (in the
+ // case of a lambda in a default member initializer), we can't have an
+ // enclosing 'this'.
+
+ const CXXRecordDecl *const CurParentClass = GetFunctionLevelDCIfCXXClass(S);
+ if (!CurParentClass)
+ return false;
+
+ // The naming class for implicit member functions call is the class in which
+ // name lookup starts.
+ const CXXRecordDecl *const NamingClass =
+ UME->getNamingClass()->getCanonicalDecl();
+ assert(NamingClass && "Must have naming class even for implicit access");
+
+ // If the unresolved member functions were found in a 'naming class' that is
+ // related (either the same or derived from) to the class that contains the
+ // member function that itself contained the implicit member access.
+
+ return CurParentClass == NamingClass ||
+ CurParentClass->isDerivedFrom(NamingClass);
+}
+
+static void
+tryImplicitlyCaptureThisIfImplicitMemberFunctionAccessWithDependentArgs(
+ Sema &S, const UnresolvedMemberExpr *const UME, SourceLocation CallLoc) {
+
+ if (!UME)
+ return;
+
+ LambdaScopeInfo *const CurLSI = S.getCurLambda();
+ // Only try and implicitly capture 'this' within a C++ Lambda if it hasn't
+ // already been captured, or if this is an implicit member function call (if
+ // it isn't, an attempt to capture 'this' should already have been made).
+ if (!CurLSI || CurLSI->ImpCaptureStyle == CurLSI->ImpCap_None ||
+ !UME->isImplicitAccess() || CurLSI->isCXXThisCaptured())
+ return;
+
+ // Check if the naming class in which the unresolved members were found is
+ // related (same as or is a base of) to the enclosing class.
+
+ if (!enclosingClassIsRelatedToClassInWhichMembersWereFound(UME, S))
+ return;
+
+
+ DeclContext *EnclosingFunctionCtx = S.CurContext->getParent()->getParent();
+ // If the enclosing function is not dependent, then this lambda is
+ // capture ready, so if we can capture this, do so.
+ if (!EnclosingFunctionCtx->isDependentContext()) {
+ // If the current lambda and all enclosing lambdas can capture 'this' -
+ // then go ahead and capture 'this' (since our unresolved overload set
+ // contains at least one non-static member function).
+ if (!S.CheckCXXThisCapture(CallLoc, /*Explcit*/ false, /*Diagnose*/ false))
+ S.CheckCXXThisCapture(CallLoc);
+ } else if (S.CurContext->isDependentContext()) {
+ // ... since this is an implicit member reference, that might potentially
+ // involve a 'this' capture, mark 'this' for potential capture in
+ // enclosing lambdas.
+ if (CurLSI->ImpCaptureStyle != CurLSI->ImpCap_None)
+ CurLSI->addPotentialThisCapture(CallLoc);
+ }
+}
+
/// ActOnCallExpr - Handle a call to Fn with the specified array of arguments.
/// This provides the location of the left/right parens and a list of comma
/// locations.
@@ -5173,6 +5254,11 @@ ExprResult Sema::ActOnCallExpr(Scope *Sc
Context, Fn, cast<CallExpr>(ExecConfig), ArgExprs,
Context.DependentTy, VK_RValue, RParenLoc);
} else {
+
+ tryImplicitlyCaptureThisIfImplicitMemberFunctionAccessWithDependentArgs(
+ *this, dyn_cast<UnresolvedMemberExpr>(Fn->IgnoreParens()),
+ Fn->getLocStart());
+
return new (Context) CallExpr(
Context, Fn, ArgExprs, Context.DependentTy, VK_RValue, RParenLoc);
}
Modified: cfe/trunk/lib/Sema/SemaExprMember.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=313487&r1=313486&r2=313487&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprMember.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprMember.cpp Sun Sep 17 08:37:51 2017
@@ -1001,53 +1001,7 @@ Sema::BuildMemberReferenceExpr(Expr *Bas
BaseExpr = Converted.get();
}
- LambdaScopeInfo *const CurLSI = getCurLambda();
- // If this is an implicit member reference and the overloaded
- // name refers to both static and non-static member functions
- // (i.e. BaseExpr is null) and if we are currently processing a lambda,
- // check if we should/can capture 'this'...
- // Keep this example in mind:
- // struct X {
- // void f(int) { }
- // static void f(double) { }
- //
- // int g() {
- // auto L = [=](auto a) {
- // return [](int i) {
- // return [=](auto b) {
- // f(b);
- // //f(decltype(a){});
- // };
- // };
- // };
- // auto M = L(0.0);
- // auto N = M(3);
- // N(5.32); // OK, must not error.
- // return 0;
- // }
- // };
- //
- if (!BaseExpr && CurLSI) {
- SourceLocation Loc = R.getNameLoc();
- if (SS.getRange().isValid())
- Loc = SS.getRange().getBegin();
- DeclContext *EnclosingFunctionCtx = CurContext->getParent()->getParent();
- // If the enclosing function is not dependent, then this lambda is
- // capture ready, so if we can capture this, do so.
- if (!EnclosingFunctionCtx->isDependentContext()) {
- // If the current lambda and all enclosing lambdas can capture 'this' -
- // then go ahead and capture 'this' (since our unresolved overload set
- // contains both static and non-static member functions).
- if (!CheckCXXThisCapture(Loc, /*Explcit*/false, /*Diagnose*/false))
- CheckCXXThisCapture(Loc);
- } else if (CurContext->isDependentContext()) {
- // ... since this is an implicit member reference, that might potentially
- // involve a 'this' capture, mark 'this' for potential capture in
- // enclosing lambdas.
- if (CurLSI->ImpCaptureStyle != CurLSI->ImpCap_None)
- CurLSI->addPotentialThisCapture(Loc);
- }
- }
+
const DeclarationNameInfo &MemberNameInfo = R.getLookupNameInfo();
DeclarationName MemberName = MemberNameInfo.getName();
SourceLocation MemberLoc = MemberNameInfo.getLoc();
Modified: cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp?rev=313487&r1=313486&r2=313487&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp Sun Sep 17 08:37:51 2017
@@ -1380,3 +1380,145 @@ XT<int> xt{};
void PR33318(int i) {
[&](auto) { static_assert(&i != nullptr, ""); }(0); // expected-warning 2{{always true}} expected-note {{instantiation}}
}
+
+// Check to make sure that we don't capture when member-calls are made to members that are not of 'this' class.
+namespace PR34266 {
+// https://bugs.llvm.org/show_bug.cgi?id=34266
+namespace ns1 {
+struct A {
+ static void bar(int) { }
+ static void bar(double) { }
+};
+
+struct B
+{
+ template<class T>
+ auto f() {
+ auto L = [=] {
+ T{}.bar(3.0);
+ T::bar(3);
+
+ };
+ ASSERT_NO_CAPTURES(L);
+ return L;
+ };
+};
+
+void test() {
+ B{}.f<A>();
+}
+} // end ns1
+
+namespace ns2 {
+struct A {
+ static void bar(int) { }
+ static void bar(double) { }
+};
+
+struct B
+{
+ using T = A;
+ auto f() {
+ auto L = [=](auto a) {
+ T{}.bar(a);
+ T::bar(a);
+
+ };
+ ASSERT_NO_CAPTURES(L);
+ return L;
+ };
+};
+
+void test() {
+ B{}.f()(3.0);
+ B{}.f()(3);
+}
+} // end ns2
+
+namespace ns3 {
+struct A {
+ void bar(int) { }
+ static void bar(double) { }
+};
+
+struct B
+{
+ using T = A;
+ auto f() {
+ auto L = [=](auto a) {
+ T{}.bar(a);
+ T::bar(a); // This call ignores the instance member function because the implicit object argument fails to convert.
+
+ };
+ ASSERT_NO_CAPTURES(L);
+ return L;
+ };
+};
+
+void test() {
+ B{}.f()(3.0);
+ B{}.f()(3);
+}
+
+} // end ns3
+
+
+namespace ns4 {
+struct A {
+ void bar(int) { }
+ static void bar(double) { }
+};
+
+struct B : A
+{
+ using T = A;
+ auto f() {
+ auto L = [=](auto a) {
+ T{}.bar(a);
+ T::bar(a);
+
+ };
+ // just check to see if the size if >= 2 bytes (which should be the case if we capture anything)
+ ASSERT_CLOSURE_SIZE(L, 2);
+ return L;
+ };
+};
+
+void test() {
+ B{}.f()(3.0);
+ B{}.f()(3);
+}
+
+} // end ns4
+
+namespace ns5 {
+struct A {
+ void bar(int) { }
+ static void bar(double) { }
+};
+
+struct B
+{
+ template<class T>
+ auto f() {
+ auto L = [&](auto a) {
+ T{}.bar(a);
+ T::bar(a);
+
+ };
+
+ ASSERT_NO_CAPTURES(L);
+ return L;
+ };
+};
+
+void test() {
+ B{}.f<A>()(3.0);
+ B{}.f<A>()(3);
+}
+
+} // end ns5
+
+
+
+} // end PR34266
More information about the cfe-commits
mailing list