[PATCH] D19783: Fix cv-qualification of '*this' captures (and nasty bug PR27507 introduced by commit 263921 "Implement Lambda Capture of *this by Value as [=, *this]")

Faisal Vali via cfe-commits cfe-commits at lists.llvm.org
Wed May 18 22:16:50 PDT 2016


faisalv marked 5 inline comments as done.
faisalv added a comment.

OK - agree (and addressed in a forthcoming patch) all your comments - except for the one I could use some clarity on - please see below


================
Comment at: lib/Sema/SemaExprCXX.cpp:910
@@ +909,3 @@
+      assert(IsFirstIteration);
+      assert(CurLSI->CallOperator->getParent()->getParent() == CurDC);
+      CurDC = CurLSI->CallOperator;
----------------
rsmith wrote:
> Please add a comment explaining this, I have no idea what special case you're checking for here.
That special case should have been removed before submission - it crept in as I was making incremental changes to the initial patch, and getCurrentThisType was still being called from RebuildLambdaScopeInfo. (Alternatively, I might have left it in to give Richard a sense of purpose ;) 

================
Comment at: lib/Sema/SemaExprCXX.cpp:952
@@ +951,3 @@
+    while (Closure &&
+           IsThisCaptured(Closure, IsByCopyCapture, IsConstCapture)) {
+      if (IsByCopyCapture) {
----------------
rsmith wrote:
> Should you really stop here if this is not captured? There could still be a surrounding lambda with a mutable *this capture.
Do you have the following example in mind?  Or is it something different?

void foo() const {
  auto L1 = [*this] { 
      static_assert(is_same<decltype(this), const X*>);

      auto M2 = [*this] () mutable { 
        static_assert(is_same<decltype(this), X*>);  
        auto N = [] {
          static_assert(is_same<decltype(this), X*>); 
        };
      };
    };
}


http://reviews.llvm.org/D19783





More information about the cfe-commits mailing list