[PATCH] D16527: [OpenMP] Parsing + sema for defaultmap clause.

[Alexey Bataev via cfe-commits]

ABataev added inline comments.

================
Comment at: include/clang/AST/OpenMPClause.h:3408-3409
@@ +3407,4 @@
+        Kind(Kind), KindLoc(KLoc) {
+    Modifier = M;
+    ModifierLoc = MLoc;
+  }
----------------
Why these are not initialized in initializer list, along with Kind, KindLoc etc.?

================
Comment at: include/clang/AST/OpenMPClause.h:3417
@@ +3416,3 @@
+        Kind(OMPC_DEFAULTMAP_unknown) {
+    Modifier = OMPC_DEFAULTMAP_MODIFIER_unknown;
+  }
----------------
Also, initialize this one in initializer list

================
Comment at: lib/AST/StmtPrinter.cpp:925
@@ +924,3 @@
+  OS << "defaultmap(";
+  if (Node->getDefaultmapModifier() != OMPC_DEFAULTMAP_MODIFIER_unknown) {
+    OS << getOpenMPSimpleClauseTypeName(OMPC_defaultmap,
----------------
I don't think that this check is required. According to defaultmap() syntax Modifier must be set, so I don't think it is allowed to have 'unknown' value here. I mean, output modifier and ':' unconditionally.

================
Comment at: lib/Sema/SemaOpenMP.cpp:8807
@@ +8806,3 @@
+    Value += "'";
+    Diag(KindLoc, diag::err_omp_unexpected_clause_value)
+        << Value << getOpenMPClauseName(OMPC_defaultmap);
----------------
I think for modifier Diag shall accept MLoc, not KindLoc

================
Comment at: lib/Sema/TreeTransform.h:7954-7957
@@ +7953,6 @@
+TreeTransform<Derived>::TransformOMPDefaultmapClause(OMPDefaultmapClause *C) {
+  return getDerived().RebuildOMPDefaultmapClause(
+      C->getDefaultmapModifier(), C->getDefaultmapKind(), C->getLocStart(),
+      C->getLParenLoc(), C->getDefaultmapModifierLoc(),
+      C->getDefaultmapKindLoc(), C->getLocEnd());
+}
----------------
I don't think you need to rebuild anything here, you can just return C. This new clause does not contain any possibly dependent members.


http://reviews.llvm.org/D16527