[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
Mon Mar 4 05:57:30 PST 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/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 <aeternalmail at 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 <aeternalmail at gmail.com>
Date: Mon, 4 Mar 2024 14:19:28 +0100
Subject: [PATCH 3/5] [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/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 <aeternalmail at 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
 ^^^^^^^^^^^^^^^^^^^^^^^^^



More information about the cfe-commits mailing list