[Clang][Sema] Properly get captured 'this' pointer in lambdas with an explicit object parameter in constant evaluator

[Sirraide]

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.

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (Sirraide)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/81102.diff

2 Files Affected:

• (modified) clang/lib/AST/ExprConstant.cpp (+75-55)
• (added) clang/test/SemaCXX/constexpr-explicit-object-lambda.cpp (+34)

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 089bc2094567f..8dc6348bc7eed 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 0000000000000..4e8e94d428d07
--- /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);

[Sirraide]

CC @AaronBallman, @cor3ntin, @shafik, @erichkeane, @Endilll

[Sirraide]

Should I fix the conflict in the release notes myself, or should that be done when this gets merged? Because at least in my experience, merge conflicts in the release notes just keep reappearing if a pr stays open for some time...

[erichkeane]

Should I fix the conflict in the release notes myself, or should that be done when this gets merged? Because at least in my experience, merge conflicts in the release notes just keep reappearing if a pr stays open for some time...

Lets hang out on that for a bit... Unfortunately you seem to have 'stepped in it' on this patch, so we might need to delay committing for a little while. But updates are coming.

[Sirraide]

@cor3ntin ping

[Sirraide]

@cor3ntin ping

[Sirraide]

@cor3ntin ping

[Sirraide]

Just requested more reviews real quick because this has been open for a month now, and Corentin apparently won’t be able to review prs until April, and I’d also feel a bit bad if I kept pinging just him about this over and over again...

[cor3ntin]

LGTM but I'll let @AaronBallman approve as I'm in a flight with bad internet today.

[AaronBallman]

LGTM aside from some coding style guideline nits