[Sema] Stop stripping CV quals from *this captures in lambdas

[royjacobson]

Summary

It appears we've been incorrectly stripping CV qualifiers when capturing this by value inside lambdas.
This patch simply removes the CV stripping code as discussed.

Closes https://github.com/llvm/llvm-project/issues/50866

[royjacobson]

rebase + release notes.

[shafik]

Can you add a summary, I realize folks can goto the release notes edit and see the bug report and then go look it up but at least a cursory summary is a little more reviewer friendly.

[cor3ntin]

LGTM. Triple checking with @hubert.reinterpretcast for conformance but I'm pretty sure that's correct.

[shafik]

Can we also add the test from the bug report as a regression test, it looks like the existing test are basically covering the same thing but I have seen stranger bugs so it would be good to explicitly cover it.

[royjacobson]

Add the test case from the GH issue.

[aaron.ballman]

LGTM aside from a minor nit, though I have some minor concerns that we may silently break people's code by calling different overloads or instantiating templates differently so I wonder if this is a potentially breaking change or not. I think we can leave it out of the potentially breaking changes section unless we get some reports about a behavior change organically during this release cycle?

clang/lib/Sema/SemaExprCXX.cpp

1138

This looks like an accidental formatting mistake that can be backed out.

[royjacobson]

fix unintended indentation & rebase

[royjacobson]

In D146168#4231457, @aaron.ballman wrote:

LGTM aside from a minor nit, though I have some minor concerns that we may silently break people's code by calling different overloads or instantiating templates differently so I wonder if this is a potentially breaking change or not. I think we can leave it out of the potentially breaking changes section unless we get some reports about a behavior change organically during this release cycle?

My rule of thumb here is since we change behavior to match other compilers it's probably not too disruptive.

clang/lib/Sema/SemaExprCXX.cpp

1138

ah, thanks for catching it!

clang/lib/Sema/SemaExprCXX.cpp

for (int I = FunctionScopes.size();
     CurLSI = cast<LambdaScopeInfo>(FunctionScopes[I]);
 
     if (!CurLSI->isCXXThisCaptured())
         continue;
 
     auto C = CurLSI->getCXXThisCapture();
 
     if (C.isCopyCapture()) {
-      ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
         if (!CurLSI->Mutable)
         ClassType.addConst();
       return ASTCtx.getPointerType(ClassType);
     }
   }
 
   // 2) We've run out of ScopeInfos but check 1. if CurDC is a lambda (which
   //    can happen during instantiation of its nested generic lambda call
   //    operator); 2. if we're in a lambda scope (lambda body).
if (CurLSI && isLambdaCallOperator(CurDC)) {
     };
 
     bool IsByCopyCapture = false;
     bool IsConstCapture = false;
     CXXRecordDecl *Closure = cast<CXXRecordDecl>(CurDC->getParent());
     while (Closure &&
            IsThisCaptured(Closure, IsByCopyCapture, IsConstCapture)) {
       if (IsByCopyCapture) {
-        ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
         if (IsConstCapture)
           ClassType.addConst();
         return ASTCtx.getPointerType(ClassType);
       }
       Closure = isLambdaCallOperator(Closure->getParent())
                     ? cast<CXXRecordDecl>(Closure->getParent()->getParent())
                     : nullptr;
     }
