[flang-commits] [flang] [flang] Don't generate module file for hermetic USE'd dependency (PR #144143)

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-semantics

Author: Peter Klausler (klausler)

<details>
<summary>Changes</summary>

It's possible for the module file generation code to think that it needs to (re)generate a module file for a dependent module read from a hermetic module file, if it defines contains a procedure imported via renaming due to a name clash.  Adjust the logic that determines whether a module file should be written to include a check for having originated in a module file.

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


2 Files Affected:

- (modified) flang/lib/Semantics/mod-file.cpp (+23-21) 
- (added) flang/test/Semantics/modfile79.F90 (+33) 


``````````diff
diff --git a/flang/lib/Semantics/mod-file.cpp b/flang/lib/Semantics/mod-file.cpp
index 9f9e9f5840456..82c8536902eb2 100644
--- a/flang/lib/Semantics/mod-file.cpp
+++ b/flang/lib/Semantics/mod-file.cpp
@@ -109,15 +109,14 @@ bool ModFileWriter::WriteAll() {
 }
 
 void ModFileWriter::WriteAll(const Scope &scope) {
-  for (const auto &child : scope.children()) {
+  for (const Scope &child : scope.children()) {
     WriteOne(child);
   }
 }
 
 void ModFileWriter::WriteOne(const Scope &scope) {
   if (scope.kind() == Scope::Kind::Module) {
-    auto *symbol{scope.symbol()};
-    if (!symbol->test(Symbol::Flag::ModFile)) {
+    if (const auto *symbol{scope.symbol()}) {
       Write(*symbol);
     }
     WriteAll(scope); // write out submodules
@@ -134,7 +133,7 @@ static std::string ModFileName(const SourceName &name,
 // Write the module file for symbol, which must be a module or submodule.
 void ModFileWriter::Write(const Symbol &symbol) {
   const auto &module{symbol.get<ModuleDetails>()};
-  if (module.moduleFileHash()) {
+  if (symbol.test(Symbol::Flag::ModFile) || module.moduleFileHash()) {
     return; // already written
   }
   const auto *ancestor{module.ancestor()};
@@ -372,16 +371,19 @@ void ModFileWriter::PutSymbols(
   CollectSymbols(scope, sorted, uses, modules);
   // Write module files for dependencies first so that their
   // hashes are known.
-  for (auto ref : modules) {
+  for (const Symbol &mod : modules) {
     if (hermeticModules) {
-      hermeticModules->insert(*ref);
+      hermeticModules->insert(mod);
     } else {
-      Write(*ref);
-      needs_ << ModHeader::need
-             << CheckSumString(
-                    ref->get<ModuleDetails>().moduleFileHash().value())
-             << (ref->owner().IsIntrinsicModules() ? " i " : " n ")
-             << ref->name().ToString() << '\n';
+      Write(mod);
+      // It's possible that the module's file already existed and
+      // without its own hash due to being embedded in a hermetic
+      // module file.
+      if (auto hash{mod.get<ModuleDetails>().moduleFileHash()}) {
+        needs_ << ModHeader::need << CheckSumString(*hash)
+               << (mod.owner().IsIntrinsicModules() ? " i " : " n ")
+               << mod.name().ToString() << '\n';
+      }
     }
   }
   std::string buf; // stuff after CONTAINS in derived type
@@ -855,25 +857,25 @@ void CollectSymbols(const Scope &scope, SymbolVector &sorted,
   auto symbols{scope.GetSymbols()};
   std::size_t commonSize{scope.commonBlocks().size()};
   sorted.reserve(symbols.size() + commonSize);
-  for (SymbolRef symbol : symbols) {
-    const auto *generic{symbol->detailsIf<GenericDetails>()};
+  for (const Symbol &symbol : symbols) {
+    const auto *generic{symbol.detailsIf<GenericDetails>()};
     if (generic) {
       uses.insert(uses.end(), generic->uses().begin(), generic->uses().end());
-      for (auto ref : generic->uses()) {
-        modules.insert(GetUsedModule(ref->get<UseDetails>()));
+      for (const Symbol &used : generic->uses()) {
+        modules.insert(GetUsedModule(used.get<UseDetails>()));
       }
-    } else if (const auto *use{symbol->detailsIf<UseDetails>()}) {
+    } else if (const auto *use{symbol.detailsIf<UseDetails>()}) {
       modules.insert(GetUsedModule(*use));
     }
-    if (symbol->test(Symbol::Flag::ParentComp)) {
-    } else if (symbol->has<NamelistDetails>()) {
+    if (symbol.test(Symbol::Flag::ParentComp)) {
+    } else if (symbol.has<NamelistDetails>()) {
       namelist.push_back(symbol);
     } else if (generic) {
       if (generic->specific() &&
-          &generic->specific()->owner() == &symbol->owner()) {
+          &generic->specific()->owner() == &symbol.owner()) {
         sorted.push_back(*generic->specific());
       } else if (generic->derivedType() &&
-          &generic->derivedType()->owner() == &symbol->owner()) {
+          &generic->derivedType()->owner() == &symbol.owner()) {
         sorted.push_back(*generic->derivedType());
       }
       generics.push_back(symbol);
diff --git a/flang/test/Semantics/modfile79.F90 b/flang/test/Semantics/modfile79.F90
new file mode 100644
index 0000000000000..7d3b42166654e
--- /dev/null
+++ b/flang/test/Semantics/modfile79.F90
@@ -0,0 +1,33 @@
+!RUN: %flang -c -DWHICH=1 %s && FileCheck %s <modfile79a.mod && %flang -c -fhermetic-module-files -DWHICH=2 %s && %flang -c %s && FileCheck %s <modfile79a.mod
+
+!Ensure that writing modfile79c.mod doesn't cause a spurious
+!regeneration of modfile79a.mod from its copy in the hermetic
+!module file modfile79b.mod.
+!CHECK: !mod$ v1 sum:93ec75fe672c5b6c
+!CHECK-NEXT: module modfile79a
+
+#if WHICH == 1
+module modfile79a
+  interface foo
+    module procedure foo
+  end interface
+ contains
+  subroutine foo
+  end
+end
+#elif WHICH == 2
+module modfile79b
+  use modfile79a
+  interface bar
+    procedure foo
+  end interface
+end
+#else
+module modfile79c
+  use modfile79b
+ contains
+  subroutine test
+    call bar
+  end
+end
+#endif

``````````

</details>


https://github.com/llvm/llvm-project/pull/144143