[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);
----------------
sameeranjoshi wrote:
> tskeith wrote:
> > Can't `constructName` be nullopt?
> I don't think we need to handle it here, because I see it's handled inside `StmtMatchesConstruct` at 
> https://github.com/llvm/llvm-project/blob/551caec4a8af79483823e2940d40afb4c1df5da1/flang/lib/Semantics/check-do-forall.cpp#L918
> 
> I also tried to use the conventional approach of using a guard for std::optional.
> ```
> if (constructName){
> // Accessing the std::optional value by dereferencing it.
> 
> }
> ```
> But I see regressions due to `CheckDoConcurrentExit`, which seem to have a dependency on the result from `StmtMatchesConstruct`.
> Please correct me if I am wrong here.
It's too late to handle it in `StmtMatchesConstruct`. It's dereferenced before `StmtMatchesConstruct` is called. `*constructName` is undefined if `constructName` is nullopt.

It would simplify things if that argument were an optional rather than a pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87906