[PATCH] D87906: [Flang][OpenACC] Fix for branching out issue in OpenACC parallel construct.

[Tim Keith via Phabricator via llvm-commits]

tskeith added inline comments.


================
Comment at: flang/include/flang/Semantics/tools.h:531
 
+template <typename A> const parser::Name *MaybeGetStmtName(const A &a);
+template <typename A> const parser::Name *MaybeGetConstructName(const A &a);
----------------
Is this used?


================
Comment at: flang/include/flang/Semantics/tools.h:534
+const parser::Name *MaybeGetConstructName(
+    const parser::BlockConstruct &blockConstruct);
+const parser::Name *MaybeGetNodeName(const ConstructNode &construct);
----------------
It looks like the only place that MaybeGetConstructName is used is in MaybeGetNodeName, so how about just doing it inline there? I don't see a benefit of making it part of the API of tools.h.


================
Comment at: flang/lib/Semantics/check-acc-structure.cpp:84
 
+  void emitBranchOutErrorWithLabel(
+      const char *stmt, const parser::Name &toLabel) const {
----------------
Function names should start with a capital letter.


================
Comment at: flang/lib/Semantics/check-acc-structure.cpp:92
+            "%s to a label '%s' outside of %s construct is not allowed"_err_en_US,
+            stmt, labelName, upperCaseConstructName)
+        .Attach(sourcePosition_, GetEnclosingMsg());
----------------
That is not a label, it's a construct name. It confuses the user and the code to call things labels that are not.


================
Comment at: flang/lib/Semantics/check-acc-structure.cpp:102
+      return false;
+    }
+  }
----------------
`return stmtName.source == constructName.source;`

This is probably better done inline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87906