[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: include/clang/AST/OpenMPClause.h:3626
+  struct OMPMappableExprListSizeTy {
+    unsigned NumVars;
+    unsigned NumUniqueDeclarations;
----------------
Add default initializers for the fields


================
Comment at: include/clang/AST/OpenMPClause.h:3652
+      OpenMPClauseKind K, SourceLocation StartLoc, SourceLocation LParenLoc,
+      SourceLocation EndLoc, unsigned NumVars, unsigned NumUniqueDeclarations,
+      unsigned NumComponentLists, unsigned NumComponents,
----------------
Remove this constructor, we should use only the one with `OMPMappableExprListSizeTy`


================
Comment at: include/clang/AST/OpenMPClause.h:3682-3685
+      OpenMPClauseKind K, SourceLocation StartLoc, SourceLocation LParenLoc,
+      SourceLocation EndLoc, OMPMappableExprListSizeTy Sizes,
+      NestedNameSpecifierLoc *MapperQualifierLocP = nullptr,
+      DeclarationNameInfo *MapperIdInfoP = nullptr)
----------------
I think it is worth to add `SourceLocation StartLoc`, `SourceLocation LParenLoc`, `SourceLocation EndLoc`, to `OMPMappableExprListSizeTy`. Of course, you need to rename the structure, something like `OMPMappableClauseData` is good enough. Or you can pack them into a different structure.


================
Comment at: include/clang/AST/OpenMPClause.h:4318
+  Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation LParenLoc,
+         SourceLocation EndLoc, ArrayRef<Expr *> Vars,
+         ArrayRef<ValueDecl *> Declarations,
----------------
Also too many params, need to "squash" some of them into a structure, maybe several structures.


================
Comment at: include/clang/Sema/Sema.h:9431
+      ArrayRef<SourceLocation> MapTypeModifiersLoc,
+      CXXScopeSpec &MapperIdScopeSpec, DeclarationNameInfo &MapperId,
+      OpenMPMapClauseKind MapType, bool IsMapTypeImplicit,
----------------
Also too many params here.


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

https://reviews.llvm.org/D58074