[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))

[Peixin Qiao via Phabricator via llvm-commits]

peixin added a comment.

I think your implementation does deal with the first point in your summary. You only add one test case, right?



================
Comment at: flang/lib/Semantics/check-directive-structure.h:157
+  void CheckConstructNameBranching(const char *stmt) {
+    // Check for associated ConstructNode within the OpenMP/OpenACC directive
+    const ConstructStack &stack{context_.constructStack()};
----------------
Instead of giving these comments. Could you please give one comment such as `CheckConstructNameBranching(const char *stmt, const parser::Name &stmtName)` to differentiate their usage? Maybe this advice is not good.


================
Comment at: flang/test/Semantics/omp-sections02.f90:22
+    !$omp section
+        open(10, file="random-file-name.txt",err=30)
+        !ERROR: invalid branch into an OpenMP structured block
----------------
Add space before `err=30`. Also please format your test cases. I think fortran test cases use two spaces on the next line instead of four spaces like the style in your part of code in `omp-simd01.f90`.


================
Comment at: flang/test/Semantics/omp-simd01.f90:14
   do i = 1, 10
     do j = 1, 10
       print *, "omp simd"
----------------
Does your implementation support the following code:

```
!$omp simd
do i = 1, 10
  30 stop
  do j = 1, 10
    goto 30
  end do
end do
!$omp end simd.
```
If no collapse clause specified, it behaves as if only one loop immediately following is associated with the construct. Could you please add the test in your test case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108904