Skip to content

Commit 38824f2

Browse files
authored
[Clang] [Sema] Fix dependence of DREs in lambdas with an explicit object parameter (#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 #70604, #79754, #84163, #84425, #86054, #86398, and #86399.
1 parent a4cf479 commit 38824f2

File tree

14 files changed

+403
-17
lines changed

14 files changed

+403
-17
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,9 @@ Bug Fixes to C++ Support
513513
- Fix crash when inheriting from a cv-qualified type. Fixes:
514514
(`#35603 <https://github.com/llvm/llvm-project/issues/35603>`_)
515515
- Fix a crash when the using enum declaration uses an anonymous enumeration. Fixes (#GH86790).
516+
- Clang now correctly tracks type dependence of by-value captures in lambdas with an explicit
517+
object parameter.
518+
Fixes (#GH70604), (#GH79754), (#GH84163), (#GH84425), (#GH86054), (#GH86398), and (#GH86399).
516519

517520
Bug Fixes to AST Handling
518521
^^^^^^^^^^^^^^^^^^^^^^^^^

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
@@ -784,6 +784,11 @@ class alignas(void *) Stmt {
784784
LLVM_PREFERRED_TYPE(bool)
785785
unsigned IsImplicit : 1;
786786

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

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
@@ -2011,6 +2011,7 @@ void StmtProfiler::VisitMSPropertySubscriptExpr(
20112011
void StmtProfiler::VisitCXXThisExpr(const CXXThisExpr *S) {
20122012
VisitExpr(S);
20132013
ID.AddBoolean(S->isImplicit());
2014+
ID.AddBoolean(S->isCapturedByCopyInLambdaWithExplicitObjectParameter());
20142015
}
20152016

20162017
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
@@ -1194,8 +1194,11 @@ void TextNodeDumper::VisitDeclRefExpr(const DeclRefExpr *Node) {
11941194
case NOUR_Constant: OS << " non_odr_use_constant"; break;
11951195
case NOUR_Discarded: OS << " non_odr_use_discarded"; break;
11961196
}
1197-
if (Node->refersToEnclosingVariableOrCapture())
1197+
if (Node->isCapturedByCopyInLambdaWithExplicitObjectParameter())
1198+
OS << " dependent_capture";
1199+
else if (Node->refersToEnclosingVariableOrCapture())
11981200
OS << " refers_to_enclosing_variable_or_capture";
1201+
11991202
if (Node->isImmediateEscalating())
12001203
OS << " immediate-escalating";
12011204
}
@@ -1351,6 +1354,8 @@ void TextNodeDumper::VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *Node) {
13511354
void TextNodeDumper::VisitCXXThisExpr(const CXXThisExpr *Node) {
13521355
if (Node->isImplicit())
13531356
OS << " implicit";
1357+
if (Node->isCapturedByCopyInLambdaWithExplicitObjectParameter())
1358+
OS << " dependent_capture";
13541359
OS << " this";
13551360
}
13561361

clang/lib/Sema/SemaExpr.cpp

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20720,20 +20720,42 @@ void Sema::MarkVariableReferenced(SourceLocation Loc, VarDecl *Var) {
2072020720
static void FixDependencyOfIdExpressionsInLambdaWithDependentObjectParameter(
2072120721
Sema &SemaRef, ValueDecl *D, Expr *E) {
2072220722
auto *ID = dyn_cast<DeclRefExpr>(E);
20723-
if (!ID || ID->isTypeDependent())
20723+
if (!ID || ID->isTypeDependent() || !ID->refersToEnclosingVariableOrCapture())
2072420724
return;
2072520725

20726+
// If any enclosing lambda with a dependent explicit object parameter either
20727+
// explicitly captures the variable by value, or has a capture default of '='
20728+
// and does not capture the variable by reference, then the type of the DRE
20729+
// is dependent on the type of that lambda's explicit object parameter.
2072620730
auto IsDependent = [&]() {
20727-
const LambdaScopeInfo *LSI = SemaRef.getCurLambda();
20728-
if (!LSI)
20729-
return false;
20730-
if (!LSI->ExplicitObjectParameter ||
20731-
!LSI->ExplicitObjectParameter->getType()->isDependentType())
20732-
return false;
20733-
if (!LSI->CaptureMap.count(D))
20734-
return false;
20735-
const Capture &Cap = LSI->getCapture(D);
20736-
return !Cap.isCopyCapture();
20731+
for (auto *Scope : llvm::reverse(SemaRef.FunctionScopes)) {
20732+
auto *LSI = dyn_cast<sema::LambdaScopeInfo>(Scope);
20733+
if (!LSI)
20734+
continue;
20735+
20736+
if (LSI->Lambda && !LSI->Lambda->Encloses(SemaRef.CurContext) &&
20737+
LSI->AfterParameterList)
20738+
return false;
20739+
20740+
const auto *MD = LSI->CallOperator;
20741+
if (MD->getType().isNull())
20742+
continue;
20743+
20744+
const auto *Ty = cast<FunctionProtoType>(MD->getType());
20745+
if (!Ty || !MD->isExplicitObjectMemberFunction() ||
20746+
!Ty->getParamType(0)->isDependentType())
20747+
continue;
20748+
20749+
if (auto *C = LSI->CaptureMap.count(D) ? &LSI->getCapture(D) : nullptr) {
20750+
if (C->isCopyCapture())
20751+
return true;
20752+
continue;
20753+
}
20754+
20755+
if (LSI->ImpCaptureStyle == LambdaScopeInfo::ImpCap_LambdaByval)
20756+
return true;
20757+
}
20758+
return false;
2073720759
}();
2073820760

2073920761
ID->setCapturedByCopyInLambdaWithExplicitObjectParameter(

clang/lib/Sema/SemaExprCXX.cpp

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

14631463
void Sema::MarkThisReferenced(CXXThisExpr *This) {
14641464
CheckCXXThisCapture(This->getExprLoc());
1465+
if (This->isTypeDependent())
1466+
return;
1467+
1468+
// Check if 'this' is captured by value in a lambda with a dependent explicit
1469+
// object parameter, and mark it as type-dependent as well if so.
1470+
auto IsDependent = [&]() {
1471+
for (auto *Scope : llvm::reverse(FunctionScopes)) {
1472+
auto *LSI = dyn_cast<sema::LambdaScopeInfo>(Scope);
1473+
if (!LSI)
1474+
continue;
1475+
1476+
if (LSI->Lambda && !LSI->Lambda->Encloses(CurContext) &&
1477+
LSI->AfterParameterList)
1478+
return false;
1479+
1480+
// If this lambda captures 'this' by value, then 'this' is dependent iff
1481+
// this lambda has a dependent explicit object parameter. If we can't
1482+
// determine whether it does (e.g. because the CXXMethodDecl's type is
1483+
// null), assume it doesn't.
1484+
if (LSI->isCXXThisCaptured()) {
1485+
if (!LSI->getCXXThisCapture().isCopyCapture())
1486+
continue;
1487+
1488+
const auto *MD = LSI->CallOperator;
1489+
if (MD->getType().isNull())
1490+
return false;
1491+
1492+
const auto *Ty = cast<FunctionProtoType>(MD->getType());
1493+
return Ty && MD->isExplicitObjectMemberFunction() &&
1494+
Ty->getParamType(0)->isDependentType();
1495+
}
1496+
}
1497+
return false;
1498+
}();
1499+
1500+
This->setCapturedByCopyInLambdaWithExplicitObjectParameter(IsDependent);
14651501
}
14661502

14671503
bool Sema::isThisOutsideMemberFunctionBody(QualType BaseType) {

clang/lib/Sema/TreeTransform.h

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

1118011180
if (!getDerived().AlwaysRebuild() &&
11181-
QualifierLoc == E->getQualifierLoc() &&
11182-
ND == E->getDecl() &&
11181+
!E->isCapturedByCopyInLambdaWithExplicitObjectParameter() &&
11182+
QualifierLoc == E->getQualifierLoc() && ND == E->getDecl() &&
1118311183
Found == E->getFoundDecl() &&
1118411184
NameInfo.getName() == E->getDecl()->getDeclName() &&
1118511185
!E->hasExplicitTemplateArgs()) {
@@ -12642,9 +12642,17 @@ TreeTransform<Derived>::TransformCXXThisExpr(CXXThisExpr *E) {
1264212642
//
1264312643
// In other contexts, the type of `this` may be overrided
1264412644
// for type deduction, so we need to recompute it.
12645-
QualType T = getSema().getCurLambda() ?
12646-
getDerived().TransformType(E->getType())
12647-
: getSema().getCurrentThisType();
12645+
//
12646+
// Always recompute the type if we're in the body of a lambda, and
12647+
// 'this' is dependent on a lambda's explicit object parameter.
12648+
QualType T = [&]() {
12649+
auto &S = getSema();
12650+
if (E->isCapturedByCopyInLambdaWithExplicitObjectParameter())
12651+
return S.getCurrentThisType();
12652+
if (S.getCurLambda())
12653+
return getDerived().TransformType(E->getType());
12654+
return S.getCurrentThisType();
12655+
}();
1264812656

1264912657
if (!getDerived().AlwaysRebuild() && T == E->getType()) {
1265012658
// 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
@@ -1841,6 +1841,7 @@ void ASTStmtReader::VisitCXXThisExpr(CXXThisExpr *E) {
18411841
VisitExpr(E);
18421842
E->setLocation(readSourceLocation());
18431843
E->setImplicit(Record.readInt());
1844+
E->setCapturedByCopyInLambdaWithExplicitObjectParameter(Record.readInt());
18441845
}
18451846

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

clang/lib/Serialization/ASTWriterStmt.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1839,6 +1839,7 @@ void ASTStmtWriter::VisitCXXThisExpr(CXXThisExpr *E) {
18391839
VisitExpr(E);
18401840
Record.AddSourceLocation(E->getLocation());
18411841
Record.push_back(E->isImplicit());
1842+
Record.push_back(E->isCapturedByCopyInLambdaWithExplicitObjectParameter());
18421843

18431844
Code = serialization::EXPR_CXX_THIS;
18441845
}

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)