[PATCH] D18542: [OPENMP] Parsing and Sema support for 'omp declare target' directive

[Aaron Ballman via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

Just some very minor nits, otherwise LGTM!


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7931
@@ +7930,3 @@
+def err_omp_enclosed_declare_target : Error<
+  "declare target region may not be enclosed into another declare target region">;
+def warn_omp_not_in_target_context : Warning<
----------------
s/into/within

================
Comment at: include/clang/Sema/Sema.h:7925
@@ +7924,3 @@
+  /// Return true inside OpenMP target region.
+  bool isInOpenMPDeclareTargetContext() {
+    return IsInOpenMPDeclareTargetContext;
----------------
This method should be marked `const`.

================
Comment at: lib/Frontend/MultiplexConsumer.cpp:225
@@ +224,3 @@
+    const Decl *D) {
+  for (size_t i = 0, e = Listeners.size(); i != e; ++i)
+    Listeners[i]->DeclarationMarkedOpenMPDeclareTarget(D);
----------------
Use std::for_each (or at least a range-based for loop) over Listeners.

================
Comment at: lib/Sema/SemaOpenMP.cpp:10405
@@ +10404,3 @@
+      D->addAttr(OMPDeclareTargetDeclAttr::CreateImplicit(SemaRef.Context));
+      if (auto *ML = SemaRef.Context.getASTMutationListener())
+        ML->DeclarationMarkedOpenMPDeclareTarget(D);
----------------
Please do not use `auto` since the type is not spelled anywhere else in the expression.

================
Comment at: lib/Sema/SemaOpenMP.cpp:10420
@@ +10419,3 @@
+      D->addAttr(OMPDeclareTargetDeclAttr::CreateImplicit(SemaRef.Context));
+      if (auto *ML = SemaRef.Context.getASTMutationListener())
+        ML->DeclarationMarkedOpenMPDeclareTarget(D);
----------------
Please do not use `auto` since the type is not spelled anywhere else in the expression.

================
Comment at: lib/Sema/SemaOpenMP.cpp:10450
@@ +10449,3 @@
+    D->addAttr(OMPDeclareTargetDeclAttr::CreateImplicit(SemaRef.Context));
+    if (auto *ML = SemaRef.Context.getASTMutationListener())
+      ML->DeclarationMarkedOpenMPDeclareTarget(D);
----------------
Please do not use `auto` since the type is not spelled anywhere else in the expression.

================
Comment at: lib/Sema/SemaOpenMP.cpp:10486
@@ +10485,3 @@
+        VD->addAttr(OMPDeclareTargetDeclAttr::CreateImplicit(Context));
+        if (auto *ML = Context.getASTMutationListener())
+          ML->DeclarationMarkedOpenMPDeclareTarget(VD);
----------------
Please do not use `auto` since the type is not spelled anywhere else in the expression.

================
Comment at: lib/Sema/SemaOpenMP.cpp:10497
@@ +10496,3 @@
+      D->addAttr(OMPDeclareTargetDeclAttr::CreateImplicit(Context));
+      if (auto *ML = Context.getASTMutationListener())
+        ML->DeclarationMarkedOpenMPDeclareTarget(D);
----------------
Please do not use `auto` since the type is not spelled anywhere else in the expression.


http://reviews.llvm.org/D18542