[clang] [Clang][Sema] Properly get captured 'this' pointer in lambdas with an explicit object parameter in constant evaluator (PR #81102)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 13 10:48:19 PDT 2024
https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/81102
>From d489acbd3cfb656d203e1f05b74c238351c5428a Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Thu, 8 Feb 2024 08:33:03 +0100
Subject: [PATCH 1/8] [Clang] Properly get captured 'this' pointer in lambdas
with an explicit object parameter in constant evaluator
---
clang/lib/AST/ExprConstant.cpp | 130 ++++++++++--------
.../constexpr-explicit-object-lambda.cpp | 34 +++++
2 files changed, 109 insertions(+), 55 deletions(-)
create mode 100644 clang/test/SemaCXX/constexpr-explicit-object-lambda.cpp
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 089bc2094567f7..8dc6348bc7eedb 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8480,6 +8480,54 @@ class LValueExprEvaluator
};
} // end anonymous namespace
+/// Get an lvalue to a field of a lambda's closure type.
+static bool GetLambdaCaptureAsLValue(EvalInfo &Info, const Expr *E,
+ LValue &Result, const CXXMethodDecl *MD,
+ const FieldDecl *FD,
+ bool LValueToRValueConversion) {
+ // Static lambda function call operators can't have captures. We already
+ // diagnosed this, so bail out here.
+ if (MD->isStatic()) {
+ assert(Info.CurrentCall->This == nullptr &&
+ "This should not be set for a static call operator");
+ return false;
+ }
+
+ // Start with 'Result' referring to the complete closure object...
+ if (MD->isExplicitObjectMemberFunction()) {
+ // Self may be passed by reference or by value.
+ auto *Self = MD->getParamDecl(0);
+ if (Self->getType()->isReferenceType()) {
+ APValue *RefValue = Info.getParamSlot(Info.CurrentCall->Arguments, Self);
+ Result.setFrom(Info.Ctx, *RefValue);
+ } else {
+ auto *VD = Info.CurrentCall->Arguments.getOrigParam(Self);
+ auto *Frame =
+ Info.getCallFrameAndDepth(Info.CurrentCall->Arguments.CallIndex)
+ .first;
+ auto Version = Info.CurrentCall->Arguments.Version;
+ Result.set({VD, Frame->Index, Version});
+ }
+ } else
+ Result = *Info.CurrentCall->This;
+
+ // ... then update it to refer to the field of the closure object
+ // that represents the capture.
+ if (!HandleLValueMember(Info, E, Result, FD))
+ return false;
+
+ // And if the field is of reference type (or if we captured '*this' by
+ // reference if this is a lambda), update 'Result' to refer to what
+ // the field refers to.
+ if (LValueToRValueConversion) {
+ APValue RVal;
+ if (!handleLValueToRValueConversion(Info, E, FD->getType(), Result, RVal))
+ return false;
+ Result.setFrom(Info.Ctx, RVal);
+ }
+ return true;
+}
+
/// Evaluate an expression as an lvalue. This can be legitimately called on
/// expressions which are not glvalues, in three cases:
/// * function designators in C, and
@@ -8524,37 +8572,8 @@ bool LValueExprEvaluator::VisitVarDecl(const Expr *E, const VarDecl *VD) {
if (auto *FD = Info.CurrentCall->LambdaCaptureFields.lookup(VD)) {
const auto *MD = cast<CXXMethodDecl>(Info.CurrentCall->Callee);
-
- // Static lambda function call operators can't have captures. We already
- // diagnosed this, so bail out here.
- if (MD->isStatic()) {
- assert(Info.CurrentCall->This == nullptr &&
- "This should not be set for a static call operator");
- return false;
- }
-
- // Start with 'Result' referring to the complete closure object...
- if (MD->isExplicitObjectMemberFunction()) {
- APValue *RefValue =
- Info.getParamSlot(Info.CurrentCall->Arguments, MD->getParamDecl(0));
- Result.setFrom(Info.Ctx, *RefValue);
- } else
- Result = *Info.CurrentCall->This;
-
- // ... then update it to refer to the field of the closure object
- // that represents the capture.
- if (!HandleLValueMember(Info, E, Result, FD))
- return false;
- // And if the field is of reference type, update 'Result' to refer to what
- // the field refers to.
- if (FD->getType()->isReferenceType()) {
- APValue RVal;
- if (!handleLValueToRValueConversion(Info, E, FD->getType(), Result,
- RVal))
- return false;
- Result.setFrom(Info.Ctx, RVal);
- }
- return true;
+ return GetLambdaCaptureAsLValue(Info, E, Result, MD, FD,
+ FD->getType()->isReferenceType());
}
}
@@ -9032,45 +9051,46 @@ class PointerExprEvaluator
return Error(E);
}
bool VisitCXXThisExpr(const CXXThisExpr *E) {
- // Can't look at 'this' when checking a potential constant expression.
- if (Info.checkingPotentialConstantExpression())
- return false;
- if (!Info.CurrentCall->This) {
+ auto DiagnoseInvalidUseOfThis = [&] {
if (Info.getLangOpts().CPlusPlus11)
Info.FFDiag(E, diag::note_constexpr_this) << E->isImplicit();
else
Info.FFDiag(E);
+ };
+
+ // Can't look at 'this' when checking a potential constant expression.
+ if (Info.checkingPotentialConstantExpression())
return false;
+
+ const bool IsExplicitLambda =
+ isLambdaCallWithExplicitObjectParameter(Info.CurrentCall->Callee);
+ if (!IsExplicitLambda) {
+ if (!Info.CurrentCall->This) {
+ DiagnoseInvalidUseOfThis();
+ return false;
+ }
+
+ Result = *Info.CurrentCall->This;
}
- Result = *Info.CurrentCall->This;
if (isLambdaCallOperator(Info.CurrentCall->Callee)) {
// Ensure we actually have captured 'this'. If something was wrong with
// 'this' capture, the error would have been previously reported.
// Otherwise we can be inside of a default initialization of an object
// declared by lambda's body, so no need to return false.
- if (!Info.CurrentCall->LambdaThisCaptureField)
- return true;
-
- // If we have captured 'this', the 'this' expression refers
- // to the enclosing '*this' object (either by value or reference) which is
- // either copied into the closure object's field that represents the
- // '*this' or refers to '*this'.
- // Update 'Result' to refer to the data member/field of the closure object
- // that represents the '*this' capture.
- if (!HandleLValueMember(Info, E, Result,
- Info.CurrentCall->LambdaThisCaptureField))
- return false;
- // If we captured '*this' by reference, replace the field with its referent.
- if (Info.CurrentCall->LambdaThisCaptureField->getType()
- ->isPointerType()) {
- APValue RVal;
- if (!handleLValueToRValueConversion(Info, E, E->getType(), Result,
- RVal))
+ if (!Info.CurrentCall->LambdaThisCaptureField) {
+ if (IsExplicitLambda && !Info.CurrentCall->This) {
+ DiagnoseInvalidUseOfThis();
return false;
+ }
- Result.setFrom(Info.Ctx, RVal);
+ return true;
}
+
+ const auto *MD = cast<CXXMethodDecl>(Info.CurrentCall->Callee);
+ return GetLambdaCaptureAsLValue(
+ Info, E, Result, MD, Info.CurrentCall->LambdaThisCaptureField,
+ Info.CurrentCall->LambdaThisCaptureField->getType()->isPointerType());
}
return true;
}
diff --git a/clang/test/SemaCXX/constexpr-explicit-object-lambda.cpp b/clang/test/SemaCXX/constexpr-explicit-object-lambda.cpp
new file mode 100644
index 00000000000000..4e8e94d428d071
--- /dev/null
+++ b/clang/test/SemaCXX/constexpr-explicit-object-lambda.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++23 -verify %s
+// expected-no-diagnostics
+
+struct S {
+ int i = 42;
+ constexpr auto f1() {
+ return [this](this auto) {
+ return this->i;
+ }();
+ };
+
+ constexpr auto f2() {
+ return [this](this auto&&) {
+ return this->i;
+ }();
+ };
+
+ constexpr auto f3() {
+ return [i = this->i](this auto) {
+ return i;
+ }();
+ };
+
+ constexpr auto f4() {
+ return [i = this->i](this auto&&) {
+ return i;
+ }();
+ };
+};
+
+static_assert(S().f1() == 42);
+static_assert(S().f2() == 42);
+static_assert(S().f3() == 42);
+static_assert(S().f4() == 42);
>From 3fe1a7f34a5da70739a656ce6c730c2188140b91 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Thu, 8 Feb 2024 09:45:40 +0100
Subject: [PATCH 2/8] [Clang] Update release notes
---
clang/docs/ReleaseNotes.rst | 3 +++
1 file changed, 3 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 802c44b6c86080..cc4e5cff89f918 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -208,6 +208,9 @@ Bug Fixes to C++ Support
parameter where we did an incorrect specialization of the initialization of
the default parameter.
Fixes (`#68490 <https://github.com/llvm/llvm-project/issues/68490>`_)
+- Fixed a bug and crashes in constant evaluation when trying to access a
+ captured ``this`` pointer in a lambda with an explicit object parameter.
+ Fixes (`#80997 https://github.com/llvm/llvm-project/issues/80997`_)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
>From 33b26f03a91257dde2217984461ce0c6007a2a38 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Mon, 4 Mar 2024 14:19:28 +0100
Subject: [PATCH 3/8] [NFC] Fix tautological comment
Co-authored-by: cor3ntin <corentinjabot at gmail.com>
---
clang/lib/AST/ExprConstant.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 8dc6348bc7eedb..f763057cfd95ce 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8517,7 +8517,7 @@ static bool GetLambdaCaptureAsLValue(EvalInfo &Info, const Expr *E,
return false;
// And if the field is of reference type (or if we captured '*this' by
- // reference if this is a lambda), update 'Result' to refer to what
+ // reference), update 'Result' to refer to what
// the field refers to.
if (LValueToRValueConversion) {
APValue RVal;
>From bf7d85c6486b389aecdaf90381bd90b675932b6e Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Mon, 4 Mar 2024 14:21:15 +0100
Subject: [PATCH 4/8] [NFC] Rename function
---
clang/lib/AST/ExprConstant.cpp | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index f763057cfd95ce..781001b775416c 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8481,10 +8481,9 @@ class LValueExprEvaluator
} // end anonymous namespace
/// Get an lvalue to a field of a lambda's closure type.
-static bool GetLambdaCaptureAsLValue(EvalInfo &Info, const Expr *E,
- LValue &Result, const CXXMethodDecl *MD,
- const FieldDecl *FD,
- bool LValueToRValueConversion) {
+static bool HandleLambdaCapture(EvalInfo &Info, const Expr *E, LValue &Result,
+ const CXXMethodDecl *MD, const FieldDecl *FD,
+ bool LValueToRValueConversion) {
// Static lambda function call operators can't have captures. We already
// diagnosed this, so bail out here.
if (MD->isStatic()) {
@@ -8572,8 +8571,8 @@ bool LValueExprEvaluator::VisitVarDecl(const Expr *E, const VarDecl *VD) {
if (auto *FD = Info.CurrentCall->LambdaCaptureFields.lookup(VD)) {
const auto *MD = cast<CXXMethodDecl>(Info.CurrentCall->Callee);
- return GetLambdaCaptureAsLValue(Info, E, Result, MD, FD,
- FD->getType()->isReferenceType());
+ return HandleLambdaCapture(Info, E, Result, MD, FD,
+ FD->getType()->isReferenceType());
}
}
@@ -9088,7 +9087,7 @@ class PointerExprEvaluator
}
const auto *MD = cast<CXXMethodDecl>(Info.CurrentCall->Callee);
- return GetLambdaCaptureAsLValue(
+ return HandleLambdaCapture(
Info, E, Result, MD, Info.CurrentCall->LambdaThisCaptureField,
Info.CurrentCall->LambdaThisCaptureField->getType()->isPointerType());
}
>From a2c61cdb6d45aa1b8d9745050de53db1acf35631 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Mon, 4 Mar 2024 14:53:52 +0100
Subject: [PATCH 5/8] Add release note back after merge
---
clang/docs/ReleaseNotes.rst | 3 +++
1 file changed, 3 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f44fef28b9f17f..27536a417e7082 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -298,6 +298,9 @@ Bug Fixes to C++ Support
Fixes (`#82941 <https://github.com/llvm/llvm-project/issues/82941>`_),
(`#42411 <https://github.com/llvm/llvm-project/issues/42411>`_), and
(`#18121 <https://github.com/llvm/llvm-project/issues/18121>`_).
+- Fixed a bug and crashes in constant evaluation when trying to access a
+ captured ``this`` pointer in a lambda with an explicit object parameter.
+ Fixes (`#80997 <https://github.com/llvm/llvm-project/issues/80997>`_)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
>From 37f2eedd862511bb7caa502eb81c4622bbd9806a Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Wed, 13 Mar 2024 16:09:47 +0100
Subject: [PATCH 6/8] Update clang/docs/ReleaseNotes.rst
Co-authored-by: cor3ntin <corentinjabot at gmail.com>
---
clang/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 941fcd63a5b837..97212a0ddf690d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -293,7 +293,7 @@ Bug Fixes to C++ Support
of templates. Previously we were diagnosing on any non-function template
instead of only on class, alias, and variable templates, as last updated by
CWG2032. Fixes (#GH#83461)
-- Fixed a bug and crashes in constant evaluation when trying to access a
+- Fixed a crash in constant evaluation when trying to access a
captured ``this`` pointer in a lambda with an explicit object parameter.
Fixes (#GH80997)
>From 8c84cfe25f00c0f06406e18cf4adeb6d94dea0c9 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Wed, 13 Mar 2024 18:40:12 +0100
Subject: [PATCH 7/8] Remove const
Co-authored-by: Aaron Ballman <aaron at aaronballman.com>
---
clang/lib/AST/ExprConstant.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 6354f5743da7f5..e49e56919fcfcc 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -9066,7 +9066,7 @@ class PointerExprEvaluator
if (Info.checkingPotentialConstantExpression())
return false;
- const bool IsExplicitLambda =
+ bool IsExplicitLambda =
isLambdaCallWithExplicitObjectParameter(Info.CurrentCall->Callee);
if (!IsExplicitLambda) {
if (!Info.CurrentCall->This) {
>From bee9b433a82038d4e70ff4bae7d5dd0aa0016504 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Wed, 13 Mar 2024 18:44:06 +0100
Subject: [PATCH 8/8] [NFC] Remove some uses of 'auto'
---
clang/lib/AST/ExprConstant.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index e49e56919fcfcc..f560a600ff1608 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8500,16 +8500,16 @@ static bool HandleLambdaCapture(EvalInfo &Info, const Expr *E, LValue &Result,
// Start with 'Result' referring to the complete closure object...
if (MD->isExplicitObjectMemberFunction()) {
// Self may be passed by reference or by value.
- auto *Self = MD->getParamDecl(0);
+ const ParmVarDecl *Self = MD->getParamDecl(0);
if (Self->getType()->isReferenceType()) {
APValue *RefValue = Info.getParamSlot(Info.CurrentCall->Arguments, Self);
Result.setFrom(Info.Ctx, *RefValue);
} else {
- auto *VD = Info.CurrentCall->Arguments.getOrigParam(Self);
- auto *Frame =
+ const ParmVarDecl *VD = Info.CurrentCall->Arguments.getOrigParam(Self);
+ CallStackFrame *Frame =
Info.getCallFrameAndDepth(Info.CurrentCall->Arguments.CallIndex)
.first;
- auto Version = Info.CurrentCall->Arguments.Version;
+ unsigned Version = Info.CurrentCall->Arguments.Version;
Result.set({VD, Frame->Index, Version});
}
} else
More information about the cfe-commits
mailing list