Merge topic 'cxxmodules-private-between-targets'

d38779df2a ci: Enable RunCMake.CXXModules collation cases in clang jobs
69e4525241 Tests/CXXModules: add example for private modules between targets
18f87c87f8 cmCxxModuleMapper: track whether modules are private or not
56f7d6f827 cmCxxModuleMapper: add a structure to represent BMI locations
8207a3a266 cmDyndepCollation: add a query for visibility of an object's modules
e8efcbec8c iwyu: ignore `std::remove_reference` requirements

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !8476

7 files changed, 158 insertions, 38 deletions

diff --git a/Source/cmCMakeHostSystemInformationCommand.cxx b/Source/cmCMakeHostSystemInformationCommand.cxx
index 8bfd7c8..1c00f15 100644
--- a/Source/cmCMakeHostSystemInformationCommand.cxx
+++ b/Source/cmCMakeHostSystemInformationCommand.cxx
@@ -8,7 +8,6 @@
 #include <initializer_list>
 #include <map>
 #include <string>
-#include <type_traits>
 #include <utility>
 
 #include <cm/optional>
diff --git a/Source/cmCxxModuleMapper.cxx b/Source/cmCxxModuleMapper.cxx
index 59bf4c7..e836a2a 100644
--- a/Source/cmCxxModuleMapper.cxx
+++ b/Source/cmCxxModuleMapper.cxx
@@ -17,13 +17,59 @@
 #include "cmStringAlgorithms.h"
 #include "cmSystemTools.h"
 
-cm::optional<std::string> CxxModuleLocations::BmiGeneratorPathForModule(
+CxxBmiLocation::CxxBmiLocation() = default;
+
+CxxBmiLocation::CxxBmiLocation(std::string path)
+  : BmiLocation(std::move(path))
+{
+}
+
+CxxBmiLocation CxxBmiLocation::Unknown()
+{
+  return {};
+}
+
+CxxBmiLocation CxxBmiLocation::Private()
+{
+  return { std::string{} };
+}
+
+CxxBmiLocation CxxBmiLocation::Known(std::string path)
+{
+  return { std::move(path) };
+}
+
+bool CxxBmiLocation::IsKnown() const
+{
+  return this->BmiLocation.has_value();
+}
+
+bool CxxBmiLocation::IsPrivate() const
+{
+  if (auto const& loc = this->BmiLocation) {
+    return loc->empty();
+  }
+  return false;
+}
+
+std::string const& CxxBmiLocation::Location() const
+{
+  if (auto const& loc = this->BmiLocation) {
+    return *loc;
+  }
+  static std::string empty;
+  return empty;
+}
+
+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 {
@@ -42,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';
     }
   }
 
@@ -76,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';
     }
   }
 
@@ -123,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';
     }
   }
 
@@ -132,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.
@@ -237,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];
@@ -260,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 0f453b0..ef01e48 100644
--- a/Source/cmCxxModuleMapper.h
+++ b/Source/cmCxxModuleMapper.h
@@ -22,6 +22,23 @@ enum class CxxModuleMapFormat
   Msvc,
 };
 
+struct CxxBmiLocation
+{
+  static CxxBmiLocation Unknown();
+  static CxxBmiLocation Private();
+  static CxxBmiLocation Known(std::string path);
+
+  bool IsKnown() const;
+  bool IsPrivate() const;
+  std::string const& Location() const;
+
+private:
+  CxxBmiLocation();
+  CxxBmiLocation(std::string path);
+
+  cm::optional<std::string> BmiLocation;
+};
+
 struct CxxModuleLocations
 {
   // The path from which all relative paths should be computed. If
@@ -33,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/cmDyndepCollation.cxx b/Source/cmDyndepCollation.cxx
index 53a262b..f45d81b 100644
--- a/Source/cmDyndepCollation.cxx
+++ b/Source/cmDyndepCollation.cxx
@@ -623,3 +623,20 @@ bool cmDyndepCollation::WriteDyndepMetadata(
 
   return result;
 }
+
+bool cmDyndepCollation::IsObjectPrivate(
+  std::string const& object, cmCxxModuleExportInfo const& export_info)
+{
+#ifdef _WIN32
+  std::string output_path = object;
+  cmSystemTools::ConvertToUnixSlashes(output_path);
+#else
+  std::string const& output_path = object;
+#endif
+  auto fileset_info_itr = export_info.ObjectToFileSet.find(output_path);
+  if (fileset_info_itr == export_info.ObjectToFileSet.end()) {
+    return false;
+  }
+  auto const& file_set = fileset_info_itr->second;
+  return !cmFileSetVisibilityIsForInterface(file_set.Visibility);
+}
diff --git a/Source/cmDyndepCollation.h b/Source/cmDyndepCollation.h
index e70ac09..48afe2b 100644
--- a/Source/cmDyndepCollation.h
+++ b/Source/cmDyndepCollation.h
@@ -49,4 +49,6 @@ struct cmDyndepCollation
                                   std::vector<cmScanDepInfo> const& objects,
                                   cmCxxModuleExportInfo const& export_info,
                                   cmDyndepMetadataCallbacks const& cb);
+  static bool IsObjectPrivate(std::string const& object,
+                              cmCxxModuleExportInfo const& export_info);
 };
diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx
index b598c9b..8698e77 100644
--- a/Source/cmGlobalNinjaGenerator.cxx
+++ b/Source/cmGlobalNinjaGenerator.cxx
@@ -2536,7 +2536,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) {
@@ -2560,7 +2565,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"];
@@ -2641,8 +2654,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);
     }
   }
 
@@ -2662,12 +2682,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.
@@ -2689,13 +2712,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();
@@ -2761,7 +2785,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 {};
   };
diff --git a/Source/cmString.cxx b/Source/cmString.cxx
index aefaa64..f7f6293 100644
--- a/Source/cmString.cxx
+++ b/Source/cmString.cxx
@@ -9,7 +9,6 @@
 #include <ostream>
 #include <stdexcept>
 #include <string>
-#include <type_traits>
 
 namespace cm {