https://github.com/Sirraide created https://github.com/llvm/llvm-project/pull/81102
There were some bugs wrt explicit object parameters in lambdas in the constant evaluator: - The code evaluating a `CXXThisExpr` wasn’t checking for explicit object parameters at all and thus assumed that there was no `this` in the current context because the lambda didn’t have one, even though we were in a member function and had captured its `this`. - The code retrieving captures as lvalues *did* account for explicit object parameters, but it did not handle the case of the explicit object parameter being passed by value rather than by reference. This fixes #80997. >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] [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); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits