[PATCH] D95108: [flang][directive] Enforce basic semantic check for all clauses

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan accepted this revision.
kiranchandramohan added a comment.
This revision is now accepted and ready to land.

LGTM. A few suggestions.  Feel free to skip or submit with changes.



================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:475
 
 // Following clauses do not have a seperate node in parse-tree.h.
+CHECK_SIMPLE_CLAUSE(AcqRel, OMPC_acq_rel)
----------------
Not of this patch but Nit: seperate -> separate.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:475
 
 // Following clauses do not have a seperate node in parse-tree.h.
+CHECK_SIMPLE_CLAUSE(AcqRel, OMPC_acq_rel)
----------------
kiranchandramohan wrote:
> Not of this patch but Nit: seperate -> separate.
Looks like you have captured more clauses here. Great!


================
Comment at: flang/lib/Semantics/check-omp-structure.h:134-141
   void Enter(const parser::OmpAtomicRead &);
   void Leave(const parser::OmpAtomicRead &);
   void Enter(const parser::OmpAtomicWrite &);
   void Leave(const parser::OmpAtomicWrite &);
   void Enter(const parser::OmpAtomicUpdate &);
   void Leave(const parser::OmpAtomicUpdate &);
   void Enter(const parser::OmpAtomicCapture &);
----------------
Can these be moved above the clauses?


================
Comment at: llvm/utils/TableGen/DirectiveEmitter.cpp:637
+// Generate check Enter functions for clauses classes.
+void GenerateFlangClauseCheckEnter(const DirectiveLanguage &DirLang,
+                                   raw_ostream &OS) {
----------------
Nit: I don't like the name. Also we are only generating the declaration. Would you be OK with GenerateFlangClauseCheckPrototypes? In the comment along it can be mentioned that the checks happen in the Enter stage during traversal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95108