Review updates

Author: Leporacanthicus

flang/include/flang/Parser/parse-tree.h

@@ -3880,9 +3880,7 @@ struct OmpDeclareMapperSpecifier {
 struct OpenMPDeclareMapperConstruct {
   TUPLE_CLASS_BOILERPLATE(OpenMPDeclareMapperConstruct);
   CharBlock source;
-  std::tuple<Verbatim, OmpDeclareMapperSpecifier,
-      std::list<std::list<OmpMapClause>>>
-      t;
+  std::tuple<Verbatim, OmpDeclareMapperSpecifier, OmpClauseList> t;
 };
 
 // 2.16 declare-reduction -> DECLARE REDUCTION (reduction-identifier : type-list
@@ -3935,10 +3933,10 @@ struct OpenMPDeclarativeAllocate {
 struct OpenMPDeclarativeConstruct {
   UNION_CLASS_BOILERPLATE(OpenMPDeclarativeConstruct);
   CharBlock source;
-  std::variant<OpenMPDeclarativeAllocate, OpenMPDeclareReductionConstruct,
-      OpenMPDeclareSimdConstruct, OpenMPDeclareTargetConstruct,
-      OpenMPThreadprivate, OpenMPRequiresConstruct,
-      OpenMPDeclareMapperConstruct>
+  std::variant<OpenMPDeclarativeAllocate, OpenMPDeclareMapperConstruct,
+      OpenMPDeclareReductionConstruct, OpenMPDeclareSimdConstruct,
+      OpenMPDeclareTargetConstruct, OpenMPThreadprivate,
+      OpenMPRequiresConstruct>
       u;
 };
 

flang/lib/Parser/openmp-parsers.cpp

@@ -840,11 +840,10 @@ TYPE_PARSER(sourced(construct<OpenMPDeclareTargetConstruct>(
 TYPE_PARSER(construct<OmpDeclareMapperSpecifier>(
     maybe(name / ":" / !":"_tok), typeSpec / "::", name))
 
-// ?.? (not 4.5) Declare Mapper Construct
-TYPE_PARSER(sourced(
-    construct<OpenMPDeclareMapperConstruct>(verbatim("DECLARE MAPPER"_tok),
-        "(" >> Parser<OmpDeclareMapperSpecifier>{} / ")",
-        many("MAP" >> parenthesized(many(Parser<OmpMapClause>{}))))))
+// OpenMP 5.2: 5.8.8 Declare Mapper Construct
+TYPE_PARSER(sourced(construct<OpenMPDeclareMapperConstruct>(
+    verbatim("DECLARE MAPPER"_tok),
+    "(" >> Parser<OmpDeclareMapperSpecifier>{} / ")", Parser<OmpClauseList>{})))
 
 TYPE_PARSER(construct<OmpReductionCombiner>(Parser<AssignmentStmt>{}) ||
     construct<OmpReductionCombiner>(

flang/lib/Parser/unparse.cpp

@@ -2677,14 +2677,7 @@ class UnparseVisitor {
               Walk(std::get<Name>(spec.t));
               Put(")");
 
-              const auto &list{
-                  std::get<std::list<std::list<Fortran::parser::OmpMapClause>>>(
-                      z.t)};
-              for (const auto &m : list) {
-                Put(" MAP(");
-                Walk(m);
-                Put(")");
-              }
+              Walk(std::get<OmpClauseList>(z.t));
               Put("\n");
               return false;
             },

flang/lib/Semantics/check-omp-structure.cpp

@@ -1434,6 +1434,26 @@ void OmpStructureChecker::Leave(const parser::OmpDeclareTargetWithClause &x) {
   }
 }
 
+void OmpStructureChecker::Enter(const parser::OpenMPDeclareMapperConstruct &x) {
+  const auto &dir{std::get<parser::Verbatim>(x.t)};
+  PushContextAndClauseSets(
+      dir.source, llvm::omp::Directive::OMPD_declare_mapper);
+  const auto &spec{std::get<parser::OmpDeclareMapperSpecifier>(x.t)};
+  const auto &type = std::get<parser::TypeSpec>(spec.t);
+  if (const auto derivedTypeSpec{
+          std::get_if<parser::DerivedTypeSpec>(&type.u)}) {
+    if (derivedTypeSpec->derivedTypeSpec->typeSymbol().attrs().test(
+            Attr::ABSTRACT))
+      context_.Say(dir.source, "Type must not be abstract"_err_en_US);
+  } else {
+    context_.Say(dir.source, "Type is not a derived type"_err_en_US);
+  }
+}
+
+void OmpStructureChecker::Leave(const parser::OpenMPDeclareMapperConstruct &) {
+  dirContext_.pop_back();
+}
+
 void OmpStructureChecker::Enter(const parser::OpenMPDeclareTargetConstruct &x) {
   const auto &dir{std::get<parser::Verbatim>(x.t)};
   PushContext(dir.source, llvm::omp::Directive::OMPD_declare_target);

flang/lib/Semantics/check-omp-structure.h

@@ -90,6 +90,8 @@ class OmpStructureChecker
   void Leave(const parser::OpenMPDeclareSimdConstruct &);
   void Enter(const parser::OpenMPDeclarativeAllocate &);
   void Leave(const parser::OpenMPDeclarativeAllocate &);
+  void Enter(const parser::OpenMPDeclareMapperConstruct &);
+  void Leave(const parser::OpenMPDeclareMapperConstruct &);
   void Enter(const parser::OpenMPDeclareTargetConstruct &);
   void Leave(const parser::OpenMPDeclareTargetConstruct &);
   void Enter(const parser::OpenMPDepobjConstruct &);

flang/lib/Semantics/resolve-directives.cpp

@@ -1949,7 +1949,6 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPDeclareTargetConstruct &x) {
 
 bool OmpAttributeVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) {
   PushContext(x.source, llvm::omp::Directive::OMPD_declare_mapper);
-
   return true;
 }
 

flang/lib/Semantics/resolve-names.cpp

@@ -1058,7 +1058,6 @@ class DeclarationVisitor : public ArraySpecVisitor,
   const parser::Name *ResolveDesignator(const parser::Designator &);
   int GetVectorElementKind(
       TypeCategory category, const std::optional<parser::KindSelector> &kind);
-  std::optional<DerivedTypeSpec> ResolveDerivedType(const parser::Name &);
 
 protected:
   bool BeginDecl();
@@ -1206,6 +1205,7 @@ class DeclarationVisitor : public ArraySpecVisitor,
   Symbol &DeclareProcEntity(
       const parser::Name &, Attrs, const Symbol *interface);
   void SetType(const parser::Name &, const DeclTypeSpec &);
+  std::optional<DerivedTypeSpec> ResolveDerivedType(const parser::Name &);
   std::optional<DerivedTypeSpec> ResolveExtendsType(
       const parser::Name &, const parser::Name *);
   Symbol *MakeTypeSymbol(const SourceName &, Details &&);
@@ -1609,21 +1609,26 @@ void OmpVisitor::Post(const parser::OpenMPBlockConstruct &x) {
   }
 }
 
+// This "manually" walks the tree of the cosntruct, because the order
+// elements are resolved by the normal visitor will try to resolve
+// the map clauses attached to the directive without having resolved
+// the type, so the type is figured out using the implicit rules.
 bool OmpVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) {
   AddOmpSourceRange(x.source);
   BeginDeclTypeSpec();
   PushScope(Scope::Kind::OtherConstruct, nullptr);
   const auto &spec{std::get<parser::OmpDeclareMapperSpecifier>(x.t)};
-  if (const auto &mapperName{
-          std::get<std::optional<Fortran::parser::Name>>(spec.t)}) {
+  if (const auto &mapperName{std::get<std::optional<parser::Name>>(spec.t)}) {
     Symbol *mapperSym{&MakeSymbol(*mapperName, Attrs{})};
     mapperName->symbol = mapperSym;
+  } else if (0) {
+    Symbol *mapperSym{&MakeSymbol("default", Attrs{})};
   }
-  Walk(std::get<Fortran::parser::TypeSpec>(spec.t));
-  const auto &varName{std::get<Fortran::parser::ObjectName>(spec.t)};
+  Walk(std::get<parser::TypeSpec>(spec.t));
+  const auto &varName{std::get<parser::ObjectName>(spec.t)};
   DeclareObjectEntity(varName);
 
-  Walk(std::get<std::list<std::list<Fortran::parser::OmpMapClause>>>(x.t));
+  Walk(std::get<parser::OmpClauseList>(x.t));
 
   EndDeclTypeSpec();
   return false;

flang/test/Lower/OpenMP/Todo/omp-declare-mapper.f90

@@ -1,8 +1,8 @@
 ! This test checks lowering of OpenMP declare mapper Directive.
 
 ! RUN: split-file %s %t
-! RUN: not %flang_fc1 -emit-fir -fopenmp %t/omp-declare-mapper-1.f90 2>&1 | FileCheck %t/omp-declare-mapper-1.f90
-! RUN  not %flang_fc1 -emit-fir -fopenmp %t/omp-declare-mapper-2.f90 2>&1 | FileCheck %t/omp-declare-mapper-2.f90
+! RUN: not %flang_fc1 -emit-fir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-1.f90 2>&1 | FileCheck %t/omp-declare-mapper-1.f90
+! RUN  not %flang_fc1 -emit-fir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-2.f90 2>&1 | FileCheck %t/omp-declare-mapper-2.f90
 
 !--- omp-declare-mapper-1.f90
 subroutine declare_mapper_1

llvm/include/llvm/Frontend/OpenMP/OMP.td

@@ -612,8 +612,8 @@ def OMP_Critical : Directive<"critical"> {
   let category = CA_Executable;
 }
 def OMP_DeclareMapper : Directive<"declare mapper"> {
-  let allowedClauses = [
-    VersionedClause<OMPC_Map>,
+  let requiredClauses = [
+    VersionedClause<OMPC_Map, 50>,
   ];
   let association = AS_None;
   let category = CA_Declarative;