Hi Alexey, Thanks for taking the time to look at it.
I’m not sure what you are thinking should occur here. These things are taken care of by attributed stmts in general and any unidentified attributes are skipped with a warning. No one is forced to use #pragma loop, so I don’t think its a problem if some features are not implemented right now. I agree there could be some changes to support c++11 attributes and template parameters. I’m working on it, but I don’t know how to support those things as of yet. Sorry the description you gave for supporting template parameters wasn’t clear enough for me to follow. Tyler On Apr 30, 2014, at 12:23 AM, Alexey Bataev <[email protected]> wrote: > Hello Tyler, > I think that still there are some problems with your patch. > At first, add tests for serialization/deserialization and -ast-print. You'll > see that at least -ast-print works incorrectly because #pragma is represented > as a C++11 attribute. > Then, I don't think that it would be possible to fix problem with the > template instantiation by some incremental patch. IMO, it will require some > serious redesign of your solution, because template instantiation requires > AST node, but you're working with the values. You need to pass the whole > _value_ as a vector of tokens to Parser, which should parse this token > sequence as an expression to build an Expr AST node. > > Best regards, > Alexey Bataev > ============= > Software Engineer > Intel Compiler Team > Intel Corp. > > 30 Апрель 2014 г. 3:00:31, Tyler Nowicki писал: >> +Alexander and Douglas >> >> Sorry you guys dropped off the cc for some reason. >> >> On Apr 29, 2014, at 3:51 PM, Tyler Nowicki <[email protected] >> <mailto:[email protected]>> wrote: >> >>> Hi, >>> >>> I’ve updated the patch with the FIXME. I’ve also added a separate >>> test for the contradictory pragmas. >>> >>> @Alexander: Since BalancedDelimiterTracker does not have any >>> benefits and just adds unnecessary complexity I opted not to use it. >>> >>> Thanks everyone for your feedback. Please review the updated patch. >>> >>> Tyler >>> >>> <pragma_loop-svn.patch> >>> >>> On Apr 29, 2014, at 11:26 AM, Nadav Rotem <[email protected] >>> <mailto:[email protected]>> wrote: >>> >>>> >>>> On Apr 29, 2014, at 11:24 AM, Hal Finkel <[email protected] >>>> <mailto:[email protected]>> wrote: >>>> >>>>> I'm fine with this as incremental progress, so long as the >>>>> follow-up happens in the near term. Please add a FIXME describing >>>>> what needs to change to support constant expressions (including use >>>>> via template instantiation). >>>> >>>> +1. I also like the incremental approach, as long as we have >>>> bugzilla PRs to track. >>> >>> >>> On Apr 29, 2014, at 9:17 AM, Alexander Musman >>> <[email protected] <mailto:[email protected]>> wrote: >>> >>>> >>> >>>> >>> ParsePragma.cpp:1619: // Read '(' >>>> >>> This looks like a good place to use BalancedDelimiterTracker for >>>> parsing '(' and ')’. >>>> >>>> >>I don’t think it is needed. It isn’t used by any other #pragma >>>> >>directives and the syntax here is rather simple. What do you think would >>>> >>be the benefit? >>>> >>>> This would improve consistency with the other places in clang where >>>> '(' and ')' are parsed. Otherwise it seems to be equivalent to >>>> handling '(' and ')' manually. >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] <mailto:[email protected]> >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
