[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: lib/AST/DeclOpenMP.cpp:164
+  if (NumClauses) {
+    Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses);
+    setClauses(CL);
----------------
No, bad idea. Use tail allocation for the clauses. Check the implementation of `OMPRequiresDecl`


================
Comment at: lib/CodeGen/CodeGenModule.h:1249
+  void EmitOMPDeclareMapper(const OMPDeclareMapperDecl *D,
+                               CodeGenFunction *CGF = nullptr);
+
----------------
Formatting.


================
Comment at: lib/Parse/ParseOpenMP.cpp:503
+    } else
+      MapperId = DeclNames.getIdentifier(Tok.getIdentifierInfo());
+    ConsumeToken();
----------------
Enclose substatement in braces.


================
Comment at: lib/Parse/ParseOpenMP.cpp:589
+
+TypeResult Parser::parseOpenMPDeclareMapperVarDecl(SourceRange *Range,
+                                                   DeclarationName &Name,
----------------
Pass `Range` by reference rather than as a pointer


================
Comment at: lib/Sema/SemaOpenMP.cpp:13587
   // OpenMP [2.13.2, critical construct, Description]
-  // ... where hint-expression is an integer constant expression that evaluates
-  // to a valid lock hint.
+  // ... where hint-expression is an integer constant expression that
+  // evaluates to a valid lock hint.
----------------
Restore this comment.


================
Comment at: lib/Sema/SemaOpenMP.cpp:13624
       // OpenMP [2.7.1, Restrictions]
-      //  chunk_size must be a loop invariant integer expression with a positive
-      //  value.
+      //  chunk_size must be a loop invariant integer expression with a
+      //  positive value.
----------------
Restore original comment


================
Comment at: lib/Sema/SemaOpenMP.cpp:13919
 
-    // We need to add a data sharing attribute for this variable to make sure it
-    // is correctly captured. A variable that shows up in a use_device_ptr has
-    // similar properties of a first private variable.
+    // We need to add a data sharing attribute for this variable to make sure
+    // it is correctly captured. A variable that shows up in a use_device_ptr
----------------
Restore al the comments that are not related to the patch itself.


================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:2868
+TemplateDeclInstantiator::VisitOMPDeclareMapperDecl(OMPDeclareMapperDecl *D) {
+  assert(!D->getType()->isDependentType() &&
+         !D->getType()->isInstantiationDependentType() &&
----------------
Why?


================
Comment at: lib/Serialization/ASTReader.cpp:12304
   for (unsigned i = 0; i != NumVars; ++i)
-    Vars.push_back(Record.readSubExpr());
+    Vars.push_back(Record.readExpr());
   C->setVarRefs(Vars);
----------------
Restore original


================
Comment at: lib/Serialization/ASTReader.cpp:12328
   for (unsigned i = 0; i < TotalComponents; ++i) {
-    Expr *AssociatedExpr = Record.readSubExpr();
+    Expr *AssociatedExpr = Record.readExpr();
     auto *AssociatedDecl = Record.readDeclAs<ValueDecl>();
----------------
Restore original


Repository:
  rC Clang

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

https://reviews.llvm.org/D56326