[flang-commits] [PATCH] D88965: [Flang][OpenMP] Rework parser changes for OpenMP atomic construct.

[Valentin Clement via Phabricator via flang-commits]

clementval added a comment.

Just to make things clear. The `OmpStructureChecker` can work with other specific clauses struct you introduce but of course if you want to leverage the current code that is in you have to use `OmpClause`.  But in the `Enter`/`Leave` function you can perform any semantic check you want. See for example `device_type` Sema in OpenACC. It is specific so of course it doesn't fall into the TableGen automatic checks. So this is fine to make the change you did but you could have also implement these checks with specific code for `OmpAtomicMemoryOrderClause`. Anyway I think if the atomic construct fits with `OmpClause` this is a better approach since the implementation is used by all other clauses.

Just added small comment. The change seems fine to me but I didn't check if it is conform to the OpenMP spec.



================
Comment at: flang/include/flang/Parser/parse-tree.h:3598
   CharBlock source;
-  std::tuple<Verbatim, std::optional<Name>, std::optional<OmpHintExpr>> t;
+  std::tuple<Verbatim, std::optional<Name>, std::optional<OmpClauseList>> t;
 };
----------------
Can your remove the optional? Empty list should do the work ... 


================
Comment at: flang/lib/Parser/openmp-parsers.cpp:219
         construct<OmpClause>(parenthesized(Parser<OmpProcBindClause>{})) ||
+
     "REDUCTION" >>
----------------
Remove new line. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88965