[PATCH] D26843: Make sizeof expression context partially evaluated

2016-12-15 Thread Paulo Matos via Phabricator via cfe-commits
pmatos closed this revision. pmatos added a comment. This is not supposed to be marked as accepted. https://reviews.llvm.org/D26843 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26843: Make sizeof expression context partially evaluated

2016-12-15 Thread Paulo Matos via Phabricator via cfe-commits
pmatos accepted this revision. pmatos added a reviewer: pmatos. pmatos added a comment. This revision is now accepted and ready to land. Abandoning this revision. https://reviews.llvm.org/D26843 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D26843: Make sizeof expression context partially evaluated

2016-12-15 Thread Paulo Matos via Phabricator via cfe-commits
pmatos added a comment. Please refer to the new one: https://reviews.llvm.org/D27800 https://reviews.llvm.org/D26843 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26843: Make sizeof expression context partially evaluated

2016-12-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. I would say make a new review thread since none of the discussion here is going to be helpful for understanding the new patch... but it really doesn't matter. https://reviews.llvm.org/D26843 ___ cfe-commits mailing list cf

[PATCH] D26843: Make sizeof expression context partially evaluated

2016-12-09 Thread Paulo Matos via Phabricator via cfe-commits
pmatos added a subscriber: eli.friedman. pmatos added a comment. OK, with a lot of help from @eli.friedman I have now a fix. Shall I reuse this review by submitting a new diff or open a new one? https://reviews.llvm.org/D26843 ___ cfe-commits maili

Re: [PATCH] D26843: Make sizeof expression context partially evaluated

2016-11-18 Thread Eric Fiselier via cfe-commits
FYI I took the example from the [C++1z standard]( http://eel.is/c++draft/expr.prim.id#2) > [expr.prim.id]p2: > An id-expression that denotes a non-static data member or non-static member function of a class can only > be used [...] if that id-expression denotes a non-static data member and it appe

Re: [PATCH] D26843: Make sizeof expression context partially evaluated

2016-11-18 Thread Paulo Matos via cfe-commits
On 18/11/16 20:30, Aaron Ballman wrote: > On Fri, Nov 18, 2016 at 2:22 PM, Paulo Matos via cfe-commits > wrote: >> pmatos added a comment. >> >> Apologies if I am being shallow and wasting your time but `sizeof(T::m)` >> doesn't compile at the moment with clang trunk. Using the same service you

Re: [PATCH] D26843: Make sizeof expression context partially evaluated

2016-11-18 Thread Aaron Ballman via cfe-commits
On Fri, Nov 18, 2016 at 2:22 PM, Paulo Matos via cfe-commits wrote: > pmatos added a comment. > > Apologies if I am being shallow and wasting your time but `sizeof(T::m)` > doesn't compile at the moment with clang trunk. Using the same service you > used before

[PATCH] D26843: Make sizeof expression context partially evaluated

2016-11-18 Thread Paulo Matos via cfe-commits
pmatos added a comment. Apologies if I am being shallow and wasting your time but `sizeof(T::m)` doesn't compile at the moment with clang trunk. Using the same service you used before . https://reviews.llvm.org/D26843

[PATCH] D26843: Make sizeof expression context partially evaluated

2016-11-18 Thread Eric Fiselier via cfe-commits
EricWF added a comment. In case your wondering *why* this happens In https://reviews.llvm.org/D26843#599804, @pmatos wrote: > In https://reviews.llvm.org/D26843#599673, @EricWF wrote: > > > > But that is not valid in C afaik and in C++ I get: > > > > > > error: invalid use of non-static data

[PATCH] D26843: Make sizeof expression context partially evaluated

2016-11-18 Thread Eric Fiselier via cfe-commits
EricWF added a comment. > This is different from your initial snippet and compiles fine with my patch. Your confused. Both examples compile fine w/o your patch and are rejected with it. I just double checked. https://reviews.llvm.org/D26843 ___ cf

[PATCH] D26843: Make sizeof expression context partially evaluated

2016-11-18 Thread Paulo Matos via cfe-commits
pmatos added a comment. In https://reviews.llvm.org/D26843#599673, @EricWF wrote: > > But that is not valid in C afaik and in C++ I get: > > > > error: invalid use of non-static data member 'm' > > int x = sizeof(T::m); > >~~~^ > > > > > > Can you post a full reproduc

[PATCH] D26843: Make sizeof expression context partially evaluated

2016-11-18 Thread Eric Fiselier via cfe-commits
EricWF added a comment. > But that is not valid in C afaik and in C++ I get: > > error: invalid use of non-static data member 'm' > int x = sizeof(T::m); >~~~^ > > > Can you post a full reproducible example of what the change breaks? That is a full reproducible exampl

[PATCH] D26843: Make sizeof expression context partially evaluated

2016-11-18 Thread Paulo Matos via cfe-commits
pmatos added a comment. In https://reviews.llvm.org/D26843#599635, @EricWF wrote: > This isn't correct. For example this change breaks: > > struct T { int m; }; > int x = sizeof(T::m); > But that is not valid in C afaik and in C++ I get: error: invalid use of non-static data member 'm'

[PATCH] D26843: Make sizeof expression context partially evaluated

2016-11-18 Thread Eric Fiselier via cfe-commits
EricWF added a comment. This isn't correct. For example this change breaks: struct T { int m; }; int x = sizeof(T::m); https://reviews.llvm.org/D26843 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D26843: Make sizeof expression context partially evaluated

2016-11-18 Thread Paulo Matos via cfe-commits
pmatos created this revision. pmatos added a reviewer: efriedma. pmatos added a subscriber: cfe-commits. Ensure sizeof expression context is partially evaluated so that potential typeof operators inside are evaluated if necessary. Fixes PR31042. https://reviews.llvm.org/D26843 Files: lib/Pa