Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flang] Submodule names can clash only with submodule names #67361

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

klausler
Copy link
Contributor

Name resolution creates symbols for submodules in their parents' scopes. This can lead to bogus errors about name clashes between submodule names and other entities in the parents' scopes.

Create symbols for submodules but do not add them to a scope's dictionary.

@klausler klausler requested a review from jeanPerier September 25, 2023 19:34
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:semantics labels Sep 25, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2023

@llvm/pr-subscribers-flang-semantics

Changes

Name resolution creates symbols for submodules in their parents' scopes. This can lead to bogus errors about name clashes between submodule names and other entities in the parents' scopes.

Create symbols for submodules but do not add them to a scope's dictionary.


Full diff: https://github.com/llvm/llvm-project/pull/67361.diff

4 Files Affected:

  • (modified) flang/lib/Semantics/mod-file.cpp (+31-12)
  • (modified) flang/lib/Semantics/resolve-names.cpp (+5-1)
  • (modified) flang/test/Semantics/modproc01.f90 (+3-1)
  • (modified) flang/test/Semantics/modproc02.f90 (-1)
diff --git a/flang/lib/Semantics/mod-file.cpp b/flang/lib/Semantics/mod-file.cpp
index 15d6f62f706c922..6eeb7bb7542b150 100644
--- a/flang/lib/Semantics/mod-file.cpp
+++ b/flang/lib/Semantics/mod-file.cpp
@@ -1175,30 +1175,49 @@ Scope *ModFileReader::Read(const SourceName &name,
   }
   Scope &topScope{isIntrinsic.value_or(false) ? context_.intrinsicModulesScope()
                                               : context_.globalScope()};
