https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/81102
>From d489acbd3cfb656d203e1f05b74c238351c5428a Mon Sep 17 00:00:00 2001 From: Sirraide <aeternalm...@gmail.com> Date: Thu, 8 Feb 2024 08:33:03 +0100 Subject: [PATCH 1/5] [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 <aeternalm...@gmail.com> Date: Thu, 8 Feb 2024 09:45:40 +0100 Subject: [PATCH 2/5] [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 <aeternalm...@gmail.com> Date: Mon, 4 Mar 2024 14:19:28 +0100 Subject: [PATCH 3/5] [NFC] Fix tautological comment Co-authored-by: cor3ntin <corentinja...@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 <aeternalm...@gmail.com> Date: Mon, 4 Mar 2024 14:21:15 +0100 Subject: [PATCH 4/5] [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 <aeternalm...@gmail.com> Date: Mon, 4 Mar 2024 14:53:52 +0100 Subject: [PATCH 5/5] 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 ^^^^^^^^^^^^^^^^^^^^^^^^^ _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits