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

[Roy Jacobson via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
royjacobson marked an inline comment as done.
Closed by commit rG0eb06cb3aa27: [Sema] Stop stripping CV quals from *this 
captures in lambdas (authored by royjacobson).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146168/new/

https://reviews.llvm.org/D146168

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/cxx1z-lambda-star-this.cpp

Index: clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
===
--- clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
+++ clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
@@ -88,13 +88,11 @@
   void foo() const { //expected-note{{const}}
 
 auto L = [*this]() mutable {
-  static_assert(is_same);
-  ++d;
+  static_assert(is_same);
   auto M = [this] {
-static_assert(is_same);
-++d;
+static_assert(is_same);
 auto N = [] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -108,9 +106,9 @@
 };
   };
   auto M2 = [*this]() mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -126,9 +124,9 @@
   };
 
   auto M2 = [*this](auto a) mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [](auto b) {
-  static_assert(is_same);
+  static_assert(is_same);
 };
 return N;
   };
@@ -143,13 +141,11 @@
   ++d; //expected-error{{cannot assign}}
 };
 auto GL = [*this](auto a) mutable {
-  static_assert(is_same);
-  ++d;
+  static_assert(is_same);
   auto M = [this](auto b) {
-static_assert(is_same);
-++d;
+static_assert(is_same);
 auto N = [](auto c) {
-  static_assert(is_same);
+  static_assert(is_same);
 };
 N(3.14);
   };
@@ -161,21 +157,21 @@
 auto L = [this]() {
   static_assert(is_same);
   auto M = [*this]() mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [this] {
-  static_assert(is_same);
+  static_assert(is_same);
   auto M = [] {
-static_assert(is_same);
+static_assert(is_same);
   };
 };
 auto N2 = [*this] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
   auto M2 = [*this]() {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [this] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -190,14 +186,13 @@
 auto L = [*this]() mutable {
   auto M = [=](auto a) {
 auto N = [this] {
-  ++d;
-  static_assert(is_same);
+  static_assert(is_same);
   auto O = [*this] {
 static_assert(is_same);
   };
 };
 N();
-static_assert(is_same);
+static_assert(is_same);
   };
   return M;
 };
@@ -308,3 +303,22 @@
 z(id,3);
 }
 } // namespace PR45881
+
+
+namespace GH50866 {
+struct S;
+
+void f(S *) = delete; // expected-note {{would lose const qualifier}}
+void f(const S *) = delete; // expected-note {{candidate function has been explicitly deleted}}
+
+struct S {
+  void g() const {
+[*this]() mutable { f(this); }(); // expected-error {{call to deleted function}}
+  }
+};
+
+void g() {
+  S s{};
+  s.g();
+}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -1135,7 +1135,6 @@
 auto C = CurLSI->getCXXThisCapture();
 
 if (C.isCopyCapture()) {
-  ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
   if (!CurLSI->Mutable)
 ClassType.addConst();
   return ASTCtx.getPointerType(ClassType);
@@ -1175,7 +1174,6 @@
 while (Closure &&
IsThisCaptured(Closure, IsByCopyCapture, IsConstCapture)) {
   if (IsByCopyCapture) {
-ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
 if (IsConstCapture)
   ClassType.addConst();
 return ASTCtx.getPointerType(ClassType);
@@ -1362,15 +1360,7 @@
 
 // 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 = This

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

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson marked an inline comment as done.
royjacobson added a comment.

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.




Comment at: clang/lib/Sema/SemaExprCXX.cpp:1138
 if (C.isCopyCapture()) {
-  ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
-  if (!CurLSI->Mutable)
+if (!CurLSI->Mutable)
 ClassType.addConst();

aaron.ballman wrote:
> This looks like an accidental formatting mistake that can be backed out.
ah, thanks for catching it!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146168/new/

https://reviews.llvm.org/D146168

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 509436.
royjacobson added a comment.

fix unintended indentation & rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146168/new/

https://reviews.llvm.org/D146168

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/cxx1z-lambda-star-this.cpp

Index: clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
===
--- clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
+++ clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
@@ -88,13 +88,11 @@
   void foo() const { //expected-note{{const}}
 
 auto L = [*this]() mutable {
-  static_assert(is_same);
-  ++d;
+  static_assert(is_same);
   auto M = [this] {
-static_assert(is_same);
-++d;
+static_assert(is_same);
 auto N = [] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -108,9 +106,9 @@
 };
   };
   auto M2 = [*this]() mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -126,9 +124,9 @@
   };
 
   auto M2 = [*this](auto a) mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [](auto b) {
-  static_assert(is_same);
+  static_assert(is_same);
 };
 return N;
   };
@@ -143,13 +141,11 @@
   ++d; //expected-error{{cannot assign}}
 };
 auto GL = [*this](auto a) mutable {
-  static_assert(is_same);
-  ++d;
+  static_assert(is_same);
   auto M = [this](auto b) {
-static_assert(is_same);
-++d;
+static_assert(is_same);
 auto N = [](auto c) {
-  static_assert(is_same);
+  static_assert(is_same);
 };
 N(3.14);
   };
@@ -161,21 +157,21 @@
 auto L = [this]() {
   static_assert(is_same);
   auto M = [*this]() mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [this] {
-  static_assert(is_same);
+  static_assert(is_same);
   auto M = [] {
-static_assert(is_same);
+static_assert(is_same);
   };
 };
 auto N2 = [*this] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
   auto M2 = [*this]() {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [this] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -190,14 +186,13 @@
 auto L = [*this]() mutable {
   auto M = [=](auto a) {
 auto N = [this] {
-  ++d;
-  static_assert(is_same);
+  static_assert(is_same);
   auto O = [*this] {
 static_assert(is_same);
   };
 };
 N();
-static_assert(is_same);
+static_assert(is_same);
   };
   return M;
 };
@@ -308,3 +303,22 @@
 z(id,3);
 }
 } // namespace PR45881
+
+
+namespace GH50866 {
+struct S;
+
+void f(S *) = delete; // expected-note {{would lose const qualifier}}
+void f(const S *) = delete; // expected-note {{candidate function has been explicitly deleted}}
+
+struct S {
+  void g() const {
+[*this]() mutable { f(this); }(); // expected-error {{call to deleted function}}
+  }
+};
+
+void g() {
+  S s{};
+  s.g();
+}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -1135,7 +1135,6 @@
 auto C = CurLSI->getCXXThisCapture();
 
 if (C.isCopyCapture()) {
-  ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
   if (!CurLSI->Mutable)
 ClassType.addConst();
   return ASTCtx.getPointerType(ClassType);
@@ -1175,7 +1174,6 @@
 while (Closure &&
IsThisCaptured(Closure, IsByCopyCapture, IsConstCapture)) {
   if (IsByCopyCapture) {
-ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
 if (IsConstCapture)
   ClassType.addConst();
 return ASTCtx.getPointerType(ClassType);
@@ -1362,15 +1360,7 @@
 
 // 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 isN

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

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?




Comment at: clang/lib/Sema/SemaExprCXX.cpp:1138
 if (C.isCopyCapture()) {
-  ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
-  if (!CurLSI->Mutable)
+if (!CurLSI->Mutable)
 ClassType.addConst();

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146168/new/

https://reviews.llvm.org/D146168

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 506295.
royjacobson added a comment.

Add the test case from the GH issue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146168/new/

https://reviews.llvm.org/D146168

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/cxx1z-lambda-star-this.cpp

Index: clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
===
--- clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
+++ clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
@@ -88,13 +88,11 @@
   void foo() const { //expected-note{{const}}
 
 auto L = [*this]() mutable {
-  static_assert(is_same);
-  ++d;
+  static_assert(is_same);
   auto M = [this] {
-static_assert(is_same);
-++d;
+static_assert(is_same);
 auto N = [] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -108,9 +106,9 @@
 };
   };
   auto M2 = [*this]() mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -126,9 +124,9 @@
   };
 
   auto M2 = [*this](auto a) mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [](auto b) {
-  static_assert(is_same);
+  static_assert(is_same);
 };
 return N;
   };
@@ -143,13 +141,11 @@
   ++d; //expected-error{{cannot assign}}
 };
 auto GL = [*this](auto a) mutable {
-  static_assert(is_same);
-  ++d;
+  static_assert(is_same);
   auto M = [this](auto b) {
-static_assert(is_same);
-++d;
+static_assert(is_same);
 auto N = [](auto c) {
-  static_assert(is_same);
+  static_assert(is_same);
 };
 N(3.14);
   };
@@ -161,21 +157,21 @@
 auto L = [this]() {
   static_assert(is_same);
   auto M = [*this]() mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [this] {
-  static_assert(is_same);
+  static_assert(is_same);
   auto M = [] {
-static_assert(is_same);
+static_assert(is_same);
   };
 };
 auto N2 = [*this] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
   auto M2 = [*this]() {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [this] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -190,14 +186,13 @@
 auto L = [*this]() mutable {
   auto M = [=](auto a) {
 auto N = [this] {
-  ++d;
-  static_assert(is_same);
+  static_assert(is_same);
   auto O = [*this] {
 static_assert(is_same);
   };
 };
 N();
-static_assert(is_same);
+static_assert(is_same);
   };
   return M;
 };
@@ -308,3 +303,22 @@
 z(id,3);
 }
 } // namespace PR45881
+
+
+namespace GH50866 {
+struct S;
+
+void f(S *) = delete; // expected-note {{would lose const qualifier}}
+void f(const S *) = delete; // expected-note {{candidate function has been explicitly deleted}}
+
+struct S {
+  void g() const {
+[*this]() mutable { f(this); }(); // expected-error {{call to deleted function}}
+  }
+};
+
+void g() {
+  S s{};
+  s.g();
+}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -1135,8 +1135,7 @@
 auto C = CurLSI->getCXXThisCapture();
 
 if (C.isCopyCapture()) {
-  ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
-  if (!CurLSI->Mutable)
+if (!CurLSI->Mutable)
 ClassType.addConst();
   return ASTCtx.getPointerType(ClassType);
 }
@@ -1175,7 +1174,6 @@
 while (Closure &&
IsThisCaptured(Closure, IsByCopyCapture, IsConstCapture)) {
   if (IsByCopyCapture) {
-ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
 if (IsConstCapture)
   ClassType.addConst();
 return ASTCtx.getPointerType(ClassType);
@@ -1362,15 +1360,7 @@
 
 // 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->get

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

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146168/new/

https://reviews.llvm.org/D146168

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added subscribers: hubert.reinterpretcast, cor3ntin.
cor3ntin accepted this revision.
cor3ntin added a comment.
This revision is now accepted and ready to land.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146168/new/

https://reviews.llvm.org/D146168

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146168/new/

https://reviews.llvm.org/D146168

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 505624.
royjacobson added a comment.

rebase + release notes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146168/new/

https://reviews.llvm.org/D146168

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/cxx1z-lambda-star-this.cpp

Index: clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
===
--- clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
+++ clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
@@ -88,13 +88,11 @@
   void foo() const { //expected-note{{const}}
 
 auto L = [*this]() mutable {
-  static_assert(is_same);
-  ++d;
+  static_assert(is_same);
   auto M = [this] {
-static_assert(is_same);
-++d;
+static_assert(is_same);
 auto N = [] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -108,9 +106,9 @@
 };
   };
   auto M2 = [*this]() mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -126,9 +124,9 @@
   };
 
   auto M2 = [*this](auto a) mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [](auto b) {
-  static_assert(is_same);
+  static_assert(is_same);
 };
 return N;
   };
@@ -143,13 +141,11 @@
   ++d; //expected-error{{cannot assign}}
 };
 auto GL = [*this](auto a) mutable {
-  static_assert(is_same);
-  ++d;
+  static_assert(is_same);
   auto M = [this](auto b) {
-static_assert(is_same);
-++d;
+static_assert(is_same);
 auto N = [](auto c) {
-  static_assert(is_same);
+  static_assert(is_same);
 };
 N(3.14);
   };
@@ -161,21 +157,21 @@
 auto L = [this]() {
   static_assert(is_same);
   auto M = [*this]() mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [this] {
-  static_assert(is_same);
+  static_assert(is_same);
   auto M = [] {
-static_assert(is_same);
+static_assert(is_same);
   };
 };
 auto N2 = [*this] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
   auto M2 = [*this]() {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [this] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -190,14 +186,13 @@
 auto L = [*this]() mutable {
   auto M = [=](auto a) {
 auto N = [this] {
-  ++d;
-  static_assert(is_same);
+  static_assert(is_same);
   auto O = [*this] {
 static_assert(is_same);
   };
 };
 N();
-static_assert(is_same);
+static_assert(is_same);
   };
   return M;
 };
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -1135,8 +1135,7 @@
 auto C = CurLSI->getCXXThisCapture();
 
 if (C.isCopyCapture()) {
-  ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
-  if (!CurLSI->Mutable)
+if (!CurLSI->Mutable)
 ClassType.addConst();
   return ASTCtx.getPointerType(ClassType);
 }
@@ -1175,7 +1174,6 @@
 while (Closure &&
IsThisCaptured(Closure, IsByCopyCapture, IsConstCapture)) {
   if (IsByCopyCapture) {
-ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
 if (IsConstCapture)
   ClassType.addConst();
 return ASTCtx.getPointerType(ClassType);
@@ -1362,15 +1360,7 @@
 
 // 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);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -225,6 +225,9 @@
 - Fix an issue about ``decltype`` in the members of class templates derived from
   templates with related parameters.
   (`#58

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

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson created this revision.
Herald added a project: All.
royjacobson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146168

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/cxx1z-lambda-star-this.cpp

Index: clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
===
--- clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
+++ clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
@@ -88,13 +88,11 @@
   void foo() const { //expected-note{{const}}
 
 auto L = [*this]() mutable {
-  static_assert(is_same);
-  ++d;
+  static_assert(is_same);
   auto M = [this] {
-static_assert(is_same);
-++d;
+static_assert(is_same);
 auto N = [] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -108,9 +106,9 @@
 };
   };
   auto M2 = [*this]() mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -126,9 +124,9 @@
   };
 
   auto M2 = [*this](auto a) mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [](auto b) {
-  static_assert(is_same);
+  static_assert(is_same);
 };
 return N;
   };
@@ -143,13 +141,11 @@
   ++d; //expected-error{{cannot assign}}
 };
 auto GL = [*this](auto a) mutable {
-  static_assert(is_same);
-  ++d;
+  static_assert(is_same);
   auto M = [this](auto b) {
-static_assert(is_same);
-++d;
+static_assert(is_same);
 auto N = [](auto c) {
-  static_assert(is_same);
+  static_assert(is_same);
 };
 N(3.14);
   };
@@ -161,21 +157,21 @@
 auto L = [this]() {
   static_assert(is_same);
   auto M = [*this]() mutable {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [this] {
-  static_assert(is_same);
+  static_assert(is_same);
   auto M = [] {
-static_assert(is_same);
+static_assert(is_same);
   };
 };
 auto N2 = [*this] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
   auto M2 = [*this]() {
-static_assert(is_same);
+static_assert(is_same);
 auto N = [this] {
-  static_assert(is_same);
+  static_assert(is_same);
 };
   };
 };
@@ -190,14 +186,13 @@
 auto L = [*this]() mutable {
   auto M = [=](auto a) {
 auto N = [this] {
-  ++d;
-  static_assert(is_same);
+  static_assert(is_same);
   auto O = [*this] {
 static_assert(is_same);
   };
 };
 N();
-static_assert(is_same);
+static_assert(is_same);
   };
   return M;
 };
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -1135,8 +1135,7 @@
 auto C = CurLSI->getCXXThisCapture();
 
 if (C.isCopyCapture()) {
-  ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
-  if (!CurLSI->Mutable)
+if (!CurLSI->Mutable)
 ClassType.addConst();
   return ASTCtx.getPointerType(ClassType);
 }
@@ -1175,7 +1174,6 @@
 while (Closure &&
IsThisCaptured(Closure, IsByCopyCapture, IsConstCapture)) {
   if (IsByCopyCapture) {
-ClassType.removeLocalCVRQualifiers(Qualifiers::CVRMask);
 if (IsConstCapture)
   ClassType.addConst();
 return ASTCtx.getPointerType(ClassType);
@@ -1362,15 +1360,7 @@
 
 // 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);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits