[PATCH] [OPENMP] 'if' clause support

Alexey Bataev a.bataev at hotmail.com
Sun Feb 9 22:56:22 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; }
----------------
Dmitri Gribenko wrote:
> I would just delete this comment.
Ok, I'll delete it

================
Comment at: include/clang/AST/StmtOpenMP.h:168-169
@@ +167,4 @@
+
+  /// \brief Returns condition.
+  Expr *getCondition() const { return cast_or_null<Expr>(Condition); }
+
----------------
Dmitri Gribenko wrote:
> Could we just store the condition as an Expr* member?
I've stored it as Stmt * for children() method. Anyway there will be some casting, either in getCondition(), or in children().

================
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) {
----------------
Dmitri Gribenko wrote:
> ... only a single if clause...
> 
> Also, the reference seems incomplete.
Good catch, thanks!

================
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();
+
----------------
Dmitri Gribenko wrote:
> 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().
I'll rework parsing for the clause a little bit. Thanks.


http://llvm-reviews.chandlerc.com/D2719



More information about the cfe-commits mailing list