[flang-commits] [flang] [flang][OpenMP] Implement flexible OpenMP clause representation (PR #81621)

[Sergio Afonso via flang-commits]

================
@@ -142,6 +142,1122 @@ static void genNestedEvaluations(Fortran::lower::AbstractConverter &converter,
     converter.genEval(e);
 }
 
+//===----------------------------------------------------------------------===//
+// Clauses
+//===----------------------------------------------------------------------===//
+
+namespace detail {
+template <typename C> //
+llvm::omp::Clause getClauseIdForClass(C &&) {
+  using namespace Fortran;
+  using A = llvm::remove_cvref_t<C>; // A is referenced in OMP.inc
+  // The code included below contains a sequence of checks like the following
+  // for each OpenMP clause
+  //   if constexpr (std::is_same_v<A, parser::OmpClause::AcqRel>)
+  //     return llvm::omp::Clause::OMPC_acq_rel;
+  //   [...]
+#define GEN_FLANG_CLAUSE_PARSER_KIND_MAP
+#include "llvm/Frontend/OpenMP/OMP.inc"
+}
+} // namespace detail
+
+static llvm::omp::Clause getClauseId(const Fortran::parser::OmpClause &clause) {
+  return std::visit([](auto &&s) { return detail::getClauseIdForClass(s); },
+                    clause.u);
+}
+
+namespace omp {
+using namespace Fortran;
+using SomeType = evaluate::SomeType;
+using SomeExpr = semantics::SomeExpr;
+using MaybeExpr = semantics::MaybeExpr;
+
+template <typename T> //
+using List = std::vector<T>;
+
+struct SymDsgExtractor {
+  using SymDsg = std::tuple<semantics::Symbol *, MaybeExpr>;
+
+  template <typename T> //
+  static T &&AsRvalueRef(T &&t) {
+    return std::move(t);
+  }
+  template <typename T> //
+  static T AsRvalueRef(const T &t) {
+    return t;
+  }
+
+  template <typename T> //
+  static SymDsg visit(T &&) {
+    // Use this to see missing overloads:
+    // llvm::errs() << "NULL: " << __PRETTY_FUNCTION__ << '\n';
+    return SymDsg{};
+  }
+
+  template <typename T> //
+  static SymDsg visit(const evaluate::Designator<T> &e) {
+    // Symbols cannot be created after semantic checks, so all symbol
+    // pointers that are non-null must point to one of those pre-existing
+    // objects. Throughout the code, symbols are often pointed to by
+    // non-const pointers, so there is no harm in casting the constness
+    // away.
+    return std::make_tuple(const_cast<semantics::Symbol *>(e.GetLastSymbol()),
+                           evaluate::AsGenericExpr(AsRvalueRef(e)));
+  }
+
+  static SymDsg visit(const evaluate::ProcedureDesignator &e) {
+    // See comment above regarding const_cast.
+    return std::make_tuple(const_cast<semantics::Symbol *>(e.GetSymbol()),
+                           std::nullopt);
+  }
+
+  template <typename T> //
+  static SymDsg visit(const evaluate::Expr<T> &e) {
+    return std::visit([](auto &&s) { return visit(s); }, e.u);
+  }
+
+  static bool verify(const SymDsg &sd) {
----------------
skatrak wrote:

What is the purpose of this function? By returning `bool` it seems to imply that it checks that its argument is valid and then the result is checked by the caller at runtime to use it or emit an error. But instead, this either returns `true` or asserts. If it's only expected to verify things that must always be true or crash, maybe it doesn't make sense to return any values. Otherwise, it should be able to return `false` and callers should handle that case.

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