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

[Tim Keith via Phabricator via flang-commits]

tskeith added inline comments.


================
Comment at: flang/lib/Semantics/check-do-forall.cpp:920
+    if (StmtMatchesConstruct(
+            stmtName, stmtType, &(*constructName), construct)) {
       CheckDoConcurrentExit(stmtType, construct);
----------------
Can't `constructName` be nullopt?


================
Comment at: flang/lib/Semantics/tools.cpp:1337
+                    },
+      construct);
+}
----------------
sameeranjoshi wrote:
> tskeith wrote:
> > The call to `common::GetPtrFromOptional` could be moved outside of `std::visit` so that it doesn't have to be repeated in each lambda.
> > 
> > Even better might be to change the return type to `const <std::optional<parser::Name>> &`. Is there any reason to convert it to a pointer?
> My knowledge of modern cpp seemed to pull me back.
> I tried various different ways to return `const std::optional<parser::Name> &`, all resulting in either seg-fault or unknown regressions in fortran loops.
> I have managed to do this change using a `return-by-value` and not `return-by-reference`.
> If these changes still need to be modified, some suggestions on using a return-by-reference would be helpful.
You may need to provide return types for the lambdas so that std::visit returns a reference and not a value. Like this:
```
  return std::visit(
      common::visitors{
          [&](const parser::BlockConstruct *blockConstruct)
              -> const std::optional<parser::Name> & {
            return std::get<0>(blockConstruct->t).statement.v;
          },
          [&](const auto *a) -> const std::optional<parser::Name> & {
            return std::get<0>(std::get<0>(a->t).statement.t);
          },
      },
      construct);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87906