Address reviewer comments. Add test to check declare mapper export to .mod file.

TIFitis · TIFitis · commit 672eb0ef3605 · 2025-10-30T14:57:29.000Z
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -4261,6 +4261,14 @@ void Fortran::lower::materializeOpenMPDeclareMappers(
   if (!root.IsModule())
     return;
 
+  // Only materialize for modules coming from mod files to avoid duplicates.
+  if (const semantics::Symbol *modSym = root.symbol()) {
+    if (!modSym->test(semantics::Symbol::Flag::ModFile))
+      return;
+  } else {
+    return;
+  }
+
   // Scan symbols in this module scope for MapperDetails.
   for (auto &it : root) {
     const semantics::Symbol &sym = *it.second;
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
@@ -2381,7 +2381,7 @@ void AttrsVisitor::SetBindNameOn(Symbol &symbol) {
   }
   symbol.SetBindName(std::move(*label));
   if (!oldBindName.empty()) {
-    if (const std::string *newBindName{symbol.GetBindName()}) {
+    if (const std::string * newBindName{symbol.GetBindName()}) {
       if (oldBindName != *newBindName) {
         Say(symbol.name(),
             "The entity '%s' has multiple BIND names ('%s' and '%s')"_err_en_US,
@@ -2507,15 +2507,15 @@ void DeclTypeSpecVisitor::Post(const parser::TypeSpec &typeSpec) {
   // expression semantics if the DeclTypeSpec is a valid TypeSpec.
   // The grammar ensures that it's an intrinsic or derived type spec,
   // not TYPE(*) or CLASS(*) or CLASS(T).
-  if (const DeclTypeSpec *spec{state_.declTypeSpec}) {
+  if (const DeclTypeSpec * spec{state_.declTypeSpec}) {
     switch (spec->category()) {
     case DeclTypeSpec::Numeric:
     case DeclTypeSpec::Logical:
     case DeclTypeSpec::Character:
       typeSpec.declTypeSpec = spec;
       break;
     case DeclTypeSpec::TypeDerived:
-      if (const DerivedTypeSpec *derived{spec->AsDerived()}) {
+      if (const DerivedTypeSpec * derived{spec->AsDerived()}) {
         CheckForAbstractType(derived->typeSymbol()); // C703
         typeSpec.declTypeSpec = spec;
       }
@@ -3106,8 +3106,8 @@ Symbol &ScopeHandler::MakeSymbol(const parser::Name &name, Attrs attrs) {
 Symbol &ScopeHandler::MakeHostAssocSymbol(
     const parser::Name &name, const Symbol &hostSymbol) {
   Symbol &symbol{*NonDerivedTypeScope()
-          .try_emplace(name.source, HostAssocDetails{hostSymbol})
-          .first->second};
+                      .try_emplace(name.source, HostAssocDetails{hostSymbol})
+                      .first->second};
   name.symbol = &symbol;
   symbol.attrs() = hostSymbol.attrs(); // TODO: except PRIVATE, PUBLIC?
   // These attributes can be redundantly reapplied without error
@@ -3195,7 +3195,7 @@ void ScopeHandler::ApplyImplicitRules(
   if (context().HasError(symbol) || !NeedsType(symbol)) {
     return;
   }
-  if (const DeclTypeSpec *type{GetImplicitType(symbol)}) {
+  if (const DeclTypeSpec * type{GetImplicitType(symbol)}) {
     if (!skipImplicitTyping_) {
       symbol.set(Symbol::Flag::Implicit);
       symbol.SetType(*type);
@@ -3295,7 +3295,7 @@ const DeclTypeSpec *ScopeHandler::GetImplicitType(
   const auto *type{implicitRulesMap_->at(scope).GetType(
       symbol.name(), respectImplicitNoneType)};
   if (type) {
-    if (const DerivedTypeSpec *derived{type->AsDerived()}) {
+    if (const DerivedTypeSpec * derived{type->AsDerived()}) {
       // Resolve any forward-referenced derived type; a quick no-op else.
       auto &instantiatable{*const_cast<DerivedTypeSpec *>(derived)};
       instantiatable.Instantiate(currScope());
@@ -4320,7 +4320,7 @@ Scope *ModuleVisitor::FindModule(const parser::Name &name,
   if (scope) {
     if (DoesScopeContain(scope, currScope())) { // 14.2.2(1)
       std::optional<SourceName> submoduleName;
-      if (const Scope *container{FindModuleOrSubmoduleContaining(currScope())};
+      if (const Scope * container{FindModuleOrSubmoduleContaining(currScope())};
           container && container->IsSubmodule()) {
         submoduleName = container->GetName();
       }
@@ -4425,7 +4425,7 @@ bool InterfaceVisitor::isAbstract() const {
 
 void InterfaceVisitor::AddSpecificProcs(
     const std::list<parser::Name> &names, ProcedureKind kind) {
-  if (Symbol *symbol{GetGenericInfo().symbol};
+  if (Symbol * symbol{GetGenericInfo().symbol};
       symbol && symbol->has<GenericDetails>()) {
     for (const auto &name : names) {
       specificsForGenericProcs_.emplace(symbol, std::make_pair(&name, kind));
@@ -4525,7 +4525,7 @@ void GenericHandler::DeclaredPossibleSpecificProc(Symbol &proc) {
 }
 
 void InterfaceVisitor::ResolveNewSpecifics() {
-  if (Symbol *generic{genericInfo_.top().symbol};
+  if (Symbol * generic{genericInfo_.top().symbol};
       generic && generic->has<GenericDetails>()) {
     ResolveSpecificsInGeneric(*generic, false);
   }
@@ -4610,7 +4610,7 @@ bool SubprogramVisitor::HandleStmtFunction(const parser::StmtFunctionStmt &x) {
           name.source);
       MakeSymbol(name, Attrs{}, UnknownDetails{});
     } else if (auto *entity{ultimate.detailsIf<EntityDetails>()};
-        entity && !ultimate.has<ProcEntityDetails>()) {
+               entity && !ultimate.has<ProcEntityDetails>()) {
       resultType = entity->type();
       ultimate.details() = UnknownDetails{}; // will be replaced below
     } else {
@@ -4669,7 +4669,7 @@ bool SubprogramVisitor::Pre(const parser::Suffix &suffix) {
     } else {
       Message &msg{Say(*suffix.resultName,
           "RESULT(%s) may appear only in a function"_err_en_US)};
-      if (const Symbol *subprogram{InclusiveScope().symbol()}) {
+      if (const Symbol * subprogram{InclusiveScope().symbol()}) {
         msg.Attach(subprogram->name(), "Containing subprogram"_en_US);
       }
     }
@@ -5185,7 +5185,7 @@ Symbol *ScopeHandler::FindSeparateModuleProcedureInterface(
       symbol = generic->specific();
     }
   }
-  if (const Symbol *defnIface{FindSeparateModuleSubprogramInterface(symbol)}) {
+  if (const Symbol * defnIface{FindSeparateModuleSubprogramInterface(symbol)}) {
     // Error recovery in case of multiple definitions
     symbol = const_cast<Symbol *>(defnIface);
   }
@@ -5324,8 +5324,8 @@ bool SubprogramVisitor::HandlePreviousCalls(
     return generic->specific() &&
         HandlePreviousCalls(name, *generic->specific(), subpFlag);
   } else if (const auto *proc{symbol.detailsIf<ProcEntityDetails>()}; proc &&
-      !proc->isDummy() &&
-      !symbol.attrs().HasAny(Attrs{Attr::INTRINSIC, Attr::POINTER})) {
+             !proc->isDummy() &&
+             !symbol.attrs().HasAny(Attrs{Attr::INTRINSIC, Attr::POINTER})) {
     // There's a symbol created for previous calls to this subprogram or
     // ENTRY's name.  We have to replace that symbol in situ to avoid the
     // obligation to rewrite symbol pointers in the parse tree.
@@ -5367,7 +5367,7 @@ const Symbol *SubprogramVisitor::CheckExtantProc(
   if (prev) {
     if (IsDummy(*prev)) {
     } else if (auto *entity{prev->detailsIf<EntityDetails>()};
-        IsPointer(*prev) && entity && !entity->type()) {
+               IsPointer(*prev) && entity && !entity->type()) {
       // POINTER attribute set before interface
     } else if (inInterfaceBlock() && currScope() != prev->owner()) {
       // Procedures in an INTERFACE block do not resolve to symbols
@@ -5439,7 +5439,7 @@ Symbol *SubprogramVisitor::PushSubprogramScope(const parser::Name &name,
     }
     set_inheritFromParent(false); // interfaces don't inherit, even if MODULE
   }
-  if (Symbol *found{FindSymbol(name)};
+  if (Symbol * found{FindSymbol(name)};
       found && found->has<HostAssocDetails>()) {
     found->set(subpFlag); // PushScope() created symbol
   }
@@ -6290,9 +6290,9 @@ void DeclarationVisitor::Post(const parser::VectorTypeSpec &x) {
     vectorDerivedType.CookParameters(GetFoldingContext());
   }
 
-  if (const DeclTypeSpec *extant{
-          ppcBuiltinTypesScope->FindInstantiatedDerivedType(
-              vectorDerivedType, DeclTypeSpec::Category::TypeDerived)}) {
+  if (const DeclTypeSpec *
+      extant{ppcBuiltinTypesScope->FindInstantiatedDerivedType(
+          vectorDerivedType, DeclTypeSpec::Category::TypeDerived)}) {
     // This derived type and parameter expressions (if any) are already present
     // in the __ppc_intrinsics scope.
     SetDeclTypeSpec(*extant);
@@ -6314,7 +6314,7 @@ bool DeclarationVisitor::Pre(const parser::DeclarationTypeSpec::Type &) {
 
 void DeclarationVisitor::Post(const parser::DeclarationTypeSpec::Type &type) {
   const parser::Name &derivedName{std::get<parser::Name>(type.derived.t)};
-  if (const Symbol *derivedSymbol{derivedName.symbol}) {
+  if (const Symbol * derivedSymbol{derivedName.symbol}) {
     CheckForAbstractType(*derivedSymbol); // C706
   }
 }
@@ -6383,8 +6383,8 @@ void DeclarationVisitor::Post(const parser::DerivedTypeSpec &x) {
   if (!spec->MightBeParameterized()) {
     spec->EvaluateParameters(context());
   }
-  if (const DeclTypeSpec *extant{
-          currScope().FindInstantiatedDerivedType(*spec, category)}) {
+  if (const DeclTypeSpec *
+      extant{currScope().FindInstantiatedDerivedType(*spec, category)}) {
     // This derived type and parameter expressions (if any) are already present
     // in this scope.
     SetDeclTypeSpec(*extant);
@@ -6415,7 +6415,8 @@ void DeclarationVisitor::Post(const parser::DeclarationTypeSpec::Record &rec) {
   if (auto spec{ResolveDerivedType(typeName)}) {
     spec->CookParameters(GetFoldingContext());
     spec->EvaluateParameters(context());
-    if (const DeclTypeSpec *extant{currScope().FindInstantiatedDerivedType(
+    if (const DeclTypeSpec *
+        extant{currScope().FindInstantiatedDerivedType(
             *spec, DeclTypeSpec::TypeDerived)}) {
       SetDeclTypeSpec(*extant);
     } else {
@@ -7426,7 +7427,7 @@ bool DeclarationVisitor::PassesLocalityChecks(
         "Coarray '%s' not allowed in a %s locality-spec"_err_en_US, specName);
     return false;
   }
-  if (const DeclTypeSpec *type{symbol.GetType()}) {
+  if (const DeclTypeSpec * type{symbol.GetType()}) {
     if (type->IsPolymorphic() && IsDummy(symbol) && !IsPointer(symbol) &&
         !isReduce) { // F'2023 C1130
       SayWithDecl(name, symbol,
@@ -7653,7 +7654,7 @@ Symbol *DeclarationVisitor::NoteInterfaceName(const parser::Name &name) {
 }
 
 void DeclarationVisitor::CheckExplicitInterface(const parser::Name &name) {
-  if (const Symbol *symbol{name.symbol}) {
+  if (const Symbol * symbol{name.symbol}) {
     const Symbol &ultimate{symbol->GetUltimate()};
     if (!context().HasError(*symbol) && !context().HasError(ultimate) &&
         !BypassGeneric(ultimate).HasExplicitInterface()) {
@@ -7972,7 +7973,7 @@ bool ConstructVisitor::Pre(const parser::DataStmtValue &x) {
   auto &mutableData{const_cast<parser::DataStmtConstant &>(data)};
   if (auto *elem{parser::Unwrap<parser::ArrayElement>(mutableData)}) {
     if (const auto *name{std::get_if<parser::Name>(&elem->base.u)}) {
-      if (const Symbol *symbol{FindSymbol(*name)};
+      if (const Symbol * symbol{FindSymbol(*name)};
           symbol && symbol->GetUltimate().has<DerivedTypeDetails>()) {
         mutableData.u = elem->ConvertToStructureConstructor(
             DerivedTypeSpec{name->source, *symbol});
@@ -8118,15 +8119,15 @@ void ConstructVisitor::Post(const parser::SelectTypeStmt &x) {
       }
     }
   } else {
-    if (const Symbol *whole{
-            UnwrapWholeSymbolDataRef(association.selector.expr)}) {
+    if (const Symbol *
+        whole{UnwrapWholeSymbolDataRef(association.selector.expr)}) {
       ConvertToObjectEntity(const_cast<Symbol &>(*whole));
       if (!IsVariableName(*whole)) {
         Say(association.selector.source, // C901
             "Selector is not a variable"_err_en_US);
         association = {};
       }
-      if (const DeclTypeSpec *type{whole->GetType()}) {
+      if (const DeclTypeSpec * type{whole->GetType()}) {
         if (!type->IsPolymorphic()) { // C1159
           Say(association.selector.source,
               "Selector '%s' in SELECT TYPE statement must be "
@@ -8266,8 +8267,8 @@ Symbol *ConstructVisitor::MakeAssocEntity() {
           "The associate name '%s' is already used in this associate statement"_err_en_US);
       return nullptr;
     }
-  } else if (const Symbol *whole{
-                 UnwrapWholeSymbolDataRef(association.selector.expr)}) {
+  } else if (const Symbol *
+      whole{UnwrapWholeSymbolDataRef(association.selector.expr)}) {
     symbol = &MakeSymbol(whole->name());
   } else {
     return nullptr;
@@ -8887,7 +8888,7 @@ bool DeclarationVisitor::CheckForHostAssociatedImplicit(
   if (name.symbol) {
     ApplyImplicitRules(*name.symbol, true);
   }
-  if (Scope *host{GetHostProcedure()}; host && !isImplicitNoneType(*host)) {
+  if (Scope * host{GetHostProcedure()}; host && !isImplicitNoneType(*host)) {
     Symbol *hostSymbol{nullptr};
     if (!name.symbol) {
       if (currScope().CanImport(name.source)) {
@@ -8958,7 +8959,7 @@ const parser::Name *DeclarationVisitor::FindComponent(
   if (!type) {
     return nullptr; // should have already reported error
   }
-  if (const IntrinsicTypeSpec *intrinsic{type->AsIntrinsic()}) {
+  if (const IntrinsicTypeSpec * intrinsic{type->AsIntrinsic()}) {
     auto category{intrinsic->category()};
     MiscDetails::Kind miscKind{MiscDetails::Kind::None};
     if (component.source == "kind") {
@@ -8980,7 +8981,7 @@ const parser::Name *DeclarationVisitor::FindComponent(
     }
   } else if (DerivedTypeSpec * derived{type->AsDerived()}) {
     derived->Instantiate(currScope()); // in case of forward referenced type
-    if (const Scope *scope{derived->scope()}) {
+    if (const Scope * scope{derived->scope()}) {
       if (Resolve(component, scope->FindComponent(component.source))) {
         if (auto msg{CheckAccessibleSymbol(currScope(), *component.symbol)}) {
           context().Say(component.source, *msg);
@@ -9131,8 +9132,8 @@ void DeclarationVisitor::PointerInitialization(
             if (evaluate::IsNullProcedurePointer(&*expr)) {
               CHECK(!details->init());
               details->set_init(nullptr);
-            } else if (const Symbol *targetSymbol{
-                           evaluate::UnwrapWholeSymbolDataRef(*expr)}) {
+            } else if (const Symbol *
+                targetSymbol{evaluate::UnwrapWholeSymbolDataRef(*expr)}) {
               CHECK(!details->init());
               details->set_init(*targetSymbol);
             } else {
@@ -9701,7 +9702,7 @@ void ResolveNamesVisitor::EarlyDummyTypeDeclaration(
       for (const auto &ent : entities) {
         const auto &objName{std::get<parser::ObjectName>(ent.t)};
         Resolve(objName, FindInScope(currScope(), objName));
-        if (Symbol *symbol{objName.symbol};
+        if (Symbol * symbol{objName.symbol};
             symbol && IsDummy(*symbol) && NeedsType(*symbol)) {
           if (!type) {
             type = ProcessTypeSpec(declTypeSpec);
@@ -9840,7 +9841,7 @@ void ResolveNamesVisitor::FinishSpecificationPart(
     if (auto *proc{symbol.detailsIf<ProcEntityDetails>()}; proc &&
         !proc->isDummy() && !IsPointer(symbol) &&
         !symbol.attrs().test(Attr::BIND_C)) {
-      if (const Symbol *iface{proc->procInterface()};
+      if (const Symbol * iface{proc->procInterface()};
           iface && IsBindCProcedure(*iface)) {
         SetImplicitAttr(symbol, Attr::BIND_C);
         SetBindNameOn(symbol);
@@ -9973,7 +9974,7 @@ bool ResolveNamesVisitor::Pre(const parser::PointerAssignmentStmt &x) {
   Symbol *ptrSymbol{parser::GetLastName(dataRef).symbol};
   Walk(bounds);
   // Resolve unrestricted specific intrinsic procedures as in "p => cos".
-  if (const parser::Name *name{parser::Unwrap<parser::Name>(expr)}) {
+  if (const parser::Name * name{parser::Unwrap<parser::Name>(expr)}) {
     if (NameIsKnownOrIntrinsic(*name)) {
       if (Symbol * symbol{name->symbol}) {
         if (IsProcedurePointer(ptrSymbol) &&
@@ -10420,8 +10421,8 @@ void ResolveNamesVisitor::ResolveSpecificationParts(ProgramTree &node) {
     // implied SAVE so that evaluate::IsSaved() will return true.
     if (node.scope()->kind() == Scope::Kind::MainProgram) {
       if (const auto *object{symbol.detailsIf<ObjectEntityDetails>()}) {
-        if (const DeclTypeSpec *type{object->type()}) {
-          if (const DerivedTypeSpec *derived{type->AsDerived()}) {
+        if (const DeclTypeSpec * type{object->type()}) {
+          if (const DerivedTypeSpec * derived{type->AsDerived()}) {
             if (!IsSaved(symbol) && FindCoarrayPotentialComponent(*derived)) {
               SetImplicitAttr(symbol, Attr::SAVE);
             }
@@ -10678,7 +10679,7 @@ void ResolveNamesVisitor::FinishDerivedTypeInstantiation(Scope &scope) {
   if (DerivedTypeSpec * spec{scope.derivedTypeSpec()}) {
     spec->Instantiate(currScope());
     const Symbol &origTypeSymbol{spec->typeSymbol()};
-    if (const Scope *origTypeScope{origTypeSymbol.scope()}) {
+    if (const Scope * origTypeScope{origTypeSymbol.scope()}) {
       CHECK(origTypeScope->IsDerivedType() &&
           origTypeScope->symbol() == &origTypeSymbol);
       auto &foldingContext{GetFoldingContext()};
@@ -10689,7 +10690,7 @@ void ResolveNamesVisitor::FinishDerivedTypeInstantiation(Scope &scope) {
         if (IsPointer(comp)) {
           if (auto *details{comp.detailsIf<ObjectEntityDetails>()}) {
             auto origDetails{origComp.get<ObjectEntityDetails>()};
-            if (const MaybeExpr &init{origDetails.init()}) {
+            if (const MaybeExpr & init{origDetails.init()}) {
               SomeExpr newInit{*init};
               MaybeExpr folded{FoldExpr(std::move(newInit))};
               details->set_init(std::move(folded));
diff --git a/flang/test/Semantics/OpenMP/declare-mapper-modfile.f90 b/flang/test/Semantics/OpenMP/declare-mapper-modfile.f90
@@ -0,0 +1,14 @@
+! RUN: split-file %s %t
+! RUN: %flang_fc1 -fsyntax-only -fopenmp -fopenmp-version=50 -module-dir %t %t/m.f90
+! RUN: cat %t/m.mod | FileCheck --ignore-case %s
+
+!--- m.f90
+module m
+  implicit none
+  type :: t
+    integer :: x
+  end type t
+  !$omp declare mapper(mymap : t :: v) map(v%x)
+end module m
+
+!CHECK: !$OMP DECLARE MAPPER(mymap:t::v) MAP(v%x)
diff --git a/flang/test/Semantics/OpenMP/map-clause-symbols.f90 b/flang/test/Semantics/OpenMP/map-clause-symbols.f90
@@ -1,12 +1,16 @@
-! RUN: not %flang_fc1 -fopenmp -fopenmp-version=50 %s 2>&1 | FileCheck %s
+! RUN: %flang_fc1 -fdebug-dump-symbols -fopenmp -fopenmp-version=50 %s | FileCheck %s
 program main
+!CHECK-LABEL:  MainProgram scope: MAIN
+  type ty
+    real(4) :: x
+  end type ty
+  !$omp declare mapper(xx : ty :: v) map(v)
   integer, parameter :: n = 256
-  real(8) :: a(256)
+  type(ty) :: a(256)
   !$omp target map(mapper(xx), from:a)
   do i=1,n
-     a(i) = 4.2
+     a(i)%x = 4.2
   end do
   !$omp end target
+!CHECK:    xx: MapperDetails
 end program main
-
-! CHECK: error: '{{.*}}' not declared
