From 7fc2a83fe6a0dceb4fabfd6aac99a5cb2fa09ec5 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Mon, 1 Jan 2024 10:58:03 -0500 Subject: Tests/CXXModules: add a test with unity build support C++ module-using TUs cannot participate in unity builds. Add a test case for this situation. --- Tests/RunCMake/CXXModules/RunCMakeTest.cmake | 1 + .../CXXModules/examples/unity-build/CMakeLists.txt | 26 ++++++++++++++++++++++ .../CXXModules/examples/unity-build/importable.cxx | 10 +++++++++ .../CXXModules/examples/unity-build/main.cxx | 6 +++++ .../CXXModules/examples/unity-build/unity.h | 7 ++++++ .../CXXModules/examples/unity-build/unity1.cxx | 8 +++++++ .../CXXModules/examples/unity-build/unity2.cxx | 10 +++++++++ 7 files changed, 68 insertions(+) create mode 100644 Tests/RunCMake/CXXModules/examples/unity-build/CMakeLists.txt create mode 100644 Tests/RunCMake/CXXModules/examples/unity-build/importable.cxx create mode 100644 Tests/RunCMake/CXXModules/examples/unity-build/main.cxx create mode 100644 Tests/RunCMake/CXXModules/examples/unity-build/unity.h create mode 100644 Tests/RunCMake/CXXModules/examples/unity-build/unity1.cxx create mode 100644 Tests/RunCMake/CXXModules/examples/unity-build/unity2.cxx diff --git a/Tests/RunCMake/CXXModules/RunCMakeTest.cmake b/Tests/RunCMake/CXXModules/RunCMakeTest.cmake index fcfa60a..cd7527a 100644 --- a/Tests/RunCMake/CXXModules/RunCMakeTest.cmake +++ b/Tests/RunCMake/CXXModules/RunCMakeTest.cmake @@ -171,6 +171,7 @@ run_cxx_module_test(scan-with-pch) if ("named" IN_LIST CMake_TEST_MODULE_COMPILATION) run_cxx_module_test(simple) run_cxx_module_test(library library-static -DBUILD_SHARED_LIBS=OFF) + run_cxx_module_test(unity-build) run_cxx_module_test(object-library) run_cxx_module_test(generated) run_cxx_module_test(deep-chain) diff --git a/Tests/RunCMake/CXXModules/examples/unity-build/CMakeLists.txt b/Tests/RunCMake/CXXModules/examples/unity-build/CMakeLists.txt new file mode 100644 index 0000000..ce1751d --- /dev/null +++ b/Tests/RunCMake/CXXModules/examples/unity-build/CMakeLists.txt @@ -0,0 +1,26 @@ +cmake_minimum_required(VERSION 3.28) +project(cxx_modules_unity CXX) + +include("${CMAKE_SOURCE_DIR}/../cxx-modules-rules.cmake") + +set(CMAKE_UNITY_BUILD 1) + +add_executable(unity) +target_sources(unity + PRIVATE + main.cxx + unity1.cxx + unity2.cxx + PRIVATE + FILE_SET CXX_MODULES + BASE_DIRS + "${CMAKE_CURRENT_SOURCE_DIR}" + FILES + importable.cxx) +target_compile_features(unity PUBLIC cxx_std_20) + +set_property(SOURCE unity1.cxx unity2.cxx + PROPERTY + CXX_SCAN_FOR_MODULES 0) + +add_test(NAME unity COMMAND unity) diff --git a/Tests/RunCMake/CXXModules/examples/unity-build/importable.cxx b/Tests/RunCMake/CXXModules/examples/unity-build/importable.cxx new file mode 100644 index 0000000..148b033 --- /dev/null +++ b/Tests/RunCMake/CXXModules/examples/unity-build/importable.cxx @@ -0,0 +1,10 @@ +module; + +#include "unity.h" + +export module importable; + +export int from_import() +{ + return unity1() + unity2(); +} diff --git a/Tests/RunCMake/CXXModules/examples/unity-build/main.cxx b/Tests/RunCMake/CXXModules/examples/unity-build/main.cxx new file mode 100644 index 0000000..feb38d2 --- /dev/null +++ b/Tests/RunCMake/CXXModules/examples/unity-build/main.cxx @@ -0,0 +1,6 @@ +import importable; + +int main(int argc, char* argv[]) +{ + return from_import(); +} diff --git a/Tests/RunCMake/CXXModules/examples/unity-build/unity.h b/Tests/RunCMake/CXXModules/examples/unity-build/unity.h new file mode 100644 index 0000000..4a65a4b --- /dev/null +++ b/Tests/RunCMake/CXXModules/examples/unity-build/unity.h @@ -0,0 +1,7 @@ +#ifndef unity_h +#define unity_h + +int unity1(); +int unity2(); + +#endif diff --git a/Tests/RunCMake/CXXModules/examples/unity-build/unity1.cxx b/Tests/RunCMake/CXXModules/examples/unity-build/unity1.cxx new file mode 100644 index 0000000..fd1cbb3 --- /dev/null +++ b/Tests/RunCMake/CXXModules/examples/unity-build/unity1.cxx @@ -0,0 +1,8 @@ +#include "unity.h" + +#define DETECT_UNITY + +int unity1() +{ + return 0; +} diff --git a/Tests/RunCMake/CXXModules/examples/unity-build/unity2.cxx b/Tests/RunCMake/CXXModules/examples/unity-build/unity2.cxx new file mode 100644 index 0000000..0f25cff --- /dev/null +++ b/Tests/RunCMake/CXXModules/examples/unity-build/unity2.cxx @@ -0,0 +1,10 @@ +#include "unity.h" + +#ifndef DETECT_UNITY +# error "Should have detected a unity build" +#endif + +int unity2() +{ + return 0; +} -- cgit v0.12 From 76b5383123daa4249c9cfffdc93727713dfad29f Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Mon, 1 Jan 2024 10:58:42 -0500 Subject: cmGlobalGenerator: add unity sources after computing target compile features We need to know which sources will be scanned for C++ module dependencies in order to exclude them from unity builds. The addition of unity sources will not change the set of features. --- Source/cmGlobalGenerator.cxx | 30 +++++++++++++++++++++++++++++- Source/cmGlobalGenerator.h | 1 + 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx index c2b972d..5aaa0b8 100644 --- a/Source/cmGlobalGenerator.cxx +++ b/Source/cmGlobalGenerator.cxx @@ -1587,6 +1587,13 @@ bool cmGlobalGenerator::Compute() } } + // Add unity sources after computing compile features. Unity sources do + // not change the set of languages or features, but we need to know them + // to filter out sources that are scanned for C++ module dependencies. + if (!this->AddUnitySources()) { + return false; + } + for (const auto& localGen : this->LocalGenerators) { cmMakefile* mf = localGen->GetMakefile(); for (const auto& g : mf->GetInstallGenerators()) { @@ -1863,7 +1870,6 @@ bool cmGlobalGenerator::AddAutomaticSources() if (!gt->CanCompileSources()) { continue; } - lg->AddUnityBuild(gt.get()); lg->AddISPCDependencies(gt.get()); // Targets that reuse a PCH are handled below. if (!gt->GetProperty("PRECOMPILE_HEADERS_REUSE_FROM")) { @@ -1895,6 +1901,28 @@ bool cmGlobalGenerator::AddAutomaticSources() return true; } +bool cmGlobalGenerator::AddUnitySources() +{ + for (const auto& lg : this->LocalGenerators) { + for (const auto& gt : lg->GetGeneratorTargets()) { + if (!gt->CanCompileSources()) { + continue; + } + lg->AddUnityBuild(gt.get()); + } + } + // The above transformation may have changed the classification of sources. + // Clear the source list and classification cache (KindedSources) of all + // targets so that it will be recomputed correctly by the generators later + // now that the above transformations are done for all targets. + for (const auto& lg : this->LocalGenerators) { + for (const auto& gt : lg->GetGeneratorTargets()) { + gt->ClearSourcesCache(); + } + } + return true; +} + std::unique_ptr cmGlobalGenerator::CreateLinkLineComputer( cmOutputConverter* outputConverter, cmStateDirectory const& stateDir) const { diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h index aa54f69..763136e 100644 --- a/Source/cmGlobalGenerator.h +++ b/Source/cmGlobalGenerator.h @@ -677,6 +677,7 @@ protected: bool AddHeaderSetVerification(); bool AddAutomaticSources(); + bool AddUnitySources(); std::string SelectMakeProgram(const std::string& makeProgram, const std::string& makeDefault = "") const; -- cgit v0.12 From 63bbb3768d3cf87459b6b66effa8726f38cc745a Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Mon, 1 Jan 2024 11:04:29 -0500 Subject: cmLocalGenerator: ignore scanned sources for unity builds --- Help/manual/cmake-cxxmodules.7.rst | 4 ++++ Help/prop_sf/SKIP_UNITY_BUILD_INCLUSION.rst | 4 ++++ Help/prop_tgt/UNITY_BUILD.rst | 5 +++++ Source/cmLocalGenerator.cxx | 9 +++++++++ 4 files changed, 22 insertions(+) diff --git a/Help/manual/cmake-cxxmodules.7.rst b/Help/manual/cmake-cxxmodules.7.rst index b4c9cf1..3ee6645 100644 --- a/Help/manual/cmake-cxxmodules.7.rst +++ b/Help/manual/cmake-cxxmodules.7.rst @@ -30,6 +30,10 @@ following queries. The first query that provides a yes/no answer is used. - Otherwise, the source file will be scanned if the compiler and generator support scanning. See policy :policy:`CMP0155`. +Note that any scanned source will be excluded from any unity build (see +:prop_tgt:`UNITY_BUILD`) because module-related statements can only happen at +one place within a C++ translation unit. + Compiler Support ================ diff --git a/Help/prop_sf/SKIP_UNITY_BUILD_INCLUSION.rst b/Help/prop_sf/SKIP_UNITY_BUILD_INCLUSION.rst index ae526ac..38a0a78 100644 --- a/Help/prop_sf/SKIP_UNITY_BUILD_INCLUSION.rst +++ b/Help/prop_sf/SKIP_UNITY_BUILD_INCLUSION.rst @@ -11,3 +11,7 @@ in the same way as it would with unity builds disabled. This property helps with "ODR (One definition rule)" problems where combining a particular source file with others might lead to build errors or other unintended side effects. + +Note that sources which are scanned for C++ modules (see +:manual:`cmake-cxxmodules(7)`) are not eligible for unity build inclusion and +will automatically be excluded. diff --git a/Help/prop_tgt/UNITY_BUILD.rst b/Help/prop_tgt/UNITY_BUILD.rst index f827a20..9d68250 100644 --- a/Help/prop_tgt/UNITY_BUILD.rst +++ b/Help/prop_tgt/UNITY_BUILD.rst @@ -64,6 +64,11 @@ a number of measures to help address such problems: :prop_sf:`INCLUDE_DIRECTORIES` source property will not be combined into a unity source. +* Any source file which is scanned for C++ module sources via + :prop_tgt:`CXX_SCAN_FOR_MODULES`, :prop_sf:`CXX_SCAN_FOR_MODULES`, or + membership of a ``CXX_MODULES`` file set will not be combined into a unity + source. See :manual:`cmake-cxxmodules(7)` for details. + * Projects can prevent an individual source file from being combined into a unity source by setting its :prop_sf:`SKIP_UNITY_BUILD_INCLUSION` source property to true. This can be a more effective way to prevent diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx index 168cd41..7337ea2 100644 --- a/Source/cmLocalGenerator.cxx +++ b/Source/cmLocalGenerator.cxx @@ -3134,6 +3134,15 @@ void cmLocalGenerator::AddUnityBuild(cmGeneratorTarget* target) std::vector sources; target->GetSourceFiles(sources, configs[ci]); for (cmSourceFile* sf : sources) { + // Files which need C++ scanning cannot participate in unity builds as + // there is a single place in TUs that may perform module-dependency bits + // and a unity source cannot `#include` them in-order and represent a + // valid TU. + if (sf->GetLanguage() == "CXX"_s && + target->NeedDyndepForSource("CXX", configs[ci], sf)) { + continue; + } + auto mi = index.find(sf); if (mi == index.end()) { unitySources.emplace_back(sf); -- cgit v0.12