[PATCH] D92741: [flang][openacc] Enforce delcare directive restriction

[Valentin Clement via Phabricator via llvm-commits]

clementval added a comment.

Thanks for the review. Did not see it before for some reason. I made some update to the patch.



================
Comment at: flang/lib/Semantics/resolve-directives.cpp:117-120
-  bool Pre(const parser::SpecificationPart &x) {
-    Walk(std::get<std::list<parser::OpenACCDeclarativeConstruct>>(x.t));
-    return false;
-  }
----------------
sameeranjoshi wrote:
> Trying to understand- not a blocking review comment.
> The `Walk` should be handled in some tree-visitor files and not here, is my understanding correct?
Yeah the Walk is already handled by the Visitor class inherited by the `DirectiveAttributeVisitor`


================
Comment at: flang/lib/Semantics/resolve-directives.cpp:467
+  } else if (const auto *copyoutClause =
+                 std::get_if<Fortran::parser::AccClause::Copyin>(&clause.u)) {
+    const Fortran::parser::AccObjectListWithModifier &listWithModifier =
----------------
sameeranjoshi wrote:
> Possibly a bug.
> Is this `Copyout` ?
Good catch!


================
Comment at: flang/lib/Semantics/resolve-directives.cpp:487
+    return linkClause->v;
+  } else {
+    llvm_unreachable("Clause without object list!");
----------------
sameeranjoshi wrote:
> I see in my OACC 3.1 copy `create` clause as one of them in the list.
> Is this missing to add over here with a test below ?
Added it. 


================
Comment at: flang/lib/Semantics/resolve-directives.cpp:497
+    DoNotAllowAssumedSizedArray(GetAccObjectList(clause));
+  }
+}
----------------
sameeranjoshi wrote:
> Are checks from line `2410-2411` and `2412-2413` from OACC 3.1 missing or are handled somewhere else in previous patches ?
Those are not yet implemented and will be done in a separate patch. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92741