Skip to content

Commit 5b78d71

Browse files
Sirraideerikolofsson
authored andcommitted
[Clang] [Sema] Fix dependence of DREs in lambdas with an explicit object parameter (llvm#84473)
This fixes some problems wrt dependence of captures in lambdas with an explicit object parameter. [temp.dep.expr] states that > An id-expression is type-dependent if [...] its terminal name is > - associated by name lookup with an entity captured by copy > ([expr.prim.lambda.capture]) in a lambda-expression that has > an explicit object parameter whose type is dependent [dcl.fct]. There were several issues with our implementation of this: 1. we were treating by-reference captures as dependent rather than by-value captures; 2. tree transform wasn't checking whether referring to such a by-value capture should make a DRE dependent; 3. when checking whether a DRE refers to such a by-value capture, we were only looking at the immediately enclosing lambda, and not at any parent lambdas; 4. we also forgot to check for implicit by-value captures; 5. lastly, we were attempting to determine whether a lambda has an explicit object parameter by checking the `LambdaScopeInfo`'s `ExplicitObjectParameter`, but it seems that that simply wasn't set (yet) by the time we got to the check. All of these should be fixed now. This fixes llvm#70604, llvm#79754, llvm#84163, llvm#84425, llvm#86054, llvm#86398, and llvm#86399.
1 parent 3b5b5c1 commit 5b78d71

File tree

14 files changed

+414
-17
lines changed

14 files changed

+414
-17
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,6 +1119,10 @@ Bug Fixes to C++ Support
11191119
incorrect constraint substitution.
11201120
(`#86769 <https://github.com/llvm/llvm-project/issues/86769>`_)
11211121

1122+
- Clang now correctly tracks type dependence of by-value captures in lambdas with an explicit
1123+
object parameter.
1124+
Fixes (#GH70604), (#GH79754), (#GH84163), (#GH84425), (#GH86054), (#GH86398), and (#GH86399).
1125+
11221126
Bug Fixes to AST Handling
11231127
^^^^^^^^^^^^^^^^^^^^^^^^^
11241128
- Fixed an import failure of recursive friend class template.

clang/include/clang/AST/ExprCXX.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,6 +1149,7 @@ class CXXThisExpr : public Expr {
11491149
CXXThisExpr(SourceLocation L, QualType Ty, bool IsImplicit, ExprValueKind VK)
11501150
: Expr(CXXThisExprClass, Ty, VK, OK_Ordinary) {
11511151
CXXThisExprBits.IsImplicit = IsImplicit;
1152+
CXXThisExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter = false;
11521153
CXXThisExprBits.Loc = L;
11531154
setDependence(computeDependence(this));
11541155
}
@@ -1170,6 +1171,15 @@ class CXXThisExpr : public Expr {
11701171
bool isImplicit() const { return CXXThisExprBits.IsImplicit; }
11711172
void setImplicit(bool I) { CXXThisExprBits.IsImplicit = I; }
11721173

1174+
bool isCapturedByCopyInLambdaWithExplicitObjectParameter() const {
1175+
return CXXThisExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter;
1176+
}
1177+
1178+
void setCapturedByCopyInLambdaWithExplicitObjectParameter(bool Set) {
1179+
CXXThisExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter = Set;
1180+
setDependence(computeDependence(this));
1181+
}
1182+
11731183
static bool classof(const Stmt *T) {
11741184
return T->getStmtClass() == CXXThisExprClass;
11751185
}

clang/include/clang/AST/Stmt.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,11 @@ class alignas(void *) Stmt {
782782
LLVM_PREFERRED_TYPE(bool)
783783
unsigned IsImplicit : 1;
784784

785+
/// Whether there is a lambda with an explicit object parameter that
786+
/// captures this "this" by copy.
787+
LLVM_PREFERRED_TYPE(bool)
788+
unsigned CapturedByCopyInLambdaWithExplicitObjectParameter : 1;
789+
785790
/// The location of the "this".
786791
SourceLocation Loc;
787792
};

clang/lib/AST/ComputeDependence.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,16 @@ ExprDependence clang::computeDependence(CXXThisExpr *E) {
310310
// 'this' is type-dependent if the class type of the enclosing
311311
// member function is dependent (C++ [temp.dep.expr]p2)
312312
auto D = toExprDependenceForImpliedType(E->getType()->getDependence());
313+
314+
// If a lambda with an explicit object parameter captures '*this', then
315+
// 'this' now refers to the captured copy of lambda, and if the lambda
316+
// is type-dependent, so is the object and thus 'this'.
317+
//
318+
// Note: The standard does not mention this case explicitly, but we need
319+
// to do this so we can mark NSDM accesses as dependent.
320+
if (E->isCapturedByCopyInLambdaWithExplicitObjectParameter())
321+
D |= ExprDependence::Type;
322+
313323
assert(!(D & ExprDependence::UnexpandedPack));
314324
return D;
315325
}

clang/lib/AST/StmtProfile.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2009,6 +2009,7 @@ void StmtProfiler::VisitMSPropertySubscriptExpr(
20092009
void StmtProfiler::VisitCXXThisExpr(const CXXThisExpr *S) {
20102010
VisitExpr(S);
20112011
ID.AddBoolean(S->isImplicit());
2012+
ID.AddBoolean(S->isCapturedByCopyInLambdaWithExplicitObjectParameter());
20122013
}
20132014

20142015
void StmtProfiler::VisitCXXThrowExpr(const CXXThrowExpr *S) {

clang/lib/AST/TextNodeDumper.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1158,8 +1158,11 @@ void TextNodeDumper::VisitDeclRefExpr(const DeclRefExpr *Node) {
11581158
case NOUR_Constant: OS << " non_odr_use_constant"; break;
11591159
case NOUR_Discarded: OS << " non_odr_use_discarded"; break;
11601160
}
1161-
if (Node->refersToEnclosingVariableOrCapture())
1161+
if (Node->isCapturedByCopyInLambdaWithExplicitObjectParameter())
1162+
OS << " dependent_capture";
1163+
else if (Node->refersToEnclosingVariableOrCapture())
11621164
OS << " refers_to_enclosing_variable_or_capture";
1165+
11631166
if (Node->isImmediateEscalating())
11641167
OS << " immediate-escalating";
11651168
}
@@ -1315,6 +1318,8 @@ void TextNodeDumper::VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *Node) {
13151318
void TextNodeDumper::VisitCXXThisExpr(const CXXThisExpr *Node) {
13161319
if (Node->isImplicit())
13171320
OS << " implicit";
1321+
if (Node->isCapturedByCopyInLambdaWithExplicitObjectParameter())
1322+
OS << " dependent_capture";
13181323
OS << " this";
13191324
}
13201325

clang/lib/Sema/SemaExpr.cpp

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20668,20 +20668,42 @@ void Sema::MarkVariableReferenced(SourceLocation Loc, VarDecl *Var) {
2066820668
static void FixDependencyOfIdExpressionsInLambdaWithDependentObjectParameter(
2066920669
Sema &SemaRef, ValueDecl *D, Expr *E) {
2067020670
auto *ID = dyn_cast<DeclRefExpr>(E);
20671-
if (!ID || ID->isTypeDependent())
20671+
if (!ID || ID->isTypeDependent() || !ID->refersToEnclosingVariableOrCapture())
2067220672
return;
2067320673

20674+
// If any enclosing lambda with a dependent explicit object parameter either
20675+
// explicitly captures the variable by value, or has a capture default of '='
20676+
// and does not capture the variable by reference, then the type of the DRE
20677+
// is dependent on the type of that lambda's explicit object parameter.
2067420678
auto IsDependent = [&]() {
20675-
const LambdaScopeInfo *LSI = SemaRef.getCurLambda();
20676-
if (!LSI)
20677-
return false;
20678-
if (!LSI->ExplicitObjectParameter ||
20679-
!LSI->ExplicitObjectParameter->getType()->isDependentType())
20680-
return false;
20681-
if (!LSI->CaptureMap.count(D))
20682-
return false;
20683-
const Capture &Cap = LSI->getCapture(D);
20684-
return !Cap.isCopyCapture();
20679+
for (auto *Scope : llvm::reverse(SemaRef.FunctionScopes)) {
20680+
auto *LSI = dyn_cast<sema::LambdaScopeInfo>(Scope);
20681+
if (!LSI)
20682+
continue;
20683+
20684+
if (LSI->Lambda && !LSI->Lambda->Encloses(SemaRef.CurContext) &&
20685+
LSI->AfterParameterList)
20686+
return false;
20687+
20688+
const auto *MD = LSI->CallOperator;
20689+
if (MD->getType().isNull())
20690+
continue;
20691+
20692+
const auto *Ty = cast<FunctionProtoType>(MD->getType());
20693+
if (!Ty || !MD->isExplicitObjectMemberFunction() ||
20694+
!Ty->getParamType(0)->isDependentType())
20695+
continue;
20696+
20697+
if (auto *C = LSI->CaptureMap.count(D) ? &LSI->getCapture(D) : nullptr) {
20698+
if (C->isCopyCapture())
20699+
return true;
20700+
continue;
20701+
}
20702+
20703+
if (LSI->ImpCaptureStyle == LambdaScopeInfo::ImpCap_LambdaByval)
20704+
return true;
20705+
}
20706+
return false;
2068520707
}();
2068620708

2068720709
ID->setCapturedByCopyInLambdaWithExplicitObjectParameter(

clang/lib/Sema/SemaExprCXX.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1440,6 +1440,42 @@ Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type,
14401440

14411441
void Sema::MarkThisReferenced(CXXThisExpr *This) {
14421442
CheckCXXThisCapture(This->getExprLoc());
1443+
if (This->isTypeDependent())
1444+
return;
1445+
1446+
// Check if 'this' is captured by value in a lambda with a dependent explicit
1447+
// object parameter, and mark it as type-dependent as well if so.
1448+
auto IsDependent = [&]() {
1449+
for (auto *Scope : llvm::reverse(FunctionScopes)) {
1450+
auto *LSI = dyn_cast<sema::LambdaScopeInfo>(Scope);
1451+
if (!LSI)
1452+
continue;
1453+
1454+
if (LSI->Lambda && !LSI->Lambda->Encloses(CurContext) &&
1455+
LSI->AfterParameterList)
1456+
return false;
1457+
1458+
// If this lambda captures 'this' by value, then 'this' is dependent iff
1459+
// this lambda has a dependent explicit object parameter. If we can't
1460+
// determine whether it does (e.g. because the CXXMethodDecl's type is
1461+
// null), assume it doesn't.
1462+
if (LSI->isCXXThisCaptured()) {
1463+
if (!LSI->getCXXThisCapture().isCopyCapture())
1464+
continue;
1465+
1466+
const auto *MD = LSI->CallOperator;
1467+
if (MD->getType().isNull())
1468+
return false;
1469+
1470+
const auto *Ty = cast<FunctionProtoType>(MD->getType());
1471+
return Ty && MD->isExplicitObjectMemberFunction() &&
1472+
Ty->getParamType(0)->isDependentType();
1473+
}
1474+
}
1475+
return false;
1476+
}();
1477+
1478+
This->setCapturedByCopyInLambdaWithExplicitObjectParameter(IsDependent);
14431479
}
14441480

14451481
bool Sema::isThisOutsideMemberFunctionBody(QualType BaseType) {

clang/lib/Sema/TreeTransform.h

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10928,8 +10928,8 @@ TreeTransform<Derived>::TransformDeclRefExpr(DeclRefExpr *E) {
1092810928
}
1092910929

1093010930
if (!getDerived().AlwaysRebuild() &&
10931-
QualifierLoc == E->getQualifierLoc() &&
10932-
ND == E->getDecl() &&
10931+
!E->isCapturedByCopyInLambdaWithExplicitObjectParameter() &&
10932+
QualifierLoc == E->getQualifierLoc() && ND == E->getDecl() &&
1093310933
Found == E->getFoundDecl() &&
1093410934
NameInfo.getName() == E->getDecl()->getDeclName() &&
1093510935
!E->hasExplicitTemplateArgs()) {
@@ -12392,9 +12392,17 @@ TreeTransform<Derived>::TransformCXXThisExpr(CXXThisExpr *E) {
1239212392
//
1239312393
// In other contexts, the type of `this` may be overrided
1239412394
// for type deduction, so we need to recompute it.
12395-
QualType T = getSema().getCurLambda() ?
12396-
getDerived().TransformType(E->getType())
12397-
: getSema().getCurrentThisType();
12395+
//
12396+
// Always recompute the type if we're in the body of a lambda, and
12397+
// 'this' is dependent on a lambda's explicit object parameter.
12398+
QualType T = [&]() {
12399+
auto &S = getSema();
12400+
if (E->isCapturedByCopyInLambdaWithExplicitObjectParameter())
12401+
return S.getCurrentThisType();
12402+
if (S.getCurLambda())
12403+
return getDerived().TransformType(E->getType());
12404+
return S.getCurrentThisType();
12405+
}();
1239812406

1239912407
if (!getDerived().AlwaysRebuild() && T == E->getType()) {
1240012408
// Mark it referenced in the new context regardless.

clang/lib/Serialization/ASTReaderStmt.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1849,6 +1849,7 @@ void ASTStmtReader::VisitCXXThisExpr(CXXThisExpr *E) {
18491849
VisitExpr(E);
18501850
E->setLocation(readSourceLocation());
18511851
E->setImplicit(Record.readInt());
1852+
E->setCapturedByCopyInLambdaWithExplicitObjectParameter(Record.readInt());
18521853
}
18531854

18541855
void ASTStmtReader::VisitCXXThrowExpr(CXXThrowExpr *E) {

clang/lib/Serialization/ASTWriterStmt.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1842,6 +1842,7 @@ void ASTStmtWriter::VisitCXXThisExpr(CXXThisExpr *E) {
18421842
VisitExpr(E);
18431843
Record.AddSourceLocation(E->getLocation());
18441844
Record.push_back(E->isImplicit());
1845+
Record.push_back(E->isCapturedByCopyInLambdaWithExplicitObjectParameter());
18451846

18461847
Code = serialization::EXPR_CXX_THIS;
18471848
}

clang/test/CodeGenCXX/cxx2b-deducing-this.cpp

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,3 +109,76 @@ void test_temporary() {
109109
//CHECK: %ref.tmp = alloca %struct.MaterializedTemporary, align 1
110110
//CHECK: call void @_ZN21MaterializedTemporaryC1Ev(ptr noundef nonnull align 1 dereferenceable(1) %ref.tmp){{.*}}
111111
//CHECK invoke void @_ZNH21MaterializedTemporary3fooEOS_(ptr noundef nonnull align 1 dereferenceable(1) %ref.tmp){{.*}}
112+
113+
namespace GH86399 {
114+
volatile int a = 0;
115+
struct function {
116+
function& operator=(function const&) {
117+
a = 1;
118+
return *this;
119+
}
120+
};
121+
122+
void f() {
123+
function list;
124+
125+
//CHECK-LABEL: define internal void @"_ZZN7GH863991f{{.*}}"(ptr %{{.*}})
126+
//CHECK: call {{.*}} @_ZN7GH863998functionaSERKS0_
127+
//CHECK-NEXT: ret void
128+
[&list](this auto self) {
129+
list = function{};
130+
}();
131+
}
132+
}
133+
134+
namespace GH84163 {
135+
// Just check that this doesn't crash (we were previously not instantiating
136+
// everything that needs instantiating in here).
137+
template <typename> struct S {};
138+
139+
void a() {
140+
int x;
141+
const auto l = [&x](this auto&) { S<decltype(x)> q; };
142+
l();
143+
}
144+
}
145+
146+
namespace GH84425 {
147+
// As above.
148+
void do_thing(int x) {
149+
auto second = [&](this auto const& self, int b) -> int {
150+
if (x) return x;
151+
else return self(x);
152+
};
153+
154+
second(1);
155+
}
156+
157+
void do_thing2(int x) {
158+
auto second = [&](this auto const& self) {
159+
if (true) return x;
160+
else return x;
161+
};
162+
163+
second();
164+
}
165+
}
166+
167+
namespace GH79754 {
168+
// As above.
169+
void f() {
170+
int x;
171+
[&x](this auto&&) {return x;}();
172+
}
173+
}
174+
175+
namespace GH70604 {
176+
auto dothing(int num)
177+
{
178+
auto fun = [&num](this auto&& self) -> void {
179+
auto copy = num;
180+
};
181+
182+
fun();
183+
}
184+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// RUN: %clang_cc1 -emit-pch -std=c++23 -o %t %s
2+
// RUN: %clang_cc1 -include-pch %t -verify -fsyntax-only -DTEST -std=c++23 %s
3+
4+
// Test that dependence of 'this' and DREs due to by-value capture by a
5+
// lambda with an explicit object parameter is serialised/deserialised
6+
// properly.
7+
8+
#ifndef HEADER
9+
#define HEADER
10+
struct S {
11+
int x;
12+
auto f() {
13+
return [*this] (this auto&&) {
14+
int y;
15+
x = 42;
16+
17+
const auto l = [y] (this auto&&) { y = 42; };
18+
l();
19+
};
20+
}
21+
};
22+
#endif
23+
24+
// expected-error@* {{read-only variable is not assignable}}
25+
// expected-error@* {{cannot assign to a variable captured by copy in a non-mutable lambda}}
26+
// expected-note@* 2 {{in instantiation of}}
27+
28+
#ifdef TEST
29+
void f() {
30+
const auto l = S{}.f();
31+
l(); // expected-note {{in instantiation of}}
32+
}
33+
#endif
34+
35+

0 commit comments

Comments
 (0)