[PATCH] D83807: [flang][openacc] Semantic checks for OpenACC 3.0 clauses validity

[Peter Klausler via Phabricator via llvm-commits]

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

Looks great to me!  Thanks for this work.  Please take a look at my minor comments.



================
Comment at: flang/lib/Semantics/canonicalize-acc.cpp:41
+private:
+  template <typename T> T *GetConstructIf(parser::ExecutionPartConstruct &x) {
+    if (auto *y{std::get_if<parser::ExecutableConstruct>(&x.u)}) {
----------------
Can be `const`, or (better) `static`, or (best) in parser/tools.h.


================
Comment at: flang/lib/Semantics/check-acc-structure.cpp:86
+
+void AccStructureChecker::Enter(const parser::OpenACCConstruct &) { return; }
+
----------------
If these are placeholders, please add a comment; otherwise, consider removing them.


================
Comment at: flang/lib/Semantics/check-acc-structure.cpp:114
+  case llvm::acc::Directive::ACCD_kernels:
+  case llvm::acc::Directive::ACCD_parallel: {
+    // Restriction - 880-881 (KERNELS)
----------------
The nested scopes here aren't necessary.


================
Comment at: flang/lib/Semantics/check-acc-structure.cpp:211
+  const auto &loopDir{std::get<parser::AccLoopDirective>(beginDir.t)};
+  switch (loopDir.v) {
+  case llvm::acc::Directive::ACCD_loop: {
----------------
Perhaps this would be more readable as an `if` statement.


================
Comment at: flang/lib/Semantics/check-acc-structure.cpp:448
+
+/**
+ * Enforce restriction where clauses in the given set are not allowed if the
----------------
// comments are preferred


================
Comment at: flang/lib/Semantics/check-acc-structure.cpp:514
+} // namespace Fortran::semantics
\ No newline at end of file

----------------
Please add a newline to the end of this source file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83807