From 5ab002ea11921c4766912cfa7ca98756271f87dc Mon Sep 17 00:00:00 2001 From: Brad King Date: Sat, 18 Mar 2023 09:55:18 -0400 Subject: cmCxxModuleMapper: Remove redundant path conversion callbacks Two calls to `PathForGenerator` were applied to values returned by `BmiGeneratorPathForModule`, that already calls `PathForGenerator`. --- Source/cmCxxModuleMapper.cxx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Source/cmCxxModuleMapper.cxx b/Source/cmCxxModuleMapper.cxx index 7952dfc..2c57c05 100644 --- a/Source/cmCxxModuleMapper.cxx +++ b/Source/cmCxxModuleMapper.cxx @@ -239,8 +239,7 @@ std::set 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 CxxModuleUsageSeed( } if (bmi_loc) { - usages.AddReference(r.LogicalName, loc.PathForGenerator(*bmi_loc), - r.Method); + usages.AddReference(r.LogicalName, *bmi_loc, r.Method); } } } -- cgit v0.12 From f79817fcf0168c377ea4cb7187830a8caf373168 Mon Sep 17 00:00:00 2001 From: Brad King Date: Sat, 18 Mar 2023 11:19:28 -0400 Subject: cmCxxModuleMapper: Use value semantics in path conversion callback The call site already owns a path it doesn't need when the callback returns. Hand ownership to the callback so it can optionally mutate the path without necessarily allocating. --- Source/cmCxxModuleMapper.cxx | 2 +- Source/cmCxxModuleMapper.h | 2 +- Source/cmGlobalNinjaGenerator.cxx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/cmCxxModuleMapper.cxx b/Source/cmCxxModuleMapper.cxx index 2c57c05..59bf4c7 100644 --- a/Source/cmCxxModuleMapper.cxx +++ b/Source/cmCxxModuleMapper.cxx @@ -21,7 +21,7 @@ cm::optional 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 {}; } diff --git a/Source/cmCxxModuleMapper.h b/Source/cmCxxModuleMapper.h index 9271978..0f453b0 100644 --- a/Source/cmCxxModuleMapper.h +++ b/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 PathForGenerator; + std::function PathForGenerator; // Lookup the BMI location of a logical module name. std::function(std::string const&)> diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 9db92c0..e739038 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -2629,7 +2629,7 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile( { CxxModuleLocations locs; locs.RootDirectory = "."; - locs.PathForGenerator = [this](std::string const& path) -> std::string { + locs.PathForGenerator = [this](std::string path) -> std::string { return this->ConvertToNinjaPath(path); }; locs.BmiLocationForModule = -- cgit v0.12 From f3ca199c9b26988645fe1da661687238c407c24e Mon Sep 17 00:00:00 2001 From: Brad King Date: Sat, 18 Mar 2023 11:20:00 -0400 Subject: cmGlobalNinjaGenerator: Factor out GNU-like command-line detection on Windows --- Source/cmGlobalNinjaGenerator.cxx | 21 +++++++++++++++------ Source/cmGlobalNinjaGenerator.h | 1 + 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index e739038..97366c2 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/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) @@ -943,12 +956,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 } } diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h index f18d63b..68b26cc 100644 --- a/Source/cmGlobalNinjaGenerator.h +++ b/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); -- cgit v0.12 From 8ebe3f92b3b270dca340ccceb34a751d213465a2 Mon Sep 17 00:00:00 2001 From: Brad King Date: Sat, 18 Mar 2023 11:21:08 -0400 Subject: cmGlobalNinjaGenerator: Detect GNU-like command-line for dyndep collator This will help the collator choose flags and path styles for modmap files. --- Source/cmGlobalNinjaGenerator.cxx | 22 ++++++++++++++++------ Source/cmNinjaTargetGenerator.cxx | 4 ++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 97366c2..4b894c7 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -2820,6 +2820,10 @@ int cmcmd_cmake_ninja_dyndep(std::vector::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); @@ -2827,14 +2831,20 @@ int cmcmd_cmake_ninja_dyndep(std::vector::const_iterator argBeg, cm.SetHomeDirectory(dir_top_src); cm.SetHomeOutputDirectory(dir_top_bld); auto ggd = cm.CreateGlobalGenerator("Ninja"); - if (!ggd || - !cm::static_reference_cast(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(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 diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 13782b0..5abf609 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -1634,6 +1634,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") { -- cgit v0.12 From edab56d29a4910ff9365c3ee673a0ab81d25005b Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 16 Mar 2023 17:07:53 -0400 Subject: cmLocalNinjaGenerator: De-duplicate condition for using 'cmd /C' on Windows --- Source/cmLocalNinjaGenerator.cxx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx index 1e2ea2a..77c5a76 100644 --- a/Source/cmLocalNinjaGenerator.cxx +++ b/Source/cmLocalNinjaGenerator.cxx @@ -498,12 +498,12 @@ std::string cmLocalNinjaGenerator::BuildCommandLine( } std::ostringstream cmd; - for (auto li = cmdLines.begin(); li != cmdLines.end(); ++li) #ifdef _WIN32 - { + bool const needCMD = cmdLines.size() > 1; + 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 +514,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 << " && "; } -- cgit v0.12 From d9d74b5e8a66222c40b343f95cb31a00636ecc6a Mon Sep 17 00:00:00 2001 From: Brad King Date: Sat, 18 Mar 2023 10:36:48 -0400 Subject: cmDyndepCollation: Drop outdated mentions of CXX_MODULE_INTERNAL_PARTITIONS These were left from an older design iteration in which, for MSVC, we needed to distinguish `cl -internalPartition` from `cl -interface` before scanning. It is no longer needed since `cl -scanDependencies` was updated to use the standard-conforming interpretation of non-exported module partition syntax. Issue: #24611 --- Source/cmDyndepCollation.cxx | 49 ++++++++++---------------------------------- 1 file changed, 11 insertions(+), 38 deletions(-) diff --git a/Source/cmDyndepCollation.cxx b/Source/cmDyndepCollation.cxx index 2827659..53a262b 100644 --- a/Source/cmDyndepCollation.cxx +++ b/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; } -- cgit v0.12 From 60132272302f106c996aa15a0c42cb3de9789a24 Mon Sep 17 00:00:00 2001 From: Brad King Date: Sat, 18 Mar 2023 11:21:50 -0400 Subject: cmGlobalNinjaGenerator: Use forward slashes in clang modmap format on Windows Issue: #24611 --- Source/cmGlobalNinjaGenerator.cxx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 4b894c7..914d1c7 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -2639,7 +2639,13 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile( CxxModuleLocations locs; locs.RootDirectory = "."; locs.PathForGenerator = [this](std::string path) -> std::string { - return this->ConvertToNinjaPath(path); + 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 { -- cgit v0.12 From ffd8537acf4ff938ec87b134f3b72e1a22d1b1d3 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 16 Mar 2023 16:40:17 -0400 Subject: Clang: Record Clang 16.0 C++ modules flags only for GNU-like front-end The settings added by commit 3fe8e33f27 (Clang: Record Clang 16.0 flags for our experimental C++ modules support, 2023-03-03, v3.26.0-rc6~6^2) work only for the GNU-like `clang++` front-end, and not for the MSVC-like `clang-cl` on Windows. Also quote the path to `clang-scan-deps` to support spaces in its path. Issue: #24611 --- Modules/Compiler/Clang-CXX.cmake | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/Modules/Compiler/Clang-CXX.cmake b/Modules/Compiler/Clang-CXX.cmake index 33154fd..a74e90b 100644 --- a/Modules/Compiler/Clang-CXX.cmake +++ b/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" - " --" - " " - " -x c++ -c -o " - " -MT " - " -MD -MF " - " > ") - set(CMAKE_EXPERIMENTAL_CXX_MODULE_MAP_FORMAT "clang") - set(CMAKE_EXPERIMENTAL_CXX_MODULE_MAP_FLAG "@") -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" + " --" + " " + " -x c++ -c -o " + " -MT " + " -MD -MF " + " > ") + set(CMAKE_EXPERIMENTAL_CXX_MODULE_MAP_FORMAT "clang") + set(CMAKE_EXPERIMENTAL_CXX_MODULE_MAP_FLAG "@") + endif() +endif() -- cgit v0.12 From 1b7c26da49e7e1a337f66951424bda8d3f4067fc Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 16 Mar 2023 17:16:39 -0400 Subject: Ninja: Wrap rules using '>' shell redirection with 'cmd /C' on Windows This is needed for the clang-scan-deps rule added by commit 0e21e55fc5 (Clang: Record Clang 16.0 C++ modules flags only for GNU-like front-end, 2023-03-16). Fixes: #24611 --- Source/cmLocalNinjaGenerator.cxx | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx index 77c5a76..3443cd3 100644 --- a/Source/cmLocalNinjaGenerator.cxx +++ b/Source/cmLocalNinjaGenerator.cxx @@ -456,6 +456,22 @@ std::string cmLocalNinjaGenerator::WriteCommandScript( return scriptPath; } +#ifdef _WIN32 +namespace { +bool RuleNeedsCMD(std::string const& cmd) +{ + std::vector 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 const& cmdLines, std::string const& outputConfig, std::string const& commandConfig, std::string const& customStep, @@ -499,7 +515,8 @@ std::string cmLocalNinjaGenerator::BuildCommandLine( std::ostringstream cmd; #ifdef _WIN32 - bool const needCMD = cmdLines.size() > 1; + 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 << " && "; -- cgit v0.12