llvm · klausler · Mar 13, 2024 · Mar 11, 2024
diff --git 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
@@ -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
@@ -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
@@ -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':