[PATCH] [OPENMP] 'if' clause support
Dmitri Gribenko
gribozavr at gmail.com
Fri Feb 7 04:35:56 PST 2014
================
Comment at: include/clang/AST/StmtOpenMP.h:139-142
@@ +138,6 @@
+
+ /// \brief Set condition.
+ ///
+ /// \param Cond Condition.
+ ///
+ void setCondition(Expr *Cond) { Condition = Cond; }
----------------
I would just delete this comment.
================
Comment at: include/clang/AST/StmtOpenMP.h:168-169
@@ +167,4 @@
+
+ /// \brief Returns condition.
+ Expr *getCondition() const { return cast_or_null<Expr>(Condition); }
+
----------------
Could we just store the condition as an Expr* member?
================
Comment at: lib/Parse/ParseOpenMP.cpp:273-275
@@ -274,3 +272,5 @@
+ case OMPC_if:
+ // OpenMP [2., Restrictions]
// Only a single default clause may be specified on a parallel or task
// directive.
if (!FirstClause) {
----------------
... only a single if clause...
Also, the reference seems incomplete.
================
Comment at: lib/Parse/ParseOpenMP.cpp:342
@@ +341,3 @@
+ if (Tok.is(tok::r_paren))
+ ConsumeAnyToken();
+
----------------
Using ConsumeParen() would avoid the extra check inside ConsumeAnyToken().
================
Comment at: lib/Parse/ParseOpenMP.cpp:325-330
@@ +324,8 @@
+ bool LParen = true;
+ if (Tok.isNot(tok::l_paren)) {
+ Diag(Tok, diag::err_expected_lparen_after) << getOpenMPClauseName(Kind);
+ LParen = false;
+ }
+ else
+ ConsumeAnyToken();
+
----------------
It would be better to swap the error and non-error path here. You also need to update LOpen, or it will stay the same as Loc.
Using ConsumeParen() would avoid the extra check inside ConsumeAnyToken().
http://llvm-reviews.chandlerc.com/D2719
More information about the cfe-commits
mailing list