[PATCH] OpenMP parallel directive

[Wei Pan]

================
Comment at: include/clang/AST/StmtOpenMP.h:144
@@ +143,3 @@
+  /// \brief Return condition.
+  Expr *getCondition() { return dyn_cast_or_null<Expr>(Condition); }
+
----------------
The constructors assume thatn E is not null. Either change the constructor or update the get method.

================
Comment at: include/clang/AST/StmtOpenMP.h:190
@@ +189,3 @@
+  /// \brief Return number of threads.
+  Expr *getNumThreads() { return dyn_cast_or_null<Expr>(NumThreads); }
+
----------------
The same as above.

================
Comment at: include/clang/AST/StmtOpenMP.h:230
@@ +229,3 @@
+  ///
+  /// \brief A Argument of the clause ('none' or 'shared').
+  /// \brief ALoc Starting location of the argument.
----------------
\param and thhe following \brief's

================
Comment at: include/clang/AST/StmtOpenMP.h:349
@@ +348,3 @@
+  /// \param C AST context.
+  /// \brief StartLoc Starting location of the clause.
+  /// \brief EndLoc Ending location of the clause.
----------------
\param

================
Comment at: include/clang/AST/StmtOpenMP.h:406
@@ +405,3 @@
+  /// \param C AST context.
+  /// \brief StartLoc Starting location of the clause.
+  /// \brief EndLoc Ending location of the clause.
----------------
\param

================
Comment at: include/clang/AST/StmtOpenMP.h:503
@@ +502,3 @@
+  OpenMPReductionClauseOperator Operator;
+  /// \brief Start location of the kind in cource code.
+  SourceLocation OperatorLoc;
----------------
location of the reduction operator kind.

================
Comment at: include/clang/AST/StmtOpenMP.h:581
@@ +580,3 @@
+template <typename T> struct make_const_ptr_clause { typedef const T *type; };
+/// \brief This class implements a simple visitor for OMPClause
+/// subclasses.
----------------
Merge into the previous line.

================
Comment at: include/clang/AST/StmtOpenMP.h:609
@@ +608,3 @@
+
+template<class ImplClass, typename RetTy=void>
+class OMPClauseVisitor :
----------------
A few extra whitespaces RetTy = void

================
Comment at: include/clang/AST/StmtOpenMP.h:685
@@ +684,3 @@
+  ///
+  /// \brief Loc New starting location of directive.
+  ///
----------------
\param

================
Comment at: include/clang/AST/StmtOpenMP.h:690
@@ +689,3 @@
+  ///
+  /// \brief Loc New ending location of directive.
+  ///
----------------
param

================
Comment at: include/clang/AST/StmtOpenMP.h:702
@@ +701,3 @@
+  OMPClause *getClause(unsigned i) {
+    assert(i < NumClauses && "Wrong number of clause!");
+    return getClauses()[i];
----------------
index out of bound?

================
Comment at: include/clang/AST/StmtOpenMP.h:727
@@ +726,3 @@
+
+  static bool classof(const Stmt *S) {
+    return S->getStmtClass() == OMPParallelDirectiveClass;
----------------
move the following two functions to the of the class. This is consistent with other statements.

================
Comment at: include/clang/AST/StmtOpenMP.h:755
@@ +754,3 @@
+  ///
+  /// \param StartLoc Starting location of the directive kind.
+  /// \param EndLoc Ending Location of the directive.
----------------
it is the starting location of the directive not the directive kind?

================
Comment at: include/clang/AST/StmtOpenMP.h:779
@@ +778,3 @@
+  /// \param Clauses List of clauses.
+  /// \param AssociatedStmt Statement, associated with the directive.
+  ///
----------------
s/,/

================
Comment at: include/clang/Basic/OpenMPKinds.def:36
@@ +35,3 @@
+
+// OpenMP clause.
+OPENMP_CLAUSE(if, OMPIfClause)
----------------
clauses

================
Comment at: include/clang/AST/StmtOpenMP.h:353
@@ +352,3 @@
+  ///
+  static OMPFirstPrivateClause *Create(ASTContext &C,
+                                       SourceLocation StartLoc,
----------------
Are they all DeclRefExpr? I assume the following code is vaild (gcc 4.6 compiles), you will generate a MemberExpr not a DeclRefExpr.

void bar(int) { }
struct C {
  int a;
  void foo() {
    #pragma omp firstprivate(a)
    for (int i = 0; i < 10; i++) {
      bar(a);
    }
  }
};


================
Comment at: lib/AST/StmtPrinter.cpp:665
@@ +664,3 @@
+      OS << (I == Node->varlist_begin() ? '(' : ',')
+         << *cast<NamedDecl>(cast<DeclRefExpr>(*I)->getDecl());
+    }
----------------
This needs to change, if it is a MemberExpr. The same as the others.

================
Comment at: lib/AST/StmtPrinter.cpp:705
@@ +704,3 @@
+  if (Node->getAssociatedStmt()) {
+    assert(isa<CompoundStmt>(Node->getAssociatedStmt()) &&
+           "Expected compound statement!");
----------------
Cannot be a for statement or AttributedStmt?

  #pragma omp firstprivate(a)
  [[some C++11 attributes]] for (int i = 0; i < 10; i++) {
  }


================
Comment at: tools/libclang/CIndex.cpp:1920
@@ +1919,3 @@
+  unsigned size = WL.size();
+  for (Stmt::const_child_range Child = S->children(); Child; ++Child) {
+    AddStmt(*Child);
----------------
no {

and

}


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