Merge topic 'clang-windows-cxx-modules'

1b7c26da49 Ninja: Wrap rules using '>' shell redirection with 'cmd /C' on Windows
ffd8537acf Clang: Record Clang 16.0 C++ modules flags only for GNU-like front-end
6013227230 cmGlobalNinjaGenerator: Use forward slashes in clang modmap format on Windows
d9d74b5e8a cmDyndepCollation: Drop outdated mentions of CXX_MODULE_INTERNAL_PARTITIONS
edab56d29a cmLocalNinjaGenerator: De-duplicate condition for using 'cmd /C' on Windows
8ebe3f92b3 cmGlobalNinjaGenerator: Detect GNU-like command-line for dyndep collator
f3ca199c9b cmGlobalNinjaGenerator: Factor out GNU-like command-line detection on Windows
f79817fcf0 cmCxxModuleMapper: Use value semantics in path conversion callback
...

Acked-by: Kitware Robot <[email protected]>
Merge-request: !8346

Modules/Compiler/Clang-CXX.cmake

@@ -30,16 +30,18 @@ if("x${CMAKE_CXX_COMPILER_FRONTEND_VARIANT}" STREQUAL "xMSVC")
   endif()
 endif()
 
-if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 16.0)
-  string(CONCAT CMAKE_EXPERIMENTAL_CXX_SCANDEP_SOURCE
-    "${CMAKE_CXX_COMPILER_CLANG_SCAN_DEPS}"
-    " -format=p1689"
-    " --"
-    " <CMAKE_CXX_COMPILER> <DEFINES> <INCLUDES> <FLAGS>"
-    " -x c++ <SOURCE> -c -o <OBJECT>"
-    " -MT <DYNDEP_FILE>"
-    " -MD -MF <DEP_FILE>"
-    " > <DYNDEP_FILE>")
-  set(CMAKE_EXPERIMENTAL_CXX_MODULE_MAP_FORMAT "clang")
-  set(CMAKE_EXPERIMENTAL_CXX_MODULE_MAP_FLAG "@<MODULE_MAP_FILE>")
-endif ()
+if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 16.0)
+  if("x${CMAKE_CXX_COMPILER_FRONTEND_VARIANT}" STREQUAL "xGNU")
+    string(CONCAT CMAKE_EXPERIMENTAL_CXX_SCANDEP_SOURCE
+      "\"${CMAKE_CXX_COMPILER_CLANG_SCAN_DEPS}\""
+      " -format=p1689"
+      " --"
+      " <CMAKE_CXX_COMPILER> <DEFINES> <INCLUDES> <FLAGS>"
+      " -x c++ <SOURCE> -c -o <OBJECT>"
+      " -MT <DYNDEP_FILE>"
+      " -MD -MF <DEP_FILE>"
+      " > <DYNDEP_FILE>")
+    set(CMAKE_EXPERIMENTAL_CXX_MODULE_MAP_FORMAT "clang")
+    set(CMAKE_EXPERIMENTAL_CXX_MODULE_MAP_FLAG "@<MODULE_MAP_FILE>")
+  endif()
+endif()

Source/cmCxxModuleMapper.cxx

@@ -21,7 +21,7 @@ cm::optional<std::string> CxxModuleLocations::BmiGeneratorPathForModule(
   std::string const& logical_name) const
 {
   if (auto l = this->BmiLocationForModule(logical_name)) {
-    return this->PathForGenerator(*l);
+    return this->PathForGenerator(std::move(*l));
   }
   return {};
 }
@@ -239,8 +239,7 @@ std::set<std::string> CxxModuleUsageSeed(
     for (auto const& p : object.Provides) {
       if (auto bmi_loc = loc.BmiGeneratorPathForModule(p.LogicalName)) {
         // XXX(cxx-modules): How to support header units?
-        usages.AddReference(p.LogicalName, loc.PathForGenerator(*bmi_loc),
-                            LookupMethod::ByName);
+        usages.AddReference(p.LogicalName, *bmi_loc, LookupMethod::ByName);
       }
     }
 
@@ -268,8 +267,7 @@ std::set<std::string> CxxModuleUsageSeed(
       }
 
       if (bmi_loc) {
-        usages.AddReference(r.LogicalName, loc.PathForGenerator(*bmi_loc),
-                            r.Method);
+        usages.AddReference(r.LogicalName, *bmi_loc, r.Method);
       }
     }
   }

