[flang-commits] [PATCH] D89879: [Flang][OpenMP-5.0] Semantic checks for flush construct.

[Valentin Clement via Phabricator via flang-commits]

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

LGTM. Please wait for the approval from someone from the OpenMP side.



================
Comment at: flang/test/Semantics/omp-flush01.f90:28
+    !ERROR: expected end of line
+    !$omp flush acq_rel acq_rel
+    !ERROR: expected end of line
----------------
sameeranjoshi wrote:
> clementval wrote:
> > sameeranjoshi wrote:
> > > clementval wrote:
> > > > Why is the error reported twice here? Would it not be nicer to have a list of `memory-order-clause` and then the error being reported by the semantic check? I mean it would be more user friendly. 
> > > It's a known issue due the improper error recovery in openmp.
> > > omp-atomic.f90 file too had to get a workaround using the same way.
> > > 
> > > Yes, I have forwarded the errors to semantic phase.
> > If you have forwarded the errors to semantic then why is there still two syntax error lines? 
> The errors forwarded to semantics are something like
> `At most one XYZ clause can appear on the FLUSH directive`.
> Generally the ones which don't have clauses apart from `memory-order-clause`.
> 
> The test cases over here have clauses which don't fall under `memory-order-clause`, hence the parser becomes very stricter over here and catches them in parsing stage.
Ok I see. It would be nice to fix the double reporting of error here but I guess it can be done in a separate patch. Can you add an entry in the progress document to not forget this issue. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89879