[PATCH] D89562: [flang]Add Parser Support for OpenMP Allocate Directive

[Valentin Clement via Phabricator via llvm-commits]

clementval requested changes to this revision.
clementval added inline comments.
This revision now requires changes to proceed.


================
Comment at: flang/include/flang/Parser/parse-tree.h:3630
+  std::tuple<Verbatim, std::optional<OmpObjectList>,
+      std::optional<AllocatorClause>, Statement<AllocateStmt>>
+      t;
----------------
You should use the OmpClauseList and let the semantic check do the work. Otherwise each directive will have there own parser for theirs associated directive and this duplicates the code a lot. 

`OMP.td` is already good for this directive. 

```
def OMP_Allocate : Directive<"allocate"> {
  let allowedClauses = [
    VersionedClause<OMPC_Allocator>
  ];
}
```


================
Comment at: flang/lib/Parser/openmp-parsers.cpp:482
+    verbatim("ALLOCATE"_tok), maybe(parenthesized(Parser<OmpObjectList>{})),
+    maybe("ALLOCATOR" >> parenthesized(scalarIntExpr)) / endOmpLine,
+    statement(allocateStmt))))
----------------
You should re-use `OmpClause` parser instead of duplicating the parsing.  If it's not in the `OmpClause` parser, just add it there. 


================
Comment at: flang/lib/Parser/openmp-parsers.cpp:498
+        parenthesized(Parser<OmpObjectList>{}),
+        maybe("ALLOCATOR" >> parenthesized(scalarIntExpr)))) /
+    lookAhead(endOmpLine / !statement(allocateStmt)))
----------------
Same here for the parser. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89562