[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