-  if (!ancestor) {
+  Symbol *moduleSymbol{nullptr};
+  if (!ancestor) { // module, not submodule
     parentScope = &topScope;
+    auto pair{parentScope->try_emplace(name, UnknownDetails{})};
+    if (!pair.second) {
+      return nullptr;
+    }
+    moduleSymbol = &*pair.first->second;
+    moduleSymbol->set(Symbol::Flag::ModFile);
   } else if (std::optional<SourceName> parent{GetSubmoduleParent(parseTree)}) {
+    // submodule with submodule parent
     parentScope = Read(*parent, false /*not intrinsic*/, ancestor, silent);
   } else {
+    // submodule with module parent
     parentScope = ancestor;
   }
-  auto pair{parentScope->try_emplace(name, UnknownDetails{})};
-  if (!pair.second) {
-    return nullptr;
-  }
   // Process declarations from the module file
-  Symbol &modSymbol{*pair.first->second};
-  modSymbol.set(Symbol::Flag::ModFile);
   bool wasInModuleFile{context_.foldingContext().inModuleFile()};
   context_.foldingContext().set_inModuleFile(true);
   ResolveNames(context_, parseTree, topScope);
   context_.foldingContext().set_inModuleFile(wasInModuleFile);
-  CHECK(modSymbol.has<ModuleDetails>());
-  CHECK(modSymbol.test(Symbol::Flag::ModFile));
-  if (isIntrinsic.value_or(false)) {
-    modSymbol.attrs().set(Attr::INTRINSIC);
+  if (!moduleSymbol) {
+    // Submodule symbols' storage are owned by their parents' scopes,
+    // but their names are not in their parents' dictionaries -- we
+    // don't want to report bogus errors about clashes between submodule
+    // names and other objects in the parent scopes.
+    if (Scope * submoduleScope{ancestor->FindSubmodule(name)}) {
+      moduleSymbol = submoduleScope->symbol();
+      if (moduleSymbol) {
+        moduleSymbol->set(Symbol::Flag::ModFile);
+      }
+    }
+  }
+  if (moduleSymbol) {
+    CHECK(moduleSymbol->has<ModuleDetails>());
+    CHECK(moduleSymbol->test(Symbol::Flag::ModFile));
+    if (isIntrinsic.value_or(false)) {
+      moduleSymbol->attrs().set(Attr::INTRINSIC);
+    }
+    return moduleSymbol->scope();
+  } else {
+    return nullptr;
   }
-  return modSymbol.scope();
 }
 
 parser::Message &ModFileReader::Say(const SourceName &name,
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 47ecaa1b82e53ed..b79f0989d46a2ec 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -3219,7 +3219,11 @@ bool ModuleVisitor::BeginSubmodule(
 }
 
 void ModuleVisitor::BeginModule(const parser::Name &name, bool isSubmodule) {
-  auto &symbol{MakeSymbol(name, ModuleDetails{isSubmodule})};
+  // Submodule symbols are not visible in their parents' scopes.
+  Symbol &symbol{isSubmodule ? Resolve(name,
+                                   currScope().MakeSymbol(name.source, Attrs{},
+                                       ModuleDetails{true}))
+                             : MakeSymbol(name, ModuleDetails{false})};
   auto &details{symbol.get<ModuleDetails>()};
   PushScope(Scope::Kind::Module, &symbol);
   details.set_scope(&currScope());
diff --git a/flang/test/Semantics/modproc01.f90 b/flang/test/Semantics/modproc01.f90
index c7d05783335e699..5652e15750c7e90 100644
--- a/flang/test/Semantics/modproc01.f90
+++ b/flang/test/Semantics/modproc01.f90
@@ -22,11 +22,12 @@ module subroutine ms(f)
       procedure(mf) :: f
     end subroutine
   end interface
+  integer sm
 end module
 !CHECK:    mf, MODULE, PUBLIC (Function): Subprogram isInterface result:TYPE(pdt2(k2=2_4,l2=n)) res (INTEGER(4) n,CHARACTER(n,1) str,TYPE(pdt1(k1=1_4,l1=n)) x1)
 !CHECK:    pdt1, PUBLIC: DerivedType components: a1
 !CHECK:    pdt2, PUBLIC: DerivedType components: j2,a2
-!CHECK:    sm: Module (m)
+!CHECK:    sm, PUBLIC size=4 offset=0: ObjectEntity type: INTEGER(4)
 !CHECK:    DerivedType scope: pdt1
 !CHECK:      a1, ALLOCATABLE: ObjectEntity type: TYPE(pdt2(int(k1,kind=4),int(l1,kind=4)))
 !CHECK:      k1: TypeParam type:INTEGER(4) Kind
@@ -128,6 +129,7 @@ program test
 !CHECK:    mf, MODULE (Function): Use from mf in m
 !CHECK:    pdt1: Use from pdt1 in m
 !CHECK:    pdt2: Use from pdt2 in m
+!CHECK:    sm: Use from sm in m
 !CHECK:    x size=88 offset=0: ObjectEntity type: TYPE(pdt2(k2=2_4,l2=3_4))
 !CHECK:    DerivedType scope: size=88 alignment=8 instantiation of pdt2(k2=2_4,l2=3_4)
 !CHECK:      a2 size=80 offset=8: ObjectEntity type: TYPE(pdt1(k1=2_4,l1=3_4)) shape: 1_8:2_8
diff --git a/flang/test/Semantics/modproc02.f90 b/flang/test/Semantics/modproc02.f90
index 229ef72e6bcf049..f47f473f081d2f8 100644
--- a/flang/test/Semantics/modproc02.f90
+++ b/flang/test/Semantics/modproc02.f90
@@ -16,7 +16,6 @@ module subroutine s(x) ! implicitly typed
 
 !CHECK:  Module scope: m size=0 alignment=1 sourceRange=63 bytes
 !CHECK:    s, MODULE, PUBLIC (Subroutine): Subprogram isInterface (REAL(4) x)
-!CHECK:    sm: Module (m)
 !CHECK:    Subprogram scope: s size=4 alignment=4 sourceRange=26 bytes
 !CHECK:      s (Subroutine): HostAssoc
 !CHECK:      x (Implicit) size=4 offset=0: ObjectEntity dummy type: REAL(4)

@klausler klausler force-pushed the bug1377 branch 2 times, most recently from 32a3e3c to 8c9a142 Compare September 29, 2023 21:18
Name resolution creates symbols for submodules in their parents'
scopes.  This can lead to bogus errors about name clashes between
submodule names and other entities in the parents' scopes.

Create symbols for submodules but do not add them to a scope's
dictionary.
@klausler klausler merged commit e200b0e into llvm:main Oct 16, 2023
@klausler klausler deleted the bug1377 branch October 17, 2023 00:35
klausler added a commit to klausler/llvm-project that referenced this pull request Oct 30, 2023
The change llvm#67361 removed
submodule name symbols from the name dictionaries of their parent
(sub)modules to prevent needless errors about name clashes, but
these symbols still need to be checked for things like excessive
length.
klausler added a commit that referenced this pull request Oct 31, 2023
The change #67361 removed
submodule name symbols from the name dictionaries of their parent
(sub)modules to prevent needless errors about name clashes, but these
symbols still need to be checked for things like excessive length.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants