Add further tests, fix comments

Author: Leporacanthicus

flang/lib/Semantics/resolve-names.cpp

@@ -1470,7 +1470,6 @@ class OmpVisitor : public virtual DeclarationVisitor {
   }
 
   bool Pre(const parser::OpenMPDeclareMapperConstruct &);
-  void Post(const parser::OpenMPDeclareMapperConstruct &) { PopScope(); };
 
   void Post(const parser::OmpBeginLoopDirective &) {
     messageHandler().set_currStmtSource(std::nullopt);
@@ -1609,10 +1608,10 @@ 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.
+// This "manually" walks the tree of the construct, because we need
+// to resolve the type before the map clauses are processed - when
+// just following the natural flow, the map clauses gets processed before
+// the type has been fully processed.
 bool OmpVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) {
   AddOmpSourceRange(x.source);
   BeginDeclTypeSpec();
@@ -1623,8 +1622,9 @@ bool OmpVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) {
         &MakeSymbol(*mapperName, MiscDetails{MiscDetails::Kind::ConstructName});
     mapperName->symbol = mapperSym;
   } else {
+    const parser::CharBlock defaultName{"default", 7};
     mapperSym = &MakeSymbol(
-        "default", Attrs{}, MiscDetails{MiscDetails::Kind::ConstructName});
+        defaultName, Attrs{}, MiscDetails{MiscDetails::Kind::ConstructName});
   }
 
   PushScope(Scope::Kind::OtherConstruct, nullptr);
@@ -1635,6 +1635,7 @@ bool OmpVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) {
   Walk(std::get<parser::OmpClauseList>(x.t));
 
   EndDeclTypeSpec();
+  PopScope();
   return false;
 }
 

flang/lib/Semantics/unparse-with-symbols.cpp

@@ -53,6 +53,14 @@ class SymbolDumpVisitor {
   void Post(const parser::OpenMPThreadprivate &) { currStmt_ = std::nullopt; }
   void Post(const parser::Name &name);
 
+  bool Pre(const parser::OpenMPDeclareMapperConstruct &x) {
+    currStmt_ = x.source;
+    return true;
+  }
+  void Post(const parser::OpenMPDeclareMapperConstruct &) {
+    currStmt_ = std::nullopt;
+  }
+
 private:
   std::optional<SourceName> currStmt_; // current statement we are processing
   std::multimap<const char *, const Symbol *> symbols_; // location to symbol

flang/test/Parser/OpenMP/declare-mapper-unparse.f90

@@ -24,5 +24,19 @@ program main
 !PARSE-TREE:           DataRef -> Name = 'mapped'
 !PARSE-TREE:           Name = 'x'  
 
+!CHECK: !$OMP DECLARE MAPPER (ty::mapped) MAP(mapped,mapped%x)
+  !$omp declare mapper(ty :: mapped) map(mapped, mapped%x)
+  
+!PARSE-TREE:      OpenMPDeclareMapperConstruct
+!PARSE-TREE:        OmpDeclareMapperSpecifier
+!PARSE-TREE:         TypeSpec -> DerivedTypeSpec
+!PARSE-TREE:           Name = 'ty'
+!PARSE-TREE:         Name = 'mapped'    
+!PARSE-TREE:        OmpMapClause
+!PARSE-TREE:          OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'mapped'
+!PARSE-TREE:          OmpObject -> Designator -> DataRef -> StructureComponent
+!PARSE-TREE:           DataRef -> Name = 'mapped'
+!PARSE-TREE:           Name = 'x'
+  
 end program main
 !CHECK-LABEL: end program main

flang/test/Semantics/OpenMP/declare-mapper-symbols.f90

@@ -0,0 +1,24 @@
+! RUN: %flang_fc1 -fdebug-dump-symbols -fopenmp -fopenmp-version=50 %s | FileCheck %s
+
+program main
+!CHECK-LABEL: MainProgram scope: main
+  implicit none
+
+  type ty
+     integer :: x
+  end type ty
+  !$omp declare mapper(mymapper : ty :: mapped) map(mapped, mapped%x)
+  !$omp declare mapper(ty :: maptwo) map(maptwo, maptwo%x)
+
+!! Note, symbols come out in their respective scope, but not in declaration order.
+!CHECK: default: Misc ConstructName
+!CHECK: mymapper: Misc ConstructName
+!CHECK: ty: DerivedType components: x
+!CHECK: DerivedType scope: ty
+!CHECK: OtherConstruct scope:
+!CHECK: mapped (OmpMapToFrom) {{.*}} ObjectEntity type: TYPE(ty)
+!CHECK: OtherConstruct scope:  
+!CHECK: maptwo (OmpMapToFrom) {{.*}} ObjectEntity type: TYPE(ty)
+  
+end program main
+

flang/test/Semantics/OpenMP/declare-mapper01.f90

@@ -1,5 +1,5 @@
 ! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=50
-  ! Test the source code starting with omp syntax
+! Test the declare mapper with non-derived type.
 
 integer :: y
 

flang/test/Semantics/OpenMP/declare-mapper02.f90

@@ -1,5 +1,5 @@
 ! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=50
-! Test the source code starting with omp syntax
+! Test the declare mapper construct with abstract type.
 
 type, abstract :: t1
    integer :: y

flang/test/Semantics/OpenMP/declare-mapper03.f90

@@ -0,0 +1,16 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=50
+! Test the declare mapper construct with two default mappers.
+
+type :: t1
+   integer :: y
+end type t1
+
+type :: t2
+   real :: y, z
+end type t2
+
+!error: 'default' is already declared in this scoping unit
+
+!$omp declare mapper(t1::x) map(x, x%y)
+!$omp declare mapper(t2::w) map(w, w%y, w%z)
+end