Browse Source

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).
Ben Boeckel 2 years ago
parent
commit
ed45432571

+ 6 - 0
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.

+ 9 - 4
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);
         }
       }
     }

+ 20 - 1
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)

+ 4 - 0
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 ()

+ 31 - 0
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)

+ 5 - 0
Tests/RunCMake/CXXModules/examples/ninja-cmp0154/importable.cxx.in

@@ -0,0 +1,5 @@
+export module importable;
+
+export int from_import() {
+  return 0;
+}

+ 8 - 0
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();
+}

+ 4 - 0
Tests/RunCMake/CXXModules/examples/ninja-cmp0154/unrelated.cxx

@@ -0,0 +1,4 @@
+int unrelated()
+{
+  return 0;
+}