r209660 - Parsing/Sema for OMPCollapseClause.
cbertol at us.ibm.com
Tue Jun 3 08:11:40 PDT 2014
I reviewed the code you put down for the OMP collapse clause (as in
It looks fine to me - I only have some minor aesthetic comments:
1. In OpenMPClause.h: Stmt * NumForLoops —> why not a Expr *? It cannot be
Is this because we need a Stmt * to be passed to the RecursiveASTVisitor
2. In Sema.h, ActOnOpenMPCollapseClause: Expr * Num variable. Consider
making it more explicit, as “NumForLoops”.
3. In SemaOpenMP.cpp, Sema::ActOnOpenMPCollapseClause, the comment seems to
suggest that only the simd construct has the collapse clause, while also
“for” and “distribute: can have it. The related sections in the OpenMP
specifications are: 2.7.1 and 2.9.6. This will have to be fixed once “for”
and “distribute” get pushed from github into the main trunk.
4. TreeTransform.h: all comments to “Rebuild*” functions are the same.
Consider specializing (what semantic analysis) if appropriate or have a
single comment? Similarly, there are no comments at all on the “Transform*”
functions. Consider adding a single comment for all of them. The
indentation of “TransformOMPCollapseClause” is different from the majority
of the other clauses. Consider making it homogeneous.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-commits