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] Support multiple distinct module files with same name in one … #84838

Merged
merged 1 commit into from
Mar 13, 2024

Conversation

klausler
Copy link
Contributor

…compilation

Allow multiple module files with the same module name to exist in one compilation; distinct modules are distinguished by their hashes.

…compilation

Allow multiple module files with the same module name to exist in one
compilation; distinct modules are distinguished by their hashes.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:semantics labels Mar 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 11, 2024

@llvm/pr-subscribers-flang-semantics

Author: Peter Klausler (klausler)

Changes

…compilation

Allow multiple module files with the same module name to exist in one compilation; distinct modules are distinguished by their hashes.


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

4 Files Affected:

  • (modified) flang/include/flang/Semantics/module-dependences.h (+2-2)
  • (modified) flang/include/flang/Semantics/symbol.h (+3)
  • (modified) flang/lib/Semantics/mod-file.cpp (+39-46)
  • (modified) flang/test/Semantics/modfile63.f90 (+2-5)
diff --git a/flang/include/flang/Semantics/module-dependences.h b/flang/include/flang/Semantics/module-dependences.h
index 29813a19a4b1bc..3401d64c95938e 100644
--- a/flang/include/flang/Semantics/module-dependences.h
+++ b/flang/include/flang/Semantics/module-dependences.h
@@ -23,9 +23,9 @@ class ModuleDependences {
   void AddDependence(
       std::string &&name, bool intrinsic, ModuleCheckSumType hash) {
     if (intrinsic) {
-      intrinsicMap_.emplace(std::move(name), hash);
+      intrinsicMap_.insert_or_assign(std::move(name), hash);
     } else {
-      nonIntrinsicMap_.emplace(std::move(name), hash);
+      nonIntrinsicMap_.insert_or_assign(std::move(name), hash);
     }
   }
   std::optional<ModuleCheckSumType> GetRequiredHash(
diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h
index c3175a5d1a111d..67153ffb3be9f6 100644
--- a/flang/include/flang/Semantics/symbol.h
+++ b/flang/include/flang/Semantics/symbol.h
@@ -91,12 +91,15 @@ class ModuleDetails : public WithOmpDeclarative {
     return moduleFileHash_;
   }
   void set_moduleFileHash(ModuleCheckSumType x) { moduleFileHash_ = x; }
+  const Symbol *previous() const { return previous_; }
+  void set_previous(const Symbol *p) { previous_ = p; }
 
 private:
   bool isSubmodule_;
   bool isDefaultPrivate_{false};
   const Scope *scope_{nullptr};
   std::optional<ModuleCheckSumType> moduleFileHash_;
+  const Symbol *previous_{nullptr}; // same name, different module file hash
 };
 
 class MainProgramDetails : public WithOmpDeclarative {
diff --git a/flang/lib/Semantics/mod-file.cpp b/flang/lib/Semantics/mod-file.cpp
index b4df7216a33e20..5d0d210fa3487d 100644
--- a/flang/lib/Semantics/mod-file.cpp
+++ b/flang/lib/Semantics/mod-file.cpp
@@ -1242,7 +1242,7 @@ static void GetModuleDependences(
   std::size_t limit{content.size()};
   std::string_view str{content.data(), limit};
   for (std::size_t j{ModHeader::len};
-       str.substr(j, ModHeader::needLen) == ModHeader::need;) {
+       str.substr(j, ModHeader::needLen) == ModHeader::need; ++j) {
     j += 7;
     auto checkSum{ExtractCheckSum(str.substr(j, ModHeader::sumLen))};
     if (!checkSum) {
@@ -1260,8 +1260,8 @@ static void GetModuleDependences(
     for (; j < limit && str.at(j) != '\n'; ++j) {
     }
     if (j > start && j < limit && str.at(j) == '\n') {
-      dependences.AddDependence(
-          std::string{str.substr(start, j - start)}, intrinsic, *checkSum);
+      std::string depModName{str.substr(start, j - start)};
+      dependences.AddDependence(std::move(depModName), intrinsic, *checkSum);
     } else {
       break;
     }
@@ -1271,7 +1271,7 @@ static void GetModuleDependences(
 Scope *ModFileReader::Read(SourceName name, std::optional<bool> isIntrinsic,
     Scope *ancestor, bool silent) {
   std::string ancestorName; // empty for module
-  Symbol *notAModule{nullptr};
+  const Symbol *notAModule{nullptr};
   bool fatalError{false};
   if (ancestor) {
     if (auto *scope{ancestor->FindSubmodule(name)}) {
@@ -1287,26 +1287,28 @@ Scope *ModFileReader::Read(SourceName name, std::optional<bool> isIntrinsic,
     if (it != context_.globalScope().end()) {
       Scope *scope{it->second->scope()};
       if (scope->kind() == Scope::Kind::Module) {
-        if (requiredHash) {
-          if (const Symbol * foundModule{scope->symbol()}) {
-            if (const auto *module{foundModule->detailsIf<ModuleDetails>()};
-                module && module->moduleFileHash() &&
-                *requiredHash != *module->moduleFileHash()) {
-              Say(name, ancestorName,
-                  "Multiple versions of the module '%s' cannot be required by the same compilation"_err_en_US,
-                  name.ToString());
-              return nullptr;
+        for (const Symbol *found{scope->symbol()}; found;) {
+          if (const auto *module{found->detailsIf<ModuleDetails>()}) {
+            if (!requiredHash ||
+                *requiredHash ==
+                    module->moduleFileHash().value_or(*requiredHash)) {
+              return const_cast<Scope *>(found->scope());
             }
+            found = module->previous(); // same name, distinct hash
+          } else {
+            notAModule = found;
+            break;
           }
         }
-        return scope;
       } else {
         notAModule = scope->symbol();
-        // USE, NON_INTRINSIC global name isn't a module?
-        fatalError = isIntrinsic.has_value();
       }
     }
   }
+  if (notAModule) {
+    // USE, NON_INTRINSIC global name isn't a module?
+    fatalError = isIntrinsic.has_value();
+  }
   auto path{ModFileName(name, ancestorName, context_.moduleFileSuffix())};
   parser::Parsing parsing{context_.allCookedSources()};
   parser::Options options;
@@ -1360,42 +1362,18 @@ Scope *ModFileReader::Read(SourceName name, std::optional<bool> isIntrinsic,
 
   // Look for the right module file if its hash is known
   if (requiredHash && !fatalError) {
-    std::vector<std::string> misses;
     for (const std::string &maybe :
         parser::LocateSourceFileAll(path, options.searchDirectories)) {
       if (const auto *srcFile{context_.allCookedSources().allSources().OpenPath(
               maybe, llvm::errs())}) {
-        if (auto checkSum{VerifyHeader(srcFile->content())}) {
-          if (*checkSum == *requiredHash) {
-            path = maybe;
-            if (!misses.empty()) {
-              auto &msg{context_.Say(name,
-                  "Module file for '%s' appears later in the module search path than conflicting modules with different checksums"_warn_en_US,
-                  name.ToString())};
-              for (const std::string &m : misses) {
-                msg.Attach(
-                    name, "Module file with a conflicting name: '%s'"_en_US, m);
-              }
-            }
-            misses.clear();
-            break;
-          } else {
-            misses.emplace_back(maybe);
-          }
+        if (auto checkSum{VerifyHeader(srcFile->content())};
+            checkSum && *checkSum == *requiredHash) {
+          path = maybe;
+          break;
         }
       }
     }
-    if (!misses.empty()) {
-      auto &msg{Say(name, ancestorName,
-          "Could not find a module file for '%s' in the module search path with the expected checksum"_err_en_US,
-          name.ToString())};
-      for (const std::string &m : misses) {
-        msg.Attach(name, "Module file with different checksum: '%s'"_en_US, m);
-      }
-      return nullptr;
-    }
   }
-
   const auto *sourceFile{fatalError ? nullptr : parsing.Prescan(path, options)};
   if (fatalError || parsing.messages().AnyFatalError()) {
     if (!silent) {
@@ -1451,11 +1429,24 @@ Scope *ModFileReader::Read(SourceName name, std::optional<bool> isIntrinsic,
   Scope &topScope{isIntrinsic.value_or(false) ? context_.intrinsicModulesScope()
                                               : context_.globalScope()};
   Symbol *moduleSymbol{nullptr};
+  const Symbol *previousModuleSymbol{nullptr};
   if (!ancestor) { // module, not submodule
     parentScope = &topScope;
     auto pair{parentScope->try_emplace(name, UnknownDetails{})};
     if (!pair.second) {
-      return nullptr;
+      // There is already a global symbol or intrinsic module of the same name.
+      previousModuleSymbol = &*pair.first->second;
+      if (const auto *details{
+              previousModuleSymbol->detailsIf<ModuleDetails>()}) {
+        if (!details->moduleFileHash().has_value()) {
+          return nullptr;
+        }
+      } else {
+        return nullptr;
+      }
+      CHECK(parentScope->erase(name) != 0);
+      pair = parentScope->try_emplace(name, UnknownDetails{});
+      CHECK(pair.second);
     }
     moduleSymbol = &*pair.first->second;
     moduleSymbol->set(Symbol::Flag::ModFile);
@@ -1486,7 +1477,9 @@ Scope *ModFileReader::Read(SourceName name, std::optional<bool> isIntrinsic,
   }
   if (moduleSymbol) {
     CHECK(moduleSymbol->test(Symbol::Flag::ModFile));
-    moduleSymbol->get<ModuleDetails>().set_moduleFileHash(checkSum.value());
+    auto &details{moduleSymbol->get<ModuleDetails>()};
+    details.set_moduleFileHash(checkSum.value());
+    details.set_previous(previousModuleSymbol);
     if (isIntrinsic.value_or(false)) {
       moduleSymbol->attrs().set(Attr::INTRINSIC);
     }
diff --git a/flang/test/Semantics/modfile63.f90 b/flang/test/Semantics/modfile63.f90
index aaf1f7beaa48fa..0783121017243e 100644
--- a/flang/test/Semantics/modfile63.f90
+++ b/flang/test/Semantics/modfile63.f90
@@ -1,12 +1,10 @@
 ! RUN: %flang_fc1 -fsyntax-only -I%S/Inputs/dir1 %s
 ! RUN: not %flang_fc1 -fsyntax-only -I%S/Inputs/dir2 %s 2>&1 | FileCheck --check-prefix=ERROR %s
 ! RUN: %flang_fc1 -Werror -fsyntax-only -I%S/Inputs/dir1 -I%S/Inputs/dir2 %s
-! RUN: not %flang_fc1 -Werror -fsyntax-only -I%S/Inputs/dir2 -I%S/Inputs/dir1 %s 2>&1 | FileCheck  --check-prefix=WARNING %s
 
 ! Inputs/dir1 and Inputs/dir2 each have identical copies of modfile63b.mod.
 ! modfile63b.mod depends on Inputs/dir1/modfile63a.mod - the version in
-! Inputs/dir2/modfile63a.mod has a distinct checksum and should be
-! ignored with a warning.
+! Inputs/dir2/modfile63a.mod has a distinct checksum.
 
 ! If it becomes necessary to recompile those modules, just use the
 ! module files as Fortran source.
@@ -15,5 +13,4 @@
 call s2
 end
 
-! ERROR: Could not find a module file for 'modfile63a' in the module search path with the expected checksum
-! WARNING: Module file for 'modfile63a' appears later in the module search path than conflicting modules with different checksums
+! ERROR: Cannot read module file for module 'modfile63a': File is not the right module file for 'modfile63a':

@klausler klausler merged commit 5661188 into llvm:main Mar 13, 2024
6 of 7 checks passed
@klausler klausler deleted the pmk branch March 13, 2024 21:42
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.

None yet

3 participants