Thanks, I’ll get that commented-out test in-place. I look forward to Richard’s review.
Tyler On Aug 1, 2014, at 12:10 PM, Aaron Ballman <[email protected]> wrote: > On Fri, Aug 1, 2014 at 3:06 PM, Tyler Nowicki <[email protected]> wrote: >>>> + SmallVector<Token, 1> ValueList; >>>> + // Read constant expression. >>>> + while (Tok.isNot(tok::r_paren) && Tok.isNot(tok::eod)) { >>>> + ValueList.push_back(Tok); >>>> + PP.Lex(Tok); >>> >>> I don't think this is correct -- it won't handle constant expressions >>> involving parentheses properly. Eg) >>> >>> #pragma unroll((2+2)+4) >>> >>> It produces: >>> E:\Aaron Ballman\Desktop\test2.cpp:9:21: warning: extra tokens at end >>> of '#pragma unroll' - ignored >>> #pragma unroll((2+2)+4) >>> >>> So that should be fixed, and you should have a test for it as well. >> >> Oops, you are right. How about we do this incrementally? We can put this >> patch in now and I'll follow up with better parsing. I’ll be able work on >> this in a couple of weeks. >> >> I think we need to use the BalancedDelimiterTracker to resolve this issue. > > If you put in a FIXME with the code, and a commented-out test case > with a FIXME, I can live with that. I also agree that this code should > be using the BalancedDelimiterTracker. It will simplify a bit of the > code when you switch over to it. > > The rest of the patch LGTM, but I would also wait for Richard to > comment on the patch just in case (he's more familiar with the > constant expression stuff than I am). > > Also, don't forget to move the test cases to their appropriate directories. > ;-) > > ~Aaron
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
