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@+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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to