assert((!ByCopy ||
          "*this) by copy");
   QualType ThisTy = getCurrentThisType();
   for (int idx = MaxFunctionScopesIndex; NumCapturingClosures;
        --idx, --NumCapturingClosures) {
     CapturingScopeInfo *CSI = cast<CapturingScopeInfo>(FunctionScopes[idx]);
 
     // The type of the corresponding data member (not a 'this' pointer if 'by
     // copy').
-    QualType CaptureType = ThisTy;
-    if (ByCopy) {
-      // If we are capturing the object referred to by '*this' by copy, ignore
-      // any cv qualifiers inherited from the type of the member function for
-      // the type of the closure-type's corresponding data member and any use
-      // of 'this'.
-      CaptureType = ThisTy->getPointeeType();
-      CaptureType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
-    }
+    QualType CaptureType = ByCopy ? ThisTy->getPointeeType() : ThisTy;
 
     bool isNested = NumCapturingClosures > 1;
     CSI->addThisCapture(isNested, Loc, CaptureType, ByCopy);
   }
   return false;
 }
 
 ExprResult Sema::ActOnCXXThis(SourceLocation Loc) {

clang/test/SemaCXX/cxx1z-lambda-star-this.cpp

struct X {
   double d = 3.14;
   X(const volatile X &);
   void foo() {
   }
 
   void foo() const { //expected-note{{const}}
 
     auto L = [*this]() mutable {
-      static_assert(is_same<decltype(this), X *>);
-      ++d;
+      static_assert(is_same<decltype(this), const X *>);
       auto M = [this] {
-        static_assert(is_same<decltype(this), X *>);
-        ++d;
+        static_assert(is_same<decltype(this), const X *>);
         auto N = [] {
-          static_assert(is_same<decltype(this), X *>);
+          static_assert(is_same<decltype(this), const X *>);
         };
       };
     };
 
     auto L1 = [*this] {
       static_assert(is_same<decltype(this), const X *>);
       auto M = [this]() mutable {
         static_assert(is_same<decltype(this), const X *>);
         auto N = [] {
           static_assert(is_same<decltype(this), const X *>);
         };
       };
       auto M2 = [*this]() mutable {
-        static_assert(is_same<decltype(this), X *>);
+        static_assert(is_same<decltype(this), const X *>);
         auto N = [] {
-          static_assert(is_same<decltype(this), X *>);
+          static_assert(is_same<decltype(this), const X *>);
         };
       };
     };
 
     auto GL1 = [*this](auto a) {
       static_assert(is_same<decltype(this), const X *>);
       auto M = [this](auto b) mutable {
         static_assert(is_same<decltype(this), const X *>);
         auto N = [](auto c) {
           static_assert(is_same<decltype(this), const X *>);
         };
         return N;
       };
 
       auto M2 = [*this](auto a) mutable {
-        static_assert(is_same<decltype(this), X *>);
+        static_assert(is_same<decltype(this), const X *>);
         auto N = [](auto b) {
-          static_assert(is_same<decltype(this), X *>);
+          static_assert(is_same<decltype(this), const X *>);
         };
         return N;
       };
       return [=](auto a) mutable { M(a)(a); M2(a)(a); };
     };
 
     GL1("abc")
     ("abc");
 
     auto L2 = [this]() mutable {
       static_assert(is_same<decltype(this), const X *>);
       ++d; //expected-error{{cannot assign}}
     };
     auto GL = [*this](auto a) mutable {
-      static_assert(is_same<decltype(this), X *>);
-      ++d;
+      static_assert(is_same<decltype(this), const X *>);
       auto M = [this](auto b) {
-        static_assert(is_same<decltype(this), X *>);
-        ++d;
+        static_assert(is_same<decltype(this), const X *>);
         auto N = [](auto c) {
-          static_assert(is_same<decltype(this), X *>);
+          static_assert(is_same<decltype(this), const X *>);
         };
         N(3.14);
       };
       M("abc");
     };
     GL(3.14);
   }
   void foo() volatile const {
     auto L = [this]() {
       static_assert(is_same<decltype(this), const volatile X *>);
       auto M = [*this]() mutable {
-        static_assert(is_same<decltype(this), X *>);
+        static_assert(is_same<decltype(this), const volatile X *>);
         auto N = [this] {
-          static_assert(is_same<decltype(this), X *>);
+          static_assert(is_same<decltype(this), const volatile X *>);
           auto M = [] {
-            static_assert(is_same<decltype(this), X *>);
+            static_assert(is_same<decltype(this), const volatile X *>);
           };
         };
         auto N2 = [*this] {
-          static_assert(is_same<decltype(this), const X *>);
+          static_assert(is_same<decltype(this), const volatile X *>);
         };
       };
       auto M2 = [*this]() {
-        static_assert(is_same<decltype(this), const X *>);
+        static_assert(is_same<decltype(this), const volatile X *>);
         auto N = [this] {
-          static_assert(is_same<decltype(this), const X *>);
+          static_assert(is_same<decltype(this), const volatile X *>);
         };
       };
     };
   }
 };
 
 } // namespace ns5
 namespace ns6 {
 struct X {
   double d;
   auto foo() const {
     auto L = [*this]() mutable {
       auto M = [=](auto a) {
         auto N = [this] {
-          ++d;
-          static_assert(is_same<decltype(this), X *>);
+          static_assert(is_same<decltype(this), const X *>);
           auto O = [*this] {
             static_assert(is_same<decltype(this), const X *>);
           };
         };
         N();
-        static_assert(is_same<decltype(this), X *>);
+        static_assert(is_same<decltype(this), const X *>);
       };
       return M;
     };
     return L;
   }
 };
 
 int main() {