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

Reply via email to