[PATCH] [PATCH] OpenMP: Data-sharing attributes analysis and clause 'shared'

[Wei Pan]

================
Comment at: include/clang/AST/StmtOpenMP.h:293
@@ +292,3 @@
+                                 ArrayRef<Expr *> VL);
+  /// \brief Creates an empty clause with the place for \a N variables.
+  ///
----------------
"Creates an empty clause with \a N variables." ?

================
Comment at: lib/AST/Stmt.cpp:1161
@@ +1160,3 @@
+  OMPSharedClause *Clause = new (Mem) OMPSharedClause(StartLoc, LParenLoc,
+                                                       EndLoc, VL.size());
+  Clause->setVarRefs(VL);
----------------
alignment here.

================
Comment at: lib/Sema/SemaOpenMP.cpp:76
@@ +75,3 @@
+
+OpenMPClauseKind getDSA(StackTy::reverse_iterator Iter,
+                        VarDecl *D,
----------------
wrong alignment

================
Comment at: lib/Sema/SemaOpenMP.cpp:81
@@ +80,3 @@
+public:
+  DSAStackTy(Sema &S) : Stack(1), Actions(S) { }
+
----------------
explicit?

================
Comment at: lib/Sema/SemaOpenMP.cpp:103
@@ +102,3 @@
+
+  /// \brief Returns currently analized directive.
+  OpenMPDirectiveKind getCurrentDirective() const {
----------------
typo analized?

================
Comment at: test/OpenMP/parallel_shared_messages.cpp:1
@@ +1,2 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify -fopenmp -ferror-limit 100 %s
+
----------------
why do we need to specify a triple here?

================
Comment at: test/OpenMP/parallel_messages.cpp:1
@@ +1,2 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify -fopenmp -ferror-limit 100 -o - %s
+
----------------
I feel it is better to split parsing and sema tests.

================
Comment at: lib/Sema/TreeTransform.h:6330
@@ +6329,3 @@
+  for (OMPVarList<OMPSharedClause>::varlist_iterator I = C->varlist_begin(),
+                                                      E = C->varlist_end();
+       I != E; ++I) {
----------------
alignment here

================
Comment at: lib/Sema/TreeTransform.h:6262
@@ +6261,3 @@
+  DeclarationNameInfo DirName;
+  getDerived().getSema().StartOpenMPDSABlock(OMPD_parallel, DirName, 0);
+
----------------
just getSema()?

================
Comment at: lib/Sema/TreeTransform.h:6272
@@ -6260,3 +6271,3 @@
       OMPClause *Clause = getDerived().TransformOMPClause(*I);
       if (!Clause)
         return StmtError();
----------------
call EndOpenMPDSABlock on exit here?

================
Comment at: lib/Sema/TreeTransform.h:6281
@@ -6269,3 +6280,3 @@
   if (!D->getAssociatedStmt())
     return StmtError();
   StmtResult AssociatedStmt =
----------------
call EndOpenMPDSABlock on exit

================
Comment at: lib/Sema/TreeTransform.h:6291
@@ +6290,3 @@
+                                                            D->getLocEnd());
+  getDerived().getSema().EndOpenMPDSABlock(Res.get());
+  return Res;
----------------
no getDerived()

================
Comment at: lib/Sema/SemaOpenMP.cpp:847
@@ +846,3 @@
+    //  structure element) cannot appear in a private clause.
+    DeclRefExpr *DE = dyn_cast_or_null<DeclRefExpr>(*I);
+    if (!DE || !isa<VarDecl>(DE->getDecl())) {
----------------
only dyn_cast<...>, otherwise line 841 will fail first.

================
Comment at: lib/Sema/SemaOpenMP.cpp:869
@@ +868,3 @@
+         << getOpenMPClauseName(OMPC_shared);
+      if (PrevRef) {
+        Diag(PrevRef->getExprLoc(), diag::note_omp_explicit_dsa)
----------------
no {
and
}

================
Comment at: lib/Sema/SemaOpenMP.cpp:872
@@ +871,3 @@
+             << getOpenMPClauseName(Kind);
+      } else {
+        Diag(VD->getLocation(), diag::note_omp_predetermined_dsa)
----------------
same here

================
Comment at: lib/Sema/SemaOpenMP.cpp:227
@@ +226,3 @@
+  Scope *CurScope = getCurScope();
+  while (CurScope && !CurScope->isDeclScope(D)) {
+    CurScope = CurScope->getParent();
----------------
no {
}

================
Comment at: lib/Sema/SemaOpenMP.cpp:230
@@ +229,3 @@
+  }
+  while (CurScope && !CurScope->isOpenMPDirectiveScope()) {
+    CurScope = CurScope->getParent();
----------------
no { and }

================
Comment at: lib/Sema/SemaOpenMP.cpp:319
@@ -30,1 +318,3 @@
+}
+
 namespace {
----------------
undef DAStack?

================
Comment at: lib/Sema/SemaOpenMP.cpp:121
@@ +120,3 @@
+
+OpenMPClauseKind DSAStackTy::getDSA(StackTy::reverse_iterator Iter,
+                                    VarDecl *D,
----------------
I feel this function could be improved if just returns a struct. Rather then returning with two references. 

================
Comment at: lib/Sema/SemaOpenMP.cpp:511
@@ +510,3 @@
+      OpenMPClauseKind Kind = Stack->getTopDSA(VD, PrevRef);
+      if (Kind != OMPC_unknown) {
+        if (DKind == OMPD_task && Kind != OMPC_shared &&
----------------
Improve this block?

I guess you mean
  if (a)
    if (b)
      return;


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