[flang-commits] [flang] [llvm] [flang][OpenMP]Add support for fail clause (PR #118683)

[Mats Petersson via flang-commits]

================
@@ -114,6 +114,11 @@ class SemanticsVisitor : public virtual BaseChecker, public virtual C... {
     context_.set_location(std::nullopt);
   }
 
+  // This is necessary to avoid "walking" into the Fail clause,
+  // which confuses the CheckAllowed into thinking there's another
+  // memoryorder, when it's actually the argument to the fail clause.
+  bool Pre(const parser::OmpFailClause &) { return false; }
----------------
Leporacanthicus wrote:

Assuming we have the correct parser, there is no way that the FAIL clause will contain anything other than a MemoryOrder. 

The problem we're trying to solve here is that the default is to `Walk` everything where `Pre` returns true.  So if we have `!$OMP ATOMIC COMPARE SEQ_CST FAIL(RELAXED)`, it will get the the FAIL, and default `Pre` returns true, so it gets to process the RELAXED. It treats that as if it was part of the regular list of memory order objects listed, and says "There's two memory-orders on the same ATOMIC COMPARE" (not the precise wording). 

We COULD use a different type, and then have a separate checker bit, but than we will also end up with two (almost) identical parsers for memory order objects, and I'm not sure exactly what it solves to do that. 

We can always call some function inside `Pre` that checks things. Or call check the fail clause in the check-omp-structure.cpp. 

Really, none of the solutions are fantastic. This works for the moment, and I'm not sure it makes sense to add more complexity, unless we have a good reason for that.

https://github.com/llvm/llvm-project/pull/118683