[PATCH] OpenMP parallel directive

[Alexey Bataev]

================
Comment at: include/clang/AST/StmtOpenMP.h:80
@@ +79,3 @@
+  /// \brief Fetches list of variables associated with this clause.
+  llvm::MutableArrayRef<Expr *> getVars() {
+    return llvm::MutableArrayRef<Expr *>(
----------------
Doug Gregor wrote:
> getVars() is slightly misleading. getVarRefs(), perhaps?
Ok

================
Comment at: include/clang/AST/StmtOpenMP.h:169
@@ +168,3 @@
+  ///
+  explicit OMPDefaultClause()
+    : OMPClause(OMPC_default, SourceLocation(), SourceLocation()),
----------------
Doug Gregor wrote:
> This doesn't need to be 'explicit'.
Ok

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6418
@@ -6407,1 +6417,3 @@
+def err_omp_clause_ref_type_arg : Error<
+  "arguments of OpenMP clause '%0' cannot be of reference type">;
 def err_omp_threadprivate_incomplete_type : Error<
----------------
Doug Gregor wrote:
> How about adding the actual type (via %1) at the end of this? It can be important when dealing with template instantiations.
Ok

================
Comment at: lib/Sema/SemaOpenMP.cpp:202
@@ +201,3 @@
+namespace {
+class StructuredStmtChecker : public StmtVisitor<StructuredStmtChecker, bool> {
+public:
----------------
Doug Gregor wrote:
> I gather that this is trying to check whether the given statement is a structured block, but it's not clear exactly what rules are being followed. (The OpenMP Specification is ridiculously vague in its definition). Note that the checking for "goto" rules are elsewhere (as they should be), and we aren't trying to check the rules for "throw" (which we can't figure out statically anyway), or "setjmp" (which could be here).
Doug, it tries to exclude stand-alone goto, break, continue, throw, return statements. All other statements are allowed.

================
Comment at: lib/Sema/SemaOpenMP.cpp:210
@@ +209,3 @@
+    //return false;
+    return true;
+  }
----------------
Doug Gregor wrote:
> Why is this always returning true?
I'll fix it

================
Comment at: lib/Sema/SemaOpenMP.cpp:255
@@ +254,3 @@
+    if (!Checker.Visit(AStmt)) {
+      Diag(StartLoc, diag::err_omp_directive_nonblock)
+        << getOpenMPDirectiveName(Kind) << AStmt;
----------------
Doug Gregor wrote:
> This isn't a very helpful diagnostic, because it doesn't show which part of the statement makes it not a structured block. Also, there don't seem to be any tests for this diagnostic.
> 
> I suggest removing the structured-block checking from this patch, because it's not nearly as baked as the rest of the patch and I don't want it holding up the rest.
Ok, I'll remove it for now.

================
Comment at: lib/Sema/SemaOpenMP.cpp:380
@@ +379,3 @@
+    //  structure element) cannot appear in a private clause.
+    DeclRefExpr *DE = dyn_cast_or_null<DeclRefExpr>(*I);
+    if (!DE || !isa<VarDecl>(DE->getDecl())) {
----------------
Doug Gregor wrote:
> It doesn't have to be handled as part of this patch, but within templates you'll also need to handle DependentScopeDeclRefExprs, as well have VD->getType() being type-dependent.
Ok.

================
Comment at: lib/Sema/SemaOpenMP.cpp:422
@@ +421,3 @@
+      if (!CD ||
+          CheckConstructorAccess(ELoc, CD,
+                                 InitializedEntity::InitializeTemporary(Type),
----------------
Doug Gregor wrote:
> Should also check that CD is not deleted and somewhere check for unavailable/deprecated (e.g., via DiagnoseUseOfDecl). I suspect you'll end up building a call to the default constructor to stash in the AST, but that doesn't need to be part of this patch.
> 
> 
Ok.

================
Comment at: lib/Sema/SemaOpenMP.cpp:436
@@ +435,3 @@
+      CXXDestructorDecl *DD = RD->getDestructor();
+      if (DD && !DD->isTrivial()) {
+        if (CheckDestructorAccess(ELoc, DD, PD) == AR_inaccessible) {
----------------
Doug Gregor wrote:
> Why the trivial check?
Sorry, copied from another place.

================
Comment at: lib/Serialization/ASTWriterStmt.cpp:1713
@@ +1712,3 @@
+void ASTStmtWriter::VisitOMPParallelDirective(OMPParallelDirective *D) {
+  VisitOMPExecutableDirective(D);
+  Code = serialization::STMT_OMP_PARALLEL_DIRECTIVE;
----------------
Doug Gregor wrote:
> There are no tests for this code?
I'll add the test


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