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
__
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
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
_
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 typed/r
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 call
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 included
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:
inc
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(IsFirst
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 i
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 as well.
>
> BT
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 '
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
___
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.
>
> ==
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 reaso
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,
Tha
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
instantia
16 matches
Mail list logo