From 51f9d9f0a2a85744e9214aaa24054fabb9e79355 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 19 Oct 2023 22:11:33 -0400 Subject: cmNinjaTargetGenerator: avoid traversing old outputs repeatedly We actually only need to look at outputs just added to the vector, not all outputs that have been detected so far. --- Source/cmNinjaTargetGenerator.cxx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 0bda945..af47158 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -1048,10 +1048,13 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatements( cmCustomCommandGenerator ccg(*cc, config, this->GetLocalGenerator()); const std::vector& ccoutputs = ccg.GetOutputs(); const std::vector& ccbyproducts = ccg.GetByproducts(); + auto const nPreviousOutputs = ccouts.size(); ccouts.insert(ccouts.end(), ccoutputs.begin(), ccoutputs.end()); ccouts.insert(ccouts.end(), ccbyproducts.begin(), ccbyproducts.end()); if (usePrivateGeneratedSources) { auto it = ccouts.begin(); + // Skip over outputs that were already detected. + std::advance(it, nPreviousOutputs); while (it != ccouts.end()) { cmFileSet const* fileset = this->GeneratorTarget->GetFileSetForSource( -- cgit v0.12 From 462517092514e9751ab3e375fdcf2173d2c43399 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 19 Oct 2023 22:37:46 -0400 Subject: cmFileSet: add a query for includeable file set types --- Source/cmFileSet.cxx | 5 +++++ Source/cmFileSet.h | 2 ++ 2 files changed, 7 insertions(+) diff --git a/Source/cmFileSet.cxx b/Source/cmFileSet.cxx index bcf7fba..b74855f 100644 --- a/Source/cmFileSet.cxx +++ b/Source/cmFileSet.cxx @@ -80,6 +80,11 @@ bool cmFileSetVisibilityIsForInterface(cmFileSetVisibility vis) return false; } +bool cmFileSetTypeCanBeIncluded(std::string const& type) +{ + return type == "HEADERS"_s; +} + cmFileSet::cmFileSet(cmake& cmakeInstance, std::string name, std::string type, cmFileSetVisibility visibility) : CMakeInstance(cmakeInstance) diff --git a/Source/cmFileSet.h b/Source/cmFileSet.h index c508e2b..6ad4c9e 100644 --- a/Source/cmFileSet.h +++ b/Source/cmFileSet.h @@ -31,6 +31,8 @@ cmFileSetVisibility cmFileSetVisibilityFromName(cm::string_view name, bool cmFileSetVisibilityIsForSelf(cmFileSetVisibility vis); bool cmFileSetVisibilityIsForInterface(cmFileSetVisibility vis); +bool cmFileSetTypeCanBeIncluded(std::string const& type); + class cmFileSet { public: -- cgit v0.12 From 0973cd670231ec6a3e09cf22065e4b7dec4503f5 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 19 Oct 2023 22:38:13 -0400 Subject: cmNinjaTargetGenerator: use the file set visibility API --- Source/cmNinjaTargetGenerator.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index af47158..bb1d6f2 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -1060,7 +1060,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatements( this->GeneratorTarget->GetFileSetForSource( config, this->Makefile->GetOrCreateGeneratedSource(*it)); if (fileset && - fileset->GetVisibility() != cmFileSetVisibility::Private) { + cmFileSetVisibilityIsForInterface(fileset->GetVisibility())) { ++it; } else { ccouts_private.push_back(*it); -- cgit v0.12 From ed45432571e23ccba77cdb64aa3eb7e109e73329 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 19 Oct 2023 22:39:30 -0400 Subject: cmNinjaTargetGenerator: do not order-depend on C++ module sources C++ module sources should not be included by any other TUs, so their presence cannot matter for order-only dependencies of the entire target. Exclude them. Update CMP0154 to take this into consideration and add tests to the `CXXModules` suite (which already deals with module support detection). --- Help/policy/CMP0154.rst | 6 +++++ Source/cmNinjaTargetGenerator.cxx | 13 ++++++--- Tests/RunCMake/CXXModules/RunCMakeTest.cmake | 21 ++++++++++++++- .../examples/ninja-cmp0154-build-check.cmake | 4 +++ .../examples/ninja-cmp0154/CMakeLists.txt | 31 ++++++++++++++++++++++ .../examples/ninja-cmp0154/importable.cxx.in | 5 ++++ .../CXXModules/examples/ninja-cmp0154/main.cxx | 8 ++++++ .../examples/ninja-cmp0154/unrelated.cxx | 4 +++ 8 files changed, 87 insertions(+), 5 deletions(-) create mode 100644 Tests/RunCMake/CXXModules/examples/ninja-cmp0154-build-check.cmake create mode 100644 Tests/RunCMake/CXXModules/examples/ninja-cmp0154/CMakeLists.txt create mode 100644 Tests/RunCMake/CXXModules/examples/ninja-cmp0154/importable.cxx.in create mode 100644 Tests/RunCMake/CXXModules/examples/ninja-cmp0154/main.cxx create mode 100644 Tests/RunCMake/CXXModules/examples/ninja-cmp0154/unrelated.cxx diff --git a/Help/policy/CMP0154.rst b/Help/policy/CMP0154.rst index cb93d41..bf398af 100644 --- a/Help/policy/CMP0154.rst +++ b/Help/policy/CMP0154.rst @@ -22,6 +22,12 @@ type ``HEADERS``. With this information, :ref:`Ninja Generators` may omit the above-mentioned conservative dependencies and produce more efficient build graphs. +Additionally, if the custom command's output is a member of a file set of type +``CXX_MODULES``, it will additionally not be required to exist before +compiling other sources in the same target. Since these files should not be +included at compile time directly, they may not be implicitly required to +exist for other compilation rules. + This policy provides compatibility for projects using file sets in targets with generated header files that have not been updated. Such projects should be updated to express generated public headers in a file set. diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index bb1d6f2..05b6690 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -1059,13 +1059,18 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatements( cmFileSet const* fileset = this->GeneratorTarget->GetFileSetForSource( config, this->Makefile->GetOrCreateGeneratedSource(*it)); - if (fileset && - cmFileSetVisibilityIsForInterface(fileset->GetVisibility())) { + bool isVisible = fileset && + cmFileSetVisibilityIsForInterface(fileset->GetVisibility()); + bool isIncludeable = + !fileset || cmFileSetTypeCanBeIncluded(fileset->GetType()); + if (fileset && isVisible && isIncludeable) { ++it; - } else { + continue; + } + if (!fileset || isIncludeable) { ccouts_private.push_back(*it); - it = ccouts.erase(it); } + it = ccouts.erase(it); } } } diff --git a/Tests/RunCMake/CXXModules/RunCMakeTest.cmake b/Tests/RunCMake/CXXModules/RunCMakeTest.cmake index 2938fa8..e687e9f 100644 --- a/Tests/RunCMake/CXXModules/RunCMakeTest.cmake +++ b/Tests/RunCMake/CXXModules/RunCMakeTest.cmake @@ -133,7 +133,11 @@ function (run_cxx_module_test directory) ${ARGN}) run_cmake("examples/${test_name}") set(RunCMake_TEST_NO_CLEAN 1) - run_cmake_command("examples/${test_name}-build" "${CMAKE_COMMAND}" --build . --config Debug) + if (RunCMake_CXXModules_TARGET) + run_cmake_command("examples/${test_name}-build" "${CMAKE_COMMAND}" --build . --config Debug --target "${RunCMake_CXXModules_TARGET}") + else () + run_cmake_command("examples/${test_name}-build" "${CMAKE_COMMAND}" --build . --config Debug) + endif () if (RunCMake_CXXModules_INSTALL) run_cmake_command("examples/${test_name}-install" "${CMAKE_COMMAND}" --build . --target install --config Debug) endif () @@ -142,8 +146,23 @@ function (run_cxx_module_test directory) endif () endfunction () +function (run_cxx_module_test_target directory target) + set(RunCMake_CXXModules_TARGET "${target}") + set(RunCMake_CXXModules_NO_TEST 1) + run_cxx_module_test("${directory}" ${ARGN}) +endfunction () + string(REPLACE "," ";" CMake_TEST_MODULE_COMPILATION "${CMake_TEST_MODULE_COMPILATION}") +if (RunCMake_GENERATOR MATCHES "Ninja") + if (RunCMake_GENERATOR_IS_MULTI_CONFIG) + set(ninja_cmp0154_target "CMakeFiles/ninja_cmp0154.dir/Debug/unrelated.cxx${CMAKE_CXX_OUTPUT_EXTENSION}") + else () + set(ninja_cmp0154_target "CMakeFiles/ninja_cmp0154.dir/unrelated.cxx${CMAKE_CXX_OUTPUT_EXTENSION}") + endif () + run_cxx_module_test_target(ninja-cmp0154 "${ninja_cmp0154_target}") +endif () + # Tests which use named modules. if ("named" IN_LIST CMake_TEST_MODULE_COMPILATION) run_cxx_module_test(simple) diff --git a/Tests/RunCMake/CXXModules/examples/ninja-cmp0154-build-check.cmake b/Tests/RunCMake/CXXModules/examples/ninja-cmp0154-build-check.cmake new file mode 100644 index 0000000..6c4812b --- /dev/null +++ b/Tests/RunCMake/CXXModules/examples/ninja-cmp0154-build-check.cmake @@ -0,0 +1,4 @@ +if (EXISTS "${RunCMake_TEST_BINARY_DIR}/importable.cxx") + list(APPEND RunCMake_TEST_FAILED + "The `importable.cxx` file should not be generated to compile `unrelated`'s object") +endif () diff --git a/Tests/RunCMake/CXXModules/examples/ninja-cmp0154/CMakeLists.txt b/Tests/RunCMake/CXXModules/examples/ninja-cmp0154/CMakeLists.txt new file mode 100644 index 0000000..1aa36c1 --- /dev/null +++ b/Tests/RunCMake/CXXModules/examples/ninja-cmp0154/CMakeLists.txt @@ -0,0 +1,31 @@ +cmake_minimum_required(VERSION 3.24...3.28) +project(cxx_modules_ninja_cmp0154 CXX) + +include("${CMAKE_SOURCE_DIR}/../cxx-modules-rules.cmake") + +add_custom_command( + OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/importable.cxx" + DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/importable.cxx.in" + COMMAND "${CMAKE_COMMAND}" + -E copy_if_different + "${CMAKE_CURRENT_SOURCE_DIR}/importable.cxx.in" + "${CMAKE_CURRENT_BINARY_DIR}/importable.cxx" + COMMENT "Copying 'importable.cxx'") + +add_executable(ninja_cmp0154) +target_sources(ninja_cmp0154 + PRIVATE + main.cxx + unrelated.cxx + PUBLIC + FILE_SET CXX_MODULES + BASE_DIRS + "${CMAKE_CURRENT_BINARY_DIR}" + FILES + "${CMAKE_CURRENT_BINARY_DIR}/importable.cxx") +target_compile_features(ninja_cmp0154 PUBLIC cxx_std_20) +set_property(SOURCE unrelated.cxx + PROPERTY + CXX_SCAN_FOR_MODULES 0) + +add_test(NAME ninja_cmp0154 COMMAND ninja_cmp0154) diff --git a/Tests/RunCMake/CXXModules/examples/ninja-cmp0154/importable.cxx.in b/Tests/RunCMake/CXXModules/examples/ninja-cmp0154/importable.cxx.in new file mode 100644 index 0000000..a9287d7 --- /dev/null +++ b/Tests/RunCMake/CXXModules/examples/ninja-cmp0154/importable.cxx.in @@ -0,0 +1,5 @@ +export module importable; + +export int from_import() { + return 0; +} diff --git a/Tests/RunCMake/CXXModules/examples/ninja-cmp0154/main.cxx b/Tests/RunCMake/CXXModules/examples/ninja-cmp0154/main.cxx new file mode 100644 index 0000000..1ac4850 --- /dev/null +++ b/Tests/RunCMake/CXXModules/examples/ninja-cmp0154/main.cxx @@ -0,0 +1,8 @@ +import importable; + +extern int unrelated(); + +int main(int argc, char* argv[]) +{ + return from_import() + unrelated(); +} diff --git a/Tests/RunCMake/CXXModules/examples/ninja-cmp0154/unrelated.cxx b/Tests/RunCMake/CXXModules/examples/ninja-cmp0154/unrelated.cxx new file mode 100644 index 0000000..d54a47f --- /dev/null +++ b/Tests/RunCMake/CXXModules/examples/ninja-cmp0154/unrelated.cxx @@ -0,0 +1,4 @@ +int unrelated() +{ + return 0; +} -- cgit v0.12