[PATCH] D108904: [flang][OpenMP] Added semantic checks for sections (associated section(s) should be structured block(s)) and simd constructs (associated loop(s) should be structured block(s))

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan requested changes to this revision.
kiranchandramohan added a comment.
This revision now requires changes to proceed.

A few more questions and comments.



================
Comment at: flang/lib/Semantics/check-directive-structure.h:19
 #include <unordered_map>
-
 namespace Fortran::semantics {
----------------
Nit: Accidentally removed?


================
Comment at: flang/lib/Semantics/check-directive-structure.h:121
       const char *stmt, const parser::Name &stmtName) {
-    const ConstructStack &stack{context_.constructStack()};
+    const ConstructStack stack{context_.constructStack()};
     for (auto iter{stack.cend()}; iter-- != stack.cbegin();) {
----------------
Wouldn't the removal of the reference result in a copy? Is that the intention here?


================
Comment at: flang/lib/Semantics/check-directive-structure.h:134
 
+  bool CheckForRequiredConstruct(const ConstructNode &construct) const {
+    return std::visit(common::visitors{
----------------
CheckForRequiredConstruct -> CheckForRequiredDoConstruct or CheckForDoConstruct


================
Comment at: flang/lib/Semantics/check-directive-structure.h:148
+      // found an enclosing looping construct for the unlabelled EXIT/CYCLE
+      if (CheckForRequiredConstruct(construct)) {
+        return;
----------------
This function can be a lambda.


================
Comment at: flang/lib/Semantics/check-directive-structure.h:160
+  // encountering an OpenMP/OpenACC directive
+  ConstructStack privateConstructStack;
   SemanticsContext &context_;
----------------
Looking at this again, do we need a construct stack? Can we just maintain a DoConstruct counter (which increments on entering (or Pre) a Do and decrements on leaving (or Post) a Do)? And if the DoConstruct counter value is greater than 0 then we are inside a DoConstruct, else outside.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108904