[flang] Handle multiple USE statements for the same module

[PeteSteinfeld]

Summary

It's possible to have several USE statements for the same module that
have different mixes of rename clauses and ONLY clauses. The presence
of a rename cause has the effect of hiding a previously associated name,
and the presence of an ONLY clause forces the name to be visible even in
the presence of a rename. We were not correctly handling this hiding.

I fixed this by keeping track of the names that appear on rename and ONLY
clauses. Then, when processing the USE association of a name, I check to see
if it previously appeared in a rename clause and not in a USE clause. If so, I
remove its USE associated symbol. Also, when USE associating all of the names
in a module, I do not USE associate names that have appeared in rename clauses.

I also added a test.

[klausler]

flang/lib/Semantics/resolve-names.cpp

2530

Symbol::name() returns a `SourceName (== parser::CharBlock); how does this reference declaration even compile?

6457

Might be better in ResolveNamesVisitor::Pre(const parser::SpecificationPart &x) after (or before) the walk of the useStmts. FinishSpecificationPart() has multiple call sites and things might be more clear if you did this use-stmt clean-up (or set-up) at the one place that deals with the list of use-stmts.

[PeteSteinfeld]

flang/lib/Semantics/resolve-names.cpp

2530

Good catch. I can only think that the C++ compiler was constructing a parser::Name in place and then creating a reference to it. I'll fix it.

6457

Good idea. I'll make it happen.

[PeteSteinfeld]

Responding to Peter's comments.

flang/lib/Semantics/resolve-names.cpp

public:
   bool Pre(const parser::Rename::Operators &);
   bool Pre(const parser::UseStmt &);
   void Post(const parser::UseStmt &);
 
   void BeginModule(const parser::Name &, bool isSubmodule);
   bool BeginSubmodule(const parser::Name &, const parser::ParentIdentifier &);
   void ApplyDefaultAccess();
   void AddGenericUse(GenericDetails &, const SourceName &, const Symbol &);
+  void ClearUseRenames() { useRenames_.clear(); }
+  void ClearUseOnly() { useOnly_.clear(); }
 
 private:
   // The default access spec for this module.
   Attr defaultAccess_{Attr::PUBLIC};
   // The location of the last AccessStmt without access-ids, if any.
   std::optional<SourceName> prevAccessStmt_;
   // The scope of the module during a UseStmt
   Scope *useModuleScope_{nullptr};
+  // Names that have appeared in a rename clause of a USE statement
+  std::set<std::pair<SourceName, Scope *>> useRenames_;
+  // Names that have appeared in an ONLY clause of a USE statement
+  std::set<std::pair<SourceName, Scope *>> useOnly_;
 
   Symbol &SetAccess(const SourceName &, Attr attr, Symbol * = nullptr);
   // A rename in a USE statement: local => use
   struct SymbolRename {
     Symbol *local{nullptr};
     Symbol *use{nullptr};
   };
   // Record a use from useModuleScope_ of use Name/Symbol as local Name/Symbol
   SymbolRename AddUse(const SourceName &localName, const SourceName &useName);
   SymbolRename AddUse(const SourceName &, const SourceName &, Symbol *);
   void DoAddUse(const SourceName &, const SourceName &, Symbol &localSymbol,
       const Symbol &useSymbol);
   void AddUse(const GenericSpecInfo &);
+  // If appropriate, erase a previously USE-associated symbol
+  void EraseRenamedSymbol(const Symbol &);
+  // Record a name appearing in a USE rename clause
+  void AddUseRename(const SourceName &name) {
+    useRenames_.emplace(std::make_pair(name, useModuleScope_));
+  }
+  bool IsUseRenamed(const SourceName &name) const {
+    return useRenames_.find({name, useModuleScope_}) != useRenames_.end();
+  }
+  // Record a name appearing in a USE ONLY clause
+  void AddUseOnly(const SourceName &name) {
+    useOnly_.emplace(std::make_pair(name, useModuleScope_));
+  }
+  bool IsUseOnly(const SourceName &name) const {
+    return useOnly_.find({name, useModuleScope_}) != useOnly_.end();
+  }
   Scope *FindModule(const parser::Name &, Scope *ancestor = nullptr);
 };
 
 class InterfaceVisitor : public virtual ScopeHandler {
 public:
   bool Pre(const parser::InterfaceStmt &);
   void Post(const parser::InterfaceStmt &);
   void Post(const parser::EndInterfaceStmt &);
void ScopeHandler::MakeExternal(Symbol &symbol) {
   }
 }
 
 // ModuleVisitor implementation
 
 bool ModuleVisitor::Pre(const parser::Only &x) {
   std::visit(common::visitors{
                  [&](const Indirection<parser::GenericSpec> &generic) {
-                   AddUse(GenericSpecInfo{generic.value()});
+                   const GenericSpecInfo &genericSpecInfo{generic.value()};
+                   AddUseOnly(genericSpecInfo.symbolName());
+                   AddUse(genericSpecInfo);
                  },
                  [&](const parser::Name &name) {
+                   AddUseOnly(name.source);
                    Resolve(name, AddUse(name.source, name.source).use);
                  },
                  [&](const parser::Rename &rename) { Walk(rename); },
              },
       x.u);
   return false;
 }
 
 bool ModuleVisitor::Pre(const parser::Rename::Names &x) {
   const auto &localName{std::get<0>(x.t)};
   const auto &useName{std::get<1>(x.t)};
+  AddUseRename(useName.source);
   SymbolRename rename{AddUse(localName.source, useName.source)};
+  if (rename.use) {
+    EraseRenamedSymbol(*rename.use);
+  }
   Resolve(useName, rename.use);
   Resolve(localName, rename.local);
   return false;
 }
 bool ModuleVisitor::Pre(const parser::Rename::Operators &x) {
   const parser::DefinedOpName &local{std::get<0>(x.t)};
   const parser::DefinedOpName &use{std::get<1>(x.t)};
   GenericSpecInfo localInfo{local};
   GenericSpecInfo useInfo{use};
   if (IsIntrinsicOperator(context(), local.v.source)) {
     Say(local.v,
         "Intrinsic operator '%s' may not be used as a defined operator"_err_en_US);
   } else if (IsLogicalConstant(context(), local.v.source)) {
     Say(local.v,
         "Logical constant '%s' may not be used as a defined operator"_err_en_US);
   } else {
     SymbolRename rename{AddUse(localInfo.symbolName(), useInfo.symbolName())};
+    if (rename.use) {
+      EraseRenamedSymbol(*rename.use);
+    }
     useInfo.Resolve(rename.use);
     localInfo.Resolve(rename.local);
   }
   return false;
 }
 
 // Set useModuleScope_ to the Scope of the module being used.
 bool ModuleVisitor::Pre(const parser::UseStmt &x) {
for (const auto &rename : *list) {
                      },
                      [&](const parser::Rename::Operators &ops) {
                        useNames.insert(std::get<1>(ops.t).v.source);
                      },
                  },
           rename.u);
     }
     for (const auto &[name, symbol] : *useModuleScope_) {
-      if (symbol->attrs().test(Attr::PUBLIC) &&
+      if (symbol->attrs().test(Attr::PUBLIC) && !IsUseRenamed(symbol->name()) &&
           (!symbol->attrs().test(Attr::INTRINSIC) ||
               symbol->has<UseDetails>()) &&
           !symbol->has<MiscDetails>() && useNames.count(name) == 0) {
         SourceName location{x.moduleName.source};
         if (auto *localSymbol{FindInScope(name)}) {
           DoAddUse(location, localSymbol->name(), *localSymbol, *symbol);
         } else {
           DoAddUse(location, location, CopySymbol(name, *symbol), *symbol);
static void ConvertToUseError(
   if (!useDetails) {
     auto &genericDetails{symbol.get<GenericDetails>()};
     useDetails = &genericDetails.uses().at(0)->get<UseDetails>();
   }
   symbol.set_details(
       UseErrorDetails{*useDetails}.add_occurrence(location, module));
 }
 
+// If a symbol has previously been USE-associated and did not appear in a USE
+// ONLY clause, erase it from the current scope.  This is needed when a name
+// appears in a USE rename clause.
+void ModuleVisitor::EraseRenamedSymbol(const Symbol &useSymbol) {
+  const SourceName &name{useSymbol.name()};
+  if (const Symbol * symbol{FindInScope(name)}) {
+    if (auto *useDetails{symbol->detailsIf<UseDetails>()}) {
+      const Symbol &moduleSymbol{useDetails->symbol()};
+      if (moduleSymbol.name() == name &&
+          moduleSymbol.owner() == useSymbol.owner() && IsUseRenamed(name) &&
+          !IsUseOnly(name)) {
+        EraseSymbol(*symbol);
+      }
+    }
+  }
+}
+
 void ModuleVisitor::DoAddUse(const SourceName &location,
     const SourceName &localName, Symbol &localSymbol, const Symbol &useSymbol) {
+  if (localName != useSymbol.name()) {
+    EraseRenamedSymbol(useSymbol);
+  }
   localSymbol.attrs() = useSymbol.attrs() & ~Attrs{Attr::PUBLIC, Attr::PRIVATE};
   localSymbol.flags() = useSymbol.flags();
   const Symbol &useUltimate{useSymbol.GetUltimate()};
   if (auto *useDetails{localSymbol.detailsIf<UseDetails>()}) {
     const Symbol &localUltimate{localSymbol.GetUltimate()};
-    if (localUltimate == useUltimate) {
+    if (localUltimate.owner() == useUltimate.owner()) {
       // use-associating the same symbol again -- ok
     } else if (localUltimate.has<GenericDetails>() &&
         useUltimate.has<GenericDetails>()) {
       // use-associating generics with the same names: merge them into a
       // new generic in this scope
       auto generic1{localUltimate.get<GenericDetails>()};
       AddGenericUse(generic1, localName, useUltimate);
       generic1.AddUse(localSymbol);
if (name.symbol) {
     }
   }
 }
 
 void DeclarationVisitor::NonPointerInitialization(
     const parser::Name &name, const parser::ConstantExpr &expr) {
   if (name.symbol) {
     Symbol &ultimate{name.symbol->GetUltimate()};
-    if (!context().HasError(ultimate)) {
+    if (!context().HasError(ultimate) && !context().HasError(name.symbol)) {
       if (IsPointer(ultimate)) {
         Say(name,
             "'%s' is a pointer but is not initialized like one"_err_en_US);
       } else if (auto *details{ultimate.detailsIf<ObjectEntityDetails>()}) {
         CHECK(!details->init());
         Walk(expr);
         if (ultimate.owner().IsParameterizedDerivedType()) {
           // Can't convert to type of component, which might not yet
const auto &[accDecls, ompDecls, compilerDirectives, useStmts, importStmts,
       implicitPart, decls] = x.t;
   auto flagRestorer{common::ScopedSet(inSpecificationPart_, true)};
   auto stateRestorer{
       common::ScopedSet(specPartState_, SpecificationPartState{})};
   Walk(accDecls);
   Walk(ompDecls);
   Walk(compilerDirectives);
   Walk(useStmts);
+  ClearUseRenames();
+  ClearUseOnly();
   Walk(importStmts);
   Walk(implicitPart);
   for (const auto &decl : decls) {
     if (const auto *spec{
             std::get_if<parser::SpecificationConstruct>(&decl.u)}) {
       PreSpecificationConstruct(*spec);
     }
   }
if (Symbol * existing{FindInScope(symbolName)}) {
     }
     EraseSymbol(*existing);
   }
   info.Resolve(&MakeSymbol(symbolName, Attrs{}, std::move(genericDetails)));
 }
 
 void ResolveNamesVisitor::FinishSpecificationPart(
     const std::list<parser::DeclarationConstruct> &decls) {
   badStmtFuncFound_ = false;
   CheckImports();
   bool inModule{currScope().kind() == Scope::Kind::Module};
   for (auto &pair : currScope()) {
     auto &symbol{*pair.second};
     if (NeedsExplicitType(symbol)) {
       ApplyImplicitRules(symbol);
     }
     if (IsDummy(symbol) && isImplicitNoneType() &&

flang/test/Semantics/modfile41.f90

+! RUN: %S/test_errors.sh %s %t %flang_fc1
+! Test USE statements that use same module multiple times mixed with rename
+! clauses and ONLY clauses
+module m1
+  integer :: a = 1
+  integer :: b = 2
+end module m1
+module m2
+  integer :: a = 3
+end module m2
+module m3
+  integer :: a = 1
+  type t1
+    real t1_value
+  end type
+  type t2
+    complex t2_value
+  end type
+end module m3
+module m4
+  use m1
+end module m4
+module m5
+  use m1
+  use m1, z=>a
+end module m5
+module m6
+  use m1, only : a
+end module m6
+program testUse1
+  use m1
+  use m1,z=>a ! This prevents the use association of m1's "a" as local "a"
+  use m2 ! m2's version of "a" gets use associated
+  !ERROR: 'a' is use-associated from module 'm2' and cannot be re-declared
+  integer :: a = 2
+end
+program testUse2
+  use m1,only : a ! This forces the use association of m1's "a" as local "a"
+  use m1,z=>a ! This rename doesn't affect the previous forced USE association
+  !ERROR: 'a' is use-associated from module 'm1' and cannot be re-declared
+  integer :: a = 2
+end
+program testUse3
+  use m1 ! By itself, this would use associate m1's "a" with a local "a"
+  use m1,z=>a ! This rename of m1'a "a" removes the previous use association
+  integer :: a = 2
+end
+program testUse4
+  use m1,only : a ! Use associate m1's "a" with local "a"
+  use m1,z=>a ! Also use associate m1's "a" with local "z", also pulls in "b"
+  !ERROR: 'b' is use-associated from module 'm1' and cannot be re-declared
+  integer :: b = 2
+end
+program testUse5
+  use m1,z=>a ! The rename prevents creation of a local "a"
+  use m1 ! Does not create a local "a" because of the previous rename
+  integer :: a = 2
+end
+program testUse6
+  use m1, z => a ! Hides m1's "a"
+  use m1, y => b ! Hides m1's "b"
+  integer :: a = 4 ! OK
+  integer :: b = 5 ! OK
+end
+program testUse7
+  use m3,t1=>t2,t2=>t1 ! Looks weird but all is good
+  type(t1) x
+  type(t2) y
+  x%t2_value = a
+  y%t1_value = z
+end
+program testUse8
+  use m4 ! This USE associates all of m1
+  !ERROR: 'a' is use-associated from module 'm4' and cannot be re-declared
+  integer :: a = 2
+end
+program testUse9
+  use m5
+  integer :: a = 2
+end
+program testUse10
+  use m4
+  use m4, z=>a ! This rename erases the USE assocated "a" from m1
+  integer :: a = 2
+end
+program testUse11
+  use m6
+  use m6, z=>a ! This rename erases the USE assocated "a" from m1
+  integer :: a = 2
+end
+program testUse12
+  use m4 ! This USE associates "a" from m1
+  use m1, z=>a ! This renames the "a" from m1, but not the one through m4
+  !ERROR: 'a' is use-associated from module 'm4' and cannot be re-declared
+  integer :: a = 2
+end