Re: [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]")

faisalv accepted this revision. faisalv added a reviewer: faisalv. faisalv added a comment. This revision is now accepted and ready to land. Committed as r272480 http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160606/161728.html http://reviews.llvm.org/D19783

Re: [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]")

faisalv added a comment. *ping* http://reviews.llvm.org/D19783 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [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]")

twoh added a comment. Hmm, yes it seems not so simple to move the call to another place. If I come up with a better idea I'll submit it as a separate patch, as I don't want to block this one. Thank you for your work, @faisalv! http://reviews.llvm.org/D19783

Re: [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]")

I'm also a huge advocate of simple composable interfaces that do the 'one' task that they are expected (i.e. named) to do - and do it well. I thought about this some, but perhaps not exhaustively - and settled on being ok with getCurThisType, always returning the correct type for 'this' as

Re: [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]")

twoh added a comment. @faisalv, thank you for the update. I wonder if getCurrentThisType() is the best place to call adjustCVQualifiersForCXXThisWithinLambda(). It seems that getCurrentThisType() does more than its name suggests. Is there any other place that the adjustment function can be

Re: [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]")

faisalv added inline comments. Comment at: lib/Parse/ParseDeclCXX.cpp:3651 @@ -3652,1 +3650,3 @@ + case AttributeList::AT_Unused: +return !ScopeName && AttrName->getName().equals("maybe_unused"); default: This whitespace change shouldn't have been

Re: [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]")

faisalv updated this revision to Diff 57733. faisalv marked an inline comment as done. faisalv added a comment. This patch addresses all of Richard's comments - except one (on which I'm awaiting some additional clarity on, before I make any changes). http://reviews.llvm.org/D19783 Files:

Re: [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]")

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 @@ +

Re: [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]")

There are tests from the test file in my patch that don't pass(*) if you just apply Oh's fix . That's not surprising since Oh's patch only meant to fix the crash, but not the 'cv' qualification issue of '*this' that we didn't have to deal with prior to by-value captures of '*this' and my initial

Re: [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]")

OK - thanks - will take a closer look at this hopefuly this evening or tomorrow - and respond to Richard's other comments too. Faisal Vali On Wed, May 18, 2016 at 1:46 PM, Taewook Oh wrote: > twoh added a comment. > > My patch passes check-clang and the test cases in this patch

Re: [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]")

twoh added a comment. My patch passes check-clang and the test cases in this patch as well. BTW, newly added test cases in the patch seem to be passed even without the patch. Isn't the bug appears when template instantiation generates nested lambdas (because template instantiation updates the

Re: [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]")

rsmith added a subscriber: rsmith. rsmith added a comment. I'd also like to know whether there are cases that this patch addresses but Taewook Oh's patch does not, as the other patch involves a lot less complexity. http://reviews.llvm.org/D19783

Re: [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]")

I'd also like to know whether there are cases that this patch addresses but Taewook Oh's patch does not, as the other patch involves a lot less complexity. On 18 May 2016 10:15 a.m., "Richard Smith via cfe-commits" < cfe-commits@lists.llvm.org> wrote: > rsmith added inline comments. > >

Re: [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]")

rsmith added inline comments. Comment at: lib/Sema/SemaExprCXX.cpp:898 @@ +897,3 @@ + // end of the TU) we need to be able to examine its enclosing lambdas and so + // we use the DeclContext to get a hold of the ClosureClass and query it for + // capture information. The

Re: [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]")

faisalv added a comment. Is it too soon for a *ping* ;) p.s. just added some comments. Comment at: lib/Sema/SemaExprCXX.cpp:904 @@ +903,3 @@ + for (int I = FunctionScopes.size(); + I-- && dyn_cast(FunctionScopes[I]); + IsFirstIteration = false,

[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]")

faisalv created this revision. faisalv added reviewers: rsmith, hubert.reinterpretcast. faisalv added subscribers: cfe-commits, gnzlbg. The bug report by Gonzalo (https://llvm.org/bugs/show_bug.cgi?id=27507 -- which results in clang crashing when generic lambdas that capture 'this' are