[cfe-dev] OpenMP support in CLANG: A proposal

[Olaf Krzikalla]

Hi,

I did a quick look at your code and have some comments about it:

1. I don't think it is neccessary to introduce different classes for the 
various sub-domains. One pragma handler class should do the trick. Also 
I don't think that you need extra tokens. The sub-domain is always an 
identifier. Parse it and schedule based on a string comparision.

2. Do not derive OpenMP directives from Stmt. Literally they are no 
statements but directives. OpenMP directives attribute the statement 
following immediately (and I use "attribute" here by full intent - 
always keep C++11 attributes in mind). If there are also other pragmas 
before a for-statement you might run in serious trouble. That said, you 
should try to use (and maybe extend) the already installed attribute 
framework in the AST.

3. I wonder if we need to touch Sema and Parser the way you do. Granted, 
the Parser needs some extensions (more on that in the next bullet). But 
the parsing should be done in the handler, since OpenMP is not a part of 
C/C++ but rather an extension.

4. Last and most important point for other people still reading: clang 
DOES need a way to parse expressions inside of pragmas. Admittedly this 
violates the separation of concerns since pragmas exist at the 
preprocessor level and expressions exist at the Sema level. However 
that's just how it is. I hacked my own solution in Scout but after many 
hours of bug fixing I won't advertise that solution here. Instead I 
would opt for a parser redesign so that it is possible to have more than 
one parser at the same time operating on a token stream. Then we could 
construct a parser in a pragma handler and just call ParserExpression 
(or what else needs to be parsed). Thus the state of the global parser 
is not touched and it will happily jump to eol.


Best regards
Olaf