cmCxxModuleMapper: track whether modules are private or not

This allows collation to give a useful error message when it finds usage
of a private module rather than collation just not informing the
compilation and the compiler erroring out about not being able to import
unknown modules (which exists, but it was not told about due to
visibility).

Fixes: #24652

3 files changed, 78 insertions, 36 deletions

diff --git a/Source/cmCxxModuleMapper.cxx b/Source/cmCxxModuleMapper.cxx
index e320f54..e836a2a 100644
--- a/Source/cmCxxModuleMapper.cxx
+++ b/Source/cmCxxModuleMapper.cxx
@@ -61,13 +61,15 @@ std::string const& CxxBmiLocation::Location() const
   return empty;
 }
 
-cm::optional<std::string> CxxModuleLocations::BmiGeneratorPathForModule(
+CxxBmiLocation CxxModuleLocations::BmiGeneratorPathForModule(
   std::string const& logical_name) const
 {
-  if (auto l = this->BmiLocationForModule(logical_name)) {
-    return this->PathForGenerator(std::move(*l));
+  auto bmi_loc = this->BmiLocationForModule(logical_name);
+  if (bmi_loc.IsKnown() && !bmi_loc.IsPrivate()) {
+    bmi_loc =
+      CxxBmiLocation::Known(this->PathForGenerator(bmi_loc.Location()));
   }
-  return {};
+  return bmi_loc;
 }
 
 namespace {
@@ -86,18 +88,21 @@ std::string CxxModuleMapContentClang(CxxModuleLocations const& loc,
   // A series of flags which tell the compiler where to look for modules.
 
   for (auto const& p : obj.Provides) {
-    if (auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName)) {
+    auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName);
+    if (bmi_loc.IsKnown()) {
       // Force the TU to be considered a C++ module source file regardless of
       // extension.
       mm << "-x c++-module\n";
 
-      mm << "-fmodule-output=" << *bmi_loc << '\n';
+      mm << "-fmodule-output=" << bmi_loc.Location() << '\n';
       break;
     }
   }
   for (auto const& r : obj.Requires) {
-    if (auto bmi_loc = loc.BmiGeneratorPathForModule(r.LogicalName)) {
-      mm << "-fmodule-file=" << r.LogicalName << "=" << *bmi_loc << '\n';
+    auto bmi_loc = loc.BmiGeneratorPathForModule(r.LogicalName);
+    if (bmi_loc.IsKnown()) {
+      mm << "-fmodule-file=" << r.LogicalName << "=" << bmi_loc.Location()
+         << '\n';
     }
   }
 
@@ -120,13 +125,15 @@ std::string CxxModuleMapContentGcc(CxxModuleLocations const& loc,
   mm << "$root " << loc.RootDirectory << "\n";
 
   for (auto const& p : obj.Provides) {
-    if (auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName)) {
-      mm << p.LogicalName << ' ' << *bmi_loc << '\n';
+    auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName);
+    if (bmi_loc.IsKnown()) {
+      mm << p.LogicalName << ' ' << bmi_loc.Location() << '\n';
     }
   }
   for (auto const& r : obj.Requires) {
-    if (auto bmi_loc = loc.BmiGeneratorPathForModule(r.LogicalName)) {
-      mm << r.LogicalName << ' ' << *bmi_loc << '\n';
+    auto bmi_loc = loc.BmiGeneratorPathForModule(r.LogicalName);
+    if (bmi_loc.IsKnown()) {
+      mm << r.LogicalName << ' ' << bmi_loc.Location() << '\n';
     }
   }
 
@@ -167,8 +174,9 @@ std::string CxxModuleMapContentMsvc(CxxModuleLocations const& loc,
       mm << "-internalPartition\n";
     }
 
-    if (auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName)) {
-      mm << "-ifcOutput " << *bmi_loc << '\n';
+    auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName);
+    if (bmi_loc.IsKnown()) {
+      mm << "-ifcOutput " << bmi_loc.Location() << '\n';
     }
   }
 
@@ -176,10 +184,11 @@ std::string CxxModuleMapContentMsvc(CxxModuleLocations const& loc,
   std::set<std::string> transitive_usage_names;
 
   for (auto const& r : obj.Requires) {
-    if (auto bmi_loc = loc.BmiGeneratorPathForModule(r.LogicalName)) {
+    auto bmi_loc = loc.BmiGeneratorPathForModule(r.LogicalName);
+    if (bmi_loc.IsKnown()) {
       auto flag = flag_for_method(r.Method);
 
-      mm << flag << ' ' << r.LogicalName << '=' << *bmi_loc << "\n";
+      mm << flag << ' ' << r.LogicalName << '=' << bmi_loc.Location() << "\n";
       transitive_usage_directs.insert(r.LogicalName);
 
       // Insert transitive usages.
@@ -281,18 +290,28 @@ std::set<std::string> CxxModuleUsageSeed(
   for (cmScanDepInfo const& object : objects) {
     // Add references for each of the provided modules.
     for (auto const& p : object.Provides) {
-      if (auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName)) {
+      auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName);
+      if (bmi_loc.IsKnown()) {
         // XXX(cxx-modules): How to support header units?
-        usages.AddReference(p.LogicalName, *bmi_loc, LookupMethod::ByName);
+        usages.AddReference(p.LogicalName, bmi_loc.Location(),
+                            LookupMethod::ByName);
       }
     }
 
     // For each requires, pull in what is required.
     for (auto const& r : object.Requires) {
-      // Find transitive usages.
-      auto transitive_usages = usages.Usage.find(r.LogicalName);
       // Find the required name in the current target.
       auto bmi_loc = loc.BmiGeneratorPathForModule(r.LogicalName);
+      if (bmi_loc.IsPrivate()) {
+        cmSystemTools::Error(
+          cmStrCat("Unable to use module '", r.LogicalName,
+                   "' as it is 'PRIVATE' and therefore not accessible outside "
+                   "of its owning target."));
+        continue;
+      }
+
+      // Find transitive usages.
+      auto transitive_usages = usages.Usage.find(r.LogicalName);
 
       for (auto const& p : object.Provides) {
         auto& this_usages = usages.Usage[p.LogicalName];
@@ -304,14 +323,14 @@ std::set<std::string> CxxModuleUsageSeed(
         if (transitive_usages != usages.Usage.end()) {
           this_usages.insert(transitive_usages->second.begin(),
                              transitive_usages->second.end());
-        } else if (bmi_loc) {
+        } else if (bmi_loc.IsKnown()) {
           // Mark that we need to update transitive usages later.
           internal_usages[p.LogicalName].insert(r.LogicalName);
         }
       }
 
-      if (bmi_loc) {
-        usages.AddReference(r.LogicalName, *bmi_loc, r.Method);
+      if (bmi_loc.IsKnown()) {
+        usages.AddReference(r.LogicalName, bmi_loc.Location(), r.Method);
       }
     }
   }
diff --git a/Source/cmCxxModuleMapper.h b/Source/cmCxxModuleMapper.h
index f405054..ef01e48 100644
--- a/Source/cmCxxModuleMapper.h
+++ b/Source/cmCxxModuleMapper.h
@@ -50,12 +50,11 @@ struct CxxModuleLocations
   std::function<std::string(std::string)> PathForGenerator;
 
   // Lookup the BMI location of a logical module name.
-  std::function<cm::optional<std::string>(std::string const&)>
-    BmiLocationForModule;
+  std::function<CxxBmiLocation(std::string const&)> BmiLocationForModule;
 
   // Returns the generator path (if known) for the BMI given a
   // logical module name.
-  cm::optional<std::string> BmiGeneratorPathForModule(
+  CxxBmiLocation BmiGeneratorPathForModule(
     std::string const& logical_name) const;
 };
 
diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx
index 7626fa3..21d1e6d 100644
--- a/Source/cmGlobalNinjaGenerator.cxx
+++ b/Source/cmGlobalNinjaGenerator.cxx
@@ -2526,7 +2526,12 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile(
   CxxModuleUsage usages;
 
   // Map from module name to module file path, if known.
-  std::map<std::string, std::string> mod_files;
+  struct AvailableModuleInfo
+  {
+    std::string BmiPath;
+    bool IsPrivate;
+  };
+  std::map<std::string, AvailableModuleInfo> mod_files;
 
   // Populate the module map with those provided by linked targets first.
   for (std::string const& linked_target_dir : linked_target_dirs) {
@@ -2550,7 +2555,15 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile(
       Json::Value const& target_modules = ltm["modules"];
       if (target_modules.isObject()) {
         for (auto i = target_modules.begin(); i != target_modules.end(); ++i) {
-          mod_files[i.key().asString()] = i->asString();
+          Json::Value const& visible_module = *i;
+          if (visible_module.isObject()) {
+            Json::Value const& bmi_path = visible_module["bmi"];
+            Json::Value const& is_private = visible_module["is-private"];
+            mod_files[i.key().asString()] = AvailableModuleInfo{
+              bmi_path.asString(),
+              is_private.asBool(),
+            };
+          }
         }
       }
       Json::Value const& target_modules_references = ltm["references"];
@@ -2631,8 +2644,15 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile(
         cmSystemTools::ReplaceString(safe_logical_name, ":", "-");
         mod = cmStrCat(module_dir, safe_logical_name, module_ext);
       }
-      mod_files[p.LogicalName] = mod;
-      target_modules[p.LogicalName] = mod;
+      mod_files[p.LogicalName] = AvailableModuleInfo{
+        mod,
+        false, // Always visible within our own target.
+      };
+      Json::Value& module_info = target_modules[p.LogicalName] =
+        Json::objectValue;
+      module_info["bmi"] = mod;
+      module_info["is-private"] =
+        cmDyndepCollation::IsObjectPrivate(object.PrimaryOutput, export_info);
     }
   }
 
@@ -2652,12 +2672,15 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile(
       return path;
     };
     locs.BmiLocationForModule =
-      [&mod_files](std::string const& logical) -> cm::optional<std::string> {
+      [&mod_files](std::string const& logical) -> CxxBmiLocation {
       auto m = mod_files.find(logical);
       if (m != mod_files.end()) {
-        return m->second;
+        if (m->second.IsPrivate) {
+          return CxxBmiLocation::Private();
+        }
+        return CxxBmiLocation::Known(m->second.BmiPath);
       }
-      return {};
+      return CxxBmiLocation::Unknown();
     };
 
     // Insert information about the current target's modules.
@@ -2679,13 +2702,14 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile(
       build.ImplicitOuts.clear();
       for (auto const& p : object.Provides) {
         build.ImplicitOuts.push_back(
-          this->ConvertToNinjaPath(mod_files[p.LogicalName]));
+          this->ConvertToNinjaPath(mod_files[p.LogicalName].BmiPath));
       }
       build.ImplicitDeps.clear();
       for (auto const& r : object.Requires) {
         auto mit = mod_files.find(r.LogicalName);
         if (mit != mod_files.end()) {
-          build.ImplicitDeps.push_back(this->ConvertToNinjaPath(mit->second));
+          build.ImplicitDeps.push_back(
+            this->ConvertToNinjaPath(mit->second.BmiPath));
         }
       }
       build.Variables.clear();
@@ -2751,7 +2775,7 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile(
     [mod_files](std::string const& name) -> cm::optional<std::string> {
     auto m = mod_files.find(name);
     if (m != mod_files.end()) {
-      return m->second;
+      return m->second.BmiPath;
     }
     return {};
   };