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

[Sourabh Singh Tomar via Phabricator via flang-commits]

SouraVX added inline comments.


================
Comment at: flang/lib/Parser/unparse.cpp:2267
     Word("!$OMP ATOMIC");
-    Walk(std::get<OmpAtomicMemoryOrderClauseList>(x.t));
+    Walk(std::get<0>(x.t));
     Word(" UPDATE");
----------------
sameeranjoshi wrote:
> SouraVX wrote:
> > NIT: Can't we use the type name here ? `get<0>` does the job but makes the code less readable.
> If you take a look at how `parse-tree.h` represents `AtomicUpdate`
> ```
> // ATOMIC UPDATE
> struct OmpAtomicUpdate {
>   TUPLE_CLASS_BOILERPLATE(OmpAtomicUpdate);
>   std::tuple<OmpClauseList, Verbatim,
>       OmpClauseList, Statement<AssignmentStmt>,
>       std::optional<OmpEndAtomic>>
>       t;
> };
> ```
> 
> When `std::tuple` contains more than 1 repeated types, there is ambiguity for compiler to choose the type.
> This makes us use indices to make it explicit.
Gotcha Thanks! Maybe `get<0/*OmpClauseList*/>`. No hard reservations, This may look not so good(Personal taste). Feel free to ignore otherwise :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88965