[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/include/clang/AST/OpenMPClause.h:5869-5870
+    if (IteratorModifier) {
+      HasIteratorModifier = true;
+      getTrailingObjects<Expr *>()[2 * varlist_size()] = IteratorModifier;
+    }
----------------
Can we use just a nullptr as a mark that there is no associated iterator?


================
Comment at: clang/include/clang/Parse/Parser.h:3474-3475
   bool parseMapperModifier(Sema::OpenMPVarListDataTy &Data);
+  /// Parses the iterator modifier in map clause.
+  bool parseIteratorModifier(Sema::OpenMPVarListDataTy &Data);
   /// Parses map-type-modifiers in map clause.
----------------
Where is it defined, cannot find it in the patch?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:1166-1168
+    return llvm::any_of(Top->IteratorVarDecls, [VD](const VarDecl *IteratorVD) {
+      return IteratorVD == VD->getCanonicalDecl();
+    });
----------------
Do you really need to store the variable in the stack, is not it enough just to check that the type of this variable is BuiltinType::OMPIterator?


================
Comment at: clang/test/OpenMP/target_map_messages.cpp:256-257
+    // lt51-omp-error at +1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
+    #pragma omp target map(iterator(it=0:10), tofrom:a)
+    {}
     return;
----------------
Cases with incorrect parsing, wrong variables etc.?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141871/new/

https://reviews.llvm.org/D141871