Source/cmCxxModuleMapper.h

@@ -30,7 +30,7 @@ struct CxxModuleLocations
   std::string RootDirectory;
 
   // A function to convert a full path to a path for the generator.
-  std::function<std::string(std::string const&)> PathForGenerator;
+  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&)>

Source/cmDyndepCollation.cxx

@@ -441,20 +441,16 @@ bool cmDyndepCollation::WriteDyndepMetadata(
     auto fileset_info_itr = export_info.ObjectToFileSet.find(output_path);
     bool const has_provides = !object.Provides.empty();
     if (fileset_info_itr == export_info.ObjectToFileSet.end()) {
-      // If it provides anything, it should have a `CXX_MODULES` or
-      // `CXX_MODULE_INTERNAL_PARTITIONS` type and be present.
+      // If it provides anything, it should have type `CXX_MODULES`
+      // and be present.
       if (has_provides) {
         // Take the first module provided to provide context.
         auto const& provides = object.Provides[0];
-        char const* ok_types = "`CXX_MODULES`";
-        if (provides.LogicalName.find(':') != std::string::npos) {
-          ok_types = "`CXX_MODULES` (or `CXX_MODULE_INTERNAL_PARTITIONS` if "
-                     "it is not `export`ed)";
-        }
-        cmSystemTools::Error(cmStrCat(
-          "Output ", object.PrimaryOutput, " provides the `",
-          provides.LogicalName,
-          "` module but it is not found in a `FILE_SET` of type ", ok_types));
+        cmSystemTools::Error(
+          cmStrCat("Output ", object.PrimaryOutput, " provides the `",
+                   provides.LogicalName,
+                   "` module but it is not found in a `FILE_SET` of type "
+                   "`CXX_MODULES`"));
         result = false;
       }
 
@@ -474,38 +470,15 @@ bool cmDyndepCollation::WriteDyndepMetadata(
         result = false;
         continue;
       }
-    } else if (file_set.Type == "CXX_MODULE_INTERNAL_PARTITIONS"_s) {
-      if (!has_provides) {
-        cmSystemTools::Error(
-          cmStrCat("Source ", file_set.SourcePath,
-                   " is of type `CXX_MODULE_INTERNAL_PARTITIONS` but does not "
-                   "provide a module"));
-        result = false;
-        continue;
-      }
-      auto const& provides = object.Provides[0];
-      if (provides.LogicalName.find(':') == std::string::npos) {
-        cmSystemTools::Error(
-          cmStrCat("Source ", file_set.SourcePath,
-                   " is of type `CXX_MODULE_INTERNAL_PARTITIONS` but does not "
-                   "provide a module partition"));
-        result = false;
-        continue;
-      }
     } else if (file_set.Type == "CXX_MODULE_HEADERS"_s) {
       // TODO.
     } else {
       if (has_provides) {
         auto const& provides = object.Provides[0];
-        char const* ok_types = "`CXX_MODULES`";
-        if (provides.LogicalName.find(':') != std::string::npos) {
-          ok_types = "`CXX_MODULES` (or `CXX_MODULE_INTERNAL_PARTITIONS` if "
-                     "it is not `export`ed)";
-        }
-        cmSystemTools::Error(
-          cmStrCat("Source ", file_set.SourcePath, " provides the `",
-                   provides.LogicalName, "` C++ module but is of type `",
-                   file_set.Type, "` module but must be of type ", ok_types));
+        cmSystemTools::Error(cmStrCat(
+          "Source ", file_set.SourcePath, " provides the `",
+          provides.LogicalName, "` C++ module but is of type `", file_set.Type,
+          "` module but must be of type `CXX_MODULES`"));
         result = false;
       }
 

Source/cmGlobalNinjaGenerator.cxx

@@ -63,6 +63,19 @@ std::string const cmGlobalNinjaGenerator::SHELL_NOOP = "cd .";
 std::string const cmGlobalNinjaGenerator::SHELL_NOOP = ":";
 #endif
 
+namespace {
+#ifdef _WIN32
+bool DetectGCCOnWindows(cm::string_view compilerId, cm::string_view simulateId,
+                        cm::string_view compilerFrontendVariant)
+{
+  return ((compilerId == "Clang"_s && compilerFrontendVariant == "GNU"_s) ||
+          (simulateId != "MSVC"_s &&
+           (compilerId == "GNU"_s || compilerId == "QCC"_s ||
+            cmHasLiteralSuffix(compilerId, "Clang"))));
+}
+#endif
+}
+
 bool operator==(
   const cmGlobalNinjaGenerator::ByConfig::TargetDependsClosureKey& lhs,
   const cmGlobalNinjaGenerator::ByConfig::TargetDependsClosureKey& rhs)
@@ -936,12 +949,8 @@ void cmGlobalNinjaGenerator::EnableLanguage(
       mf->GetSafeDefinition(cmStrCat("CMAKE_", l, "_SIMULATE_ID"));
     std::string const& compilerFrontendVariant = mf->GetSafeDefinition(
       cmStrCat("CMAKE_", l, "_COMPILER_FRONTEND_VARIANT"));
-    if ((compilerId == "Clang" && compilerFrontendVariant == "GNU") ||
-        (simulateId != "MSVC" &&
-         (compilerId == "GNU" || compilerId == "QCC" ||
-          cmHasLiteralSuffix(compilerId, "Clang")))) {
-      this->UsingGCCOnWindows = true;
-    }
+    this->SetUsingGCCOnWindows(
+      DetectGCCOnWindows(compilerId, simulateId, compilerFrontendVariant));
 #endif
   }
 }
@@ -2629,8 +2638,14 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile(
   {
     CxxModuleLocations locs;
     locs.RootDirectory = ".";
-    locs.PathForGenerator = [this](std::string const& path) -> std::string {
-      return this->ConvertToNinjaPath(path);
+    locs.PathForGenerator = [this](std::string path) -> std::string {
+      path = this->ConvertToNinjaPath(path);
+#  ifdef _WIN32
+      if (this->IsGCCOnWindows()) {
+        std::replace(path.begin(), path.end(), '\\', '/');
+      }
+#  endif
+      return path;
     };
     locs.BmiLocationForModule =
       [&mod_files](std::string const& logical) -> cm::optional<std::string> {
@@ -2811,6 +2826,10 @@ int cmcmd_cmake_ninja_dyndep(std::vector<std::string>::const_iterator argBeg,
       linked_target_dirs.push_back(tdi_linked_target_dir.asString());
     }
   }
+  std::string const compilerId = tdi["compiler-id"].asString();
+  std::string const simulateId = tdi["compiler-simulate-id"].asString();
+  std::string const compilerFrontendVariant =
+    tdi["compiler-frontend-variant"].asString();
 
   auto export_info = cmDyndepCollation::ParseExportInfo(tdi);
 
@@ -2818,14 +2837,20 @@ int cmcmd_cmake_ninja_dyndep(std::vector<std::string>::const_iterator argBeg,
   cm.SetHomeDirectory(dir_top_src);
   cm.SetHomeOutputDirectory(dir_top_bld);
   auto ggd = cm.CreateGlobalGenerator("Ninja");
-  if (!ggd ||
-      !cm::static_reference_cast<cmGlobalNinjaGenerator>(ggd).WriteDyndepFile(
-        dir_top_src, dir_top_bld, dir_cur_src, dir_cur_bld, arg_dd, arg_ddis,
-        module_dir, linked_target_dirs, arg_lang, arg_modmapfmt,
-        *export_info)) {
+  if (!ggd) {
     return 1;
   }
-  return 0;
+  cmGlobalNinjaGenerator& gg =
+    cm::static_reference_cast<cmGlobalNinjaGenerator>(ggd);
+#  ifdef _WIN32
+  gg.SetUsingGCCOnWindows(
+    DetectGCCOnWindows(compilerId, simulateId, compilerFrontendVariant));
+#  endif
+  return gg.WriteDyndepFile(dir_top_src, dir_top_bld, dir_cur_src, dir_cur_bld,
+                            arg_dd, arg_ddis, module_dir, linked_target_dirs,
+                            arg_lang, arg_modmapfmt, *export_info)
+    ? 0
+    : 1;
 }
 
 #endif

Source/cmGlobalNinjaGenerator.h

@@ -169,6 +169,7 @@ public:
                            const std::string& comment = "");
 
   bool IsGCCOnWindows() const { return this->UsingGCCOnWindows; }
+  void SetUsingGCCOnWindows(bool b) { this->UsingGCCOnWindows = b; }
 
   cmGlobalNinjaGenerator(cmake* cm);
 

Source/cmLocalNinjaGenerator.cxx

@@ -456,6 +456,22 @@ std::string cmLocalNinjaGenerator::WriteCommandScript(
   return scriptPath;
 }
 
+#ifdef _WIN32
+namespace {
+bool RuleNeedsCMD(std::string const& cmd)
+{
+  std::vector<std::string> args;
+  cmSystemTools::ParseWindowsCommandLine(cmd.c_str(), args);
+  auto it = std::find_if(args.cbegin(), args.cend(),
+                         [](std::string const& arg) -> bool {
+                           // FIXME: Detect more windows shell operators.
+                           return cmHasLiteralPrefix(arg, ">");
+                         });
+  return it != args.cend();
+}
+}
+#endif
+
 std::string cmLocalNinjaGenerator::BuildCommandLine(
   std::vector<std::string> const& cmdLines, std::string const& outputConfig,
   std::string const& commandConfig, std::string const& customStep,
@@ -498,12 +514,13 @@ std::string cmLocalNinjaGenerator::BuildCommandLine(
   }
 
   std::ostringstream cmd;
-  for (auto li = cmdLines.begin(); li != cmdLines.end(); ++li)
 #ifdef _WIN32
-  {
+  bool const needCMD =
+    cmdLines.size() > 1 || (customStep.empty() && RuleNeedsCMD(cmdLines[0]));
+  for (auto li = cmdLines.begin(); li != cmdLines.end(); ++li) {
     if (li != cmdLines.begin()) {
       cmd << " && ";
-    } else if (cmdLines.size() > 1) {
+    } else if (needCMD) {
       cmd << "cmd.exe /C \"";
     }
     // Put current cmdLine in brackets if it contains "||" because it has
@@ -514,11 +531,11 @@ std::string cmLocalNinjaGenerator::BuildCommandLine(
       cmd << *li;
     }
   }
-  if (cmdLines.size() > 1) {
+  if (needCMD) {
     cmd << "\"";
   }
 #else
-  {
+  for (auto li = cmdLines.begin(); li != cmdLines.end(); ++li) {
     if (li != cmdLines.begin()) {
       cmd << " && ";
     }

Source/cmNinjaTargetGenerator.cxx

@@ -1651,6 +1651,10 @@ void cmNinjaTargetGenerator::WriteTargetDependInfo(std::string const& lang,
   tdi["language"] = lang;
   tdi["compiler-id"] = this->Makefile->GetSafeDefinition(
     cmStrCat("CMAKE_", lang, "_COMPILER_ID"));
+  tdi["compiler-simulate-id"] = this->Makefile->GetSafeDefinition(
+    cmStrCat("CMAKE_", lang, "_SIMULATE_ID"));
+  tdi["compiler-frontend-variant"] = this->Makefile->GetSafeDefinition(
+    cmStrCat("CMAKE_", lang, "_COMPILER_FRONTEND_VARIANT"));
 
   std::string mod_dir;
   if (lang == "Fortran") {