[PATCH] OpenMP threadprivate directive

[Dmitri Gribenko]

================
Comment at: include/clang/AST/DeclOpenMP.h:28
@@ +27,3 @@
+///
+/// @code
+/// int a;
----------------
Please use backslashes for all doxygen commands (for consistency).

================
Comment at: include/clang/AST/DeclOpenMP.h:52
@@ +51,3 @@
+
+  llvm::MutableArrayRef<DeclRefExpr *> getVars() {
+    return llvm::MutableArrayRef<DeclRefExpr *>(
----------------
The difference between MutableArrayRef and ArrayRef here is that MutableArrayRef would allow users to modify the *pointers* themselves (and I don't think this is a good idea).  ArrayRef should probably be used here.

================
Comment at: lib/Parse/ParseOpenMP.cpp:68
@@ +67,3 @@
+/// directive
+///         ( unqualified-id {, unqualified-id} ) annot_pragma_openmp_end
+///
----------------
Please add production name here: `simple-variable-list`.

================
Comment at: lib/Parse/ParsePragma.cpp:721
@@ -720,1 +720,3 @@
 
+/// \brief Handle '#pragma omp ...' when no '-fopenmp' is provided.
+///
----------------
It might be better to explain this in terms of LangOpt flags, or just with a general "OpenMP is enabled".

================
Comment at: lib/Parse/ParsePragma.cpp:727-729
@@ +726,5 @@
+                                    Token &FirstTok) {
+  switch (PP.getDiagnostics().getDiagnosticLevel(diag::warn_pragma_omp_ignored,
+                                                 FirstTok.getLocation())) {
+  case DiagnosticsEngine::Ignored:
+    break;
----------------
You cloud replace the switch with a `!=` comparison with `Ignored`?

================
Comment at: lib/Parse/Parser.cpp:101-102
@@ +100,4 @@
+    OpenMPHandler.reset(new PragmaOpenMPHandler());
+  }
+  else {
+    OpenMPHandler.reset(new PragmaNoOpenMPHandler());
----------------
These braces should be on the same line.  Or better, just drop them.


================
Comment at: lib/Sema/SemaOpenMP.cpp:23
@@ +22,3 @@
+
+
+namespace {
----------------
Extra empty line?

================
Comment at: lib/Sema/SemaOpenMP.cpp:33-34
@@ +32,4 @@
+        if (NamedDecl *ND = Candidate.getCorrectionDecl()) {
+          return isa<VarDecl>(ND) &&
+                 cast<VarDecl>(ND)->hasGlobalStorage() &&
+                 Actions.isDeclInScope(ND, Actions.getCurLexicalContext(),
----------------
You can fuse this isa/cast pair to the previous line using `dyn_cast_or_null`.


================
Comment at: lib/Sema/SemaOpenMP.cpp:100
@@ +99,3 @@
+    //   in the scope of the variable and not in a nested scope.
+    NamedDecl *ND = cast<NamedDecl>(VD);
+    if (!isDeclInScope(ND, getCurLexicalContext(), CurScope)) {
----------------
VarDecl is derived from ValueDecl, which is derived from NamedDecl, there is no need for this variable.


================
Comment at: lib/Sema/SemaOpenMP.cpp:124-125
@@ +123,4 @@
+  }
+  if (OMPThreadPrivateDecl *D = CheckOMPThreadPrivateDecl(Loc,
+                                                          Vars)) {
+    CurContext->addDecl(D);
----------------
Weird line break.


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