Skip to content

[Clang] [Sema] Fix dependence of DREs in lambdas with an explicit object parameter #84473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Apr 9, 2024
Merged
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,9 @@ Bug Fixes to C++ Support
- Fix crash when inheriting from a cv-qualified type. Fixes:
(`#35603 <https://github.com/llvm/llvm-project/issues/35603>`_)
- Fix a crash when the using enum declaration uses an anonymous enumeration. Fixes (#GH86790).
- Clang now correctly tracks type dependence of by-value captures in lambdas with an explicit
object parameter.
Fixes (#GH70604), (#GH79754), (#GH84163), (#GH84425), (#GH86054), (#GH86398), and (#GH86399).

Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
10 changes: 10 additions & 0 deletions clang/include/clang/AST/ExprCXX.h
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,7 @@ class CXXThisExpr : public Expr {
CXXThisExpr(SourceLocation L, QualType Ty, bool IsImplicit, ExprValueKind VK)
: Expr(CXXThisExprClass, Ty, VK, OK_Ordinary) {
CXXThisExprBits.IsImplicit = IsImplicit;
CXXThisExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter = false;
CXXThisExprBits.Loc = L;
setDependence(computeDependence(this));
}
Expand All @@ -1170,6 +1171,15 @@ class CXXThisExpr : public Expr {
bool isImplicit() const { return CXXThisExprBits.IsImplicit; }
void setImplicit(bool I) { CXXThisExprBits.IsImplicit = I; }

bool isCapturedByCopyInLambdaWithExplicitObjectParameter() const {
return CXXThisExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter;
}

void setCapturedByCopyInLambdaWithExplicitObjectParameter(bool Set) {
CXXThisExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter = Set;
setDependence(computeDependence(this));
}

static bool classof(const Stmt *T) {
return T->getStmtClass() == CXXThisExprClass;
}
Expand Down
5 changes: 5 additions & 0 deletions clang/include/clang/AST/Stmt.h
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,11 @@ class alignas(void *) Stmt {
LLVM_PREFERRED_TYPE(bool)
unsigned IsImplicit : 1;

/// Whether there is a lambda with an explicit object parameter that
/// captures this "this" by copy.
LLVM_PREFERRED_TYPE(bool)
unsigned CapturedByCopyInLambdaWithExplicitObjectParameter : 1;

/// The location of the "this".
SourceLocation Loc;
};
Expand Down
10 changes: 10 additions & 0 deletions clang/lib/AST/ComputeDependence.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,16 @@ ExprDependence clang::computeDependence(CXXThisExpr *E) {
// 'this' is type-dependent if the class type of the enclosing
// member function is dependent (C++ [temp.dep.expr]p2)
auto D = toExprDependenceForImpliedType(E->getType()->getDependence());

// If a lambda with an explicit object parameter captures '*this', then
// 'this' now refers to the captured copy of lambda, and if the lambda
// is type-dependent, so is the object and thus 'this'.
//
// Note: The standard does not mention this case explicitly, but we need
// to do this so we can mark NSDM accesses as dependent.
if (E->isCapturedByCopyInLambdaWithExplicitObjectParameter())
D |= ExprDependence::Type;

assert(!(D & ExprDependence::UnexpandedPack));
return D;
}
Expand Down
1 change: 1 addition & 0 deletions clang/lib/AST/StmtProfile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2011,6 +2011,7 @@ void StmtProfiler::VisitMSPropertySubscriptExpr(
void StmtProfiler::VisitCXXThisExpr(const CXXThisExpr *S) {
VisitExpr(S);
ID.AddBoolean(S->isImplicit());
ID.AddBoolean(S->isCapturedByCopyInLambdaWithExplicitObjectParameter());
}

void StmtProfiler::VisitCXXThrowExpr(const CXXThrowExpr *S) {
Expand Down
7 changes: 6 additions & 1 deletion clang/lib/AST/TextNodeDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1194,8 +1194,11 @@ void TextNodeDumper::VisitDeclRefExpr(const DeclRefExpr *Node) {
case NOUR_Constant: OS << " non_odr_use_constant"; break;
case NOUR_Discarded: OS << " non_odr_use_discarded"; break;
}
if (Node->refersToEnclosingVariableOrCapture())
if (Node->isCapturedByCopyInLambdaWithExplicitObjectParameter())
OS << " dependent_capture";
else if (Node->refersToEnclosingVariableOrCapture())
OS << " refers_to_enclosing_variable_or_capture";

if (Node->isImmediateEscalating())
OS << " immediate-escalating";
}
Expand Down Expand Up @@ -1351,6 +1354,8 @@ void TextNodeDumper::VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *Node) {
void TextNodeDumper::VisitCXXThisExpr(const CXXThisExpr *Node) {
if (Node->isImplicit())
OS << " implicit";
if (Node->isCapturedByCopyInLambdaWithExplicitObjectParameter())
OS << " dependent_capture";
OS << " this";
}

Expand Down
44 changes: 33 additions & 11 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20720,20 +20720,42 @@ void Sema::MarkVariableReferenced(SourceLocation Loc, VarDecl *Var) {
static void FixDependencyOfIdExpressionsInLambdaWithDependentObjectParameter(
Sema &SemaRef, ValueDecl *D, Expr *E) {
auto *ID = dyn_cast<DeclRefExpr>(E);
if (!ID || ID->isTypeDependent())
if (!ID || ID->isTypeDependent() || !ID->refersToEnclosingVariableOrCapture())
return;

// If any enclosing lambda with a dependent explicit object parameter either
// explicitly captures the variable by value, or has a capture default of '='
// and does not capture the variable by reference, then the type of the DRE
// is dependent on the type of that lambda's explicit object parameter.
auto IsDependent = [&]() {
const LambdaScopeInfo *LSI = SemaRef.getCurLambda();
if (!LSI)
return false;
if (!LSI->ExplicitObjectParameter ||
!LSI->ExplicitObjectParameter->getType()->isDependentType())
return false;
if (!LSI->CaptureMap.count(D))
return false;
const Capture &Cap = LSI->getCapture(D);
return !Cap.isCopyCapture();
for (auto *Scope : llvm::reverse(SemaRef.FunctionScopes)) {
auto *LSI = dyn_cast<sema::LambdaScopeInfo>(Scope);
if (!LSI)
continue;

if (LSI->Lambda && !LSI->Lambda->Encloses(SemaRef.CurContext) &&
LSI->AfterParameterList)
return false;

const auto *MD = LSI->CallOperator;
if (MD->getType().isNull())
continue;

const auto *Ty = cast<FunctionProtoType>(MD->getType());
if (!Ty || !MD->isExplicitObjectMemberFunction() ||
!Ty->getParamType(0)->isDependentType())
continue;

if (auto *C = LSI->CaptureMap.count(D) ? &LSI->getCapture(D) : nullptr) {
if (C->isCopyCapture())
return true;
continue;
}

if (LSI->ImpCaptureStyle == LambdaScopeInfo::ImpCap_LambdaByval)
return true;
}
return false;
}();

ID->setCapturedByCopyInLambdaWithExplicitObjectParameter(
Expand Down
36 changes: 36 additions & 0 deletions clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1462,6 +1462,42 @@ Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type,

void Sema::MarkThisReferenced(CXXThisExpr *This) {
CheckCXXThisCapture(This->getExprLoc());
if (This->isTypeDependent())
return;

// Check if 'this' is captured by value in a lambda with a dependent explicit
// object parameter, and mark it as type-dependent as well if so.
auto IsDependent = [&]() {
for (auto *Scope : llvm::reverse(FunctionScopes)) {
auto *LSI = dyn_cast<sema::LambdaScopeInfo>(Scope);
if (!LSI)
continue;

if (LSI->Lambda && !LSI->Lambda->Encloses(CurContext) &&
LSI->AfterParameterList)
return false;

// If this lambda captures 'this' by value, then 'this' is dependent iff
// this lambda has a dependent explicit object parameter. If we can't
// determine whether it does (e.g. because the CXXMethodDecl's type is
// null), assume it doesn't.
if (LSI->isCXXThisCaptured()) {
if (!LSI->getCXXThisCapture().isCopyCapture())
continue;

const auto *MD = LSI->CallOperator;
if (MD->getType().isNull())
return false;

const auto *Ty = cast<FunctionProtoType>(MD->getType());
return Ty && MD->isExplicitObjectMemberFunction() &&
Ty->getParamType(0)->isDependentType();
}
}
return false;
}();

This->setCapturedByCopyInLambdaWithExplicitObjectParameter(IsDependent);
}

bool Sema::isThisOutsideMemberFunctionBody(QualType BaseType) {
Expand Down
18 changes: 13 additions & 5 deletions clang/lib/Sema/TreeTransform.h
Original file line number Diff line number Diff line change
Expand Up @@ -11178,8 +11178,8 @@ TreeTransform<Derived>::TransformDeclRefExpr(DeclRefExpr *E) {
}

if (!getDerived().AlwaysRebuild() &&
QualifierLoc == E->getQualifierLoc() &&
ND == E->getDecl() &&
!E->isCapturedByCopyInLambdaWithExplicitObjectParameter() &&
QualifierLoc == E->getQualifierLoc() && ND == E->getDecl() &&
Found == E->getFoundDecl() &&
NameInfo.getName() == E->getDecl()->getDeclName() &&
!E->hasExplicitTemplateArgs()) {
Expand Down Expand Up @@ -12642,9 +12642,17 @@ TreeTransform<Derived>::TransformCXXThisExpr(CXXThisExpr *E) {
//
// In other contexts, the type of `this` may be overrided
// for type deduction, so we need to recompute it.
QualType T = getSema().getCurLambda() ?
getDerived().TransformType(E->getType())
: getSema().getCurrentThisType();
//
// Always recompute the type if we're in the body of a lambda, and
// 'this' is dependent on a lambda's explicit object parameter.
QualType T = [&]() {
auto &S = getSema();
if (E->isCapturedByCopyInLambdaWithExplicitObjectParameter())
return S.getCurrentThisType();
if (S.getCurLambda())
return getDerived().TransformType(E->getType());
return S.getCurrentThisType();
}();

if (!getDerived().AlwaysRebuild() && T == E->getType()) {
// Mark it referenced in the new context regardless.
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Serialization/ASTReaderStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1841,6 +1841,7 @@ void ASTStmtReader::VisitCXXThisExpr(CXXThisExpr *E) {
VisitExpr(E);
E->setLocation(readSourceLocation());
E->setImplicit(Record.readInt());
E->setCapturedByCopyInLambdaWithExplicitObjectParameter(Record.readInt());
}

void ASTStmtReader::VisitCXXThrowExpr(CXXThrowExpr *E) {
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Serialization/ASTWriterStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1839,6 +1839,7 @@ void ASTStmtWriter::VisitCXXThisExpr(CXXThisExpr *E) {
VisitExpr(E);
Record.AddSourceLocation(E->getLocation());
Record.push_back(E->isImplicit());
Record.push_back(E->isCapturedByCopyInLambdaWithExplicitObjectParameter());

Code = serialization::EXPR_CXX_THIS;
}
Expand Down
73 changes: 73 additions & 0 deletions clang/test/CodeGenCXX/cxx2b-deducing-this.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,76 @@ void test_temporary() {
//CHECK: %ref.tmp = alloca %struct.MaterializedTemporary, align 1
//CHECK: call void @_ZN21MaterializedTemporaryC1Ev(ptr noundef nonnull align 1 dereferenceable(1) %ref.tmp){{.*}}
//CHECK invoke void @_ZNH21MaterializedTemporary3fooEOS_(ptr noundef nonnull align 1 dereferenceable(1) %ref.tmp){{.*}}

namespace GH86399 {
volatile int a = 0;
struct function {
function& operator=(function const&) {
a = 1;
return *this;
}
};

void f() {
function list;

//CHECK-LABEL: define internal void @"_ZZN7GH863991f{{.*}}"(ptr %{{.*}})
//CHECK: call {{.*}} @_ZN7GH863998functionaSERKS0_
//CHECK-NEXT: ret void
[&list](this auto self) {
list = function{};
}();
}
}

namespace GH84163 {
// Just check that this doesn't crash (we were previously not instantiating
// everything that needs instantiating in here).
template <typename> struct S {};

void a() {
int x;
const auto l = [&x](this auto&) { S<decltype(x)> q; };
l();
}
}

namespace GH84425 {
// As above.
void do_thing(int x) {
auto second = [&](this auto const& self, int b) -> int {
if (x) return x;
else return self(x);
};

second(1);
}

void do_thing2(int x) {
auto second = [&](this auto const& self) {
if (true) return x;
else return x;
};

second();
}
}

namespace GH79754 {
// As above.
void f() {
int x;
[&x](this auto&&) {return x;}();
}
}

namespace GH70604 {
auto dothing(int num)
{
auto fun = [&num](this auto&& self) -> void {
auto copy = num;
};

fun();
}
}
35 changes: 35 additions & 0 deletions clang/test/PCH/cxx23-deducing-this-lambda.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// RUN: %clang_cc1 -emit-pch -std=c++23 -o %t %s
// RUN: %clang_cc1 -include-pch %t -verify -fsyntax-only -DTEST -std=c++23 %s

// Test that dependence of 'this' and DREs due to by-value capture by a
// lambda with an explicit object parameter is serialised/deserialised
// properly.

#ifndef HEADER
#define HEADER
struct S {
int x;
auto f() {
return [*this] (this auto&&) {
int y;
x = 42;

const auto l = [y] (this auto&&) { y = 42; };
l();
};
}
};
#endif

// expected-error@* {{read-only variable is not assignable}}
// expected-error@* {{cannot assign to a variable captured by copy in a non-mutable lambda}}
// expected-note@* 2 {{in instantiation of}}

#ifdef TEST
void f() {
const auto l = S{}.f();
l(); // expected-note {{in instantiation of}}
}
#endif


Loading