Browse Source

Merge topic 'cxxmodules-no-unity'

63bbb3768d cmLocalGenerator: ignore scanned sources for unity builds
76b5383123 cmGlobalGenerator: add unity sources after computing target compile features
7fc2a83fe6 Tests/CXXModules: add a test with unity build support

Acked-by: Kitware Robot <[email protected]>
Acked-by: buildbot <[email protected]>
Merge-request: !9118
Brad King 1 year ago
parent
commit
446abdf324

+ 4 - 0
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
 ================
 

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

+ 5 - 0
Help/prop_tgt/UNITY_BUILD.rst

@@ -76,6 +76,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

+ 29 - 1
Source/cmGlobalGenerator.cxx

@@ -1612,6 +1612,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()) {
@@ -1888,7 +1895,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")) {
@@ -1920,6 +1926,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<cmLinkLineComputer> cmGlobalGenerator::CreateLinkLineComputer(
   cmOutputConverter* outputConverter, cmStateDirectory const& stateDir) const
 {

+ 1 - 0
Source/cmGlobalGenerator.h

@@ -679,6 +679,7 @@ protected:
   bool AddHeaderSetVerification();
 
   bool AddAutomaticSources();
+  bool AddUnitySources();
 
   std::string SelectMakeProgram(const std::string& makeProgram,
                                 const std::string& makeDefault = "") const;

+ 9 - 0
Source/cmLocalGenerator.cxx

@@ -3210,6 +3210,15 @@ void cmLocalGenerator::AddUnityBuild(cmGeneratorTarget* target)
     std::vector<cmSourceFile*> 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);

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

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

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

+ 6 - 0
Tests/RunCMake/CXXModules/examples/unity-build/main.cxx

@@ -0,0 +1,6 @@
+import importable;
+
+int main(int argc, char* argv[])
+{
+  return from_import();
+}

+ 7 - 0
Tests/RunCMake/CXXModules/examples/unity-build/unity.h

@@ -0,0 +1,7 @@
+#ifndef unity_h
+#define unity_h
+
+int unity1();
+int unity2();
+
+#endif

+ 8 - 0
Tests/RunCMake/CXXModules/examples/unity-build/unity1.cxx

@@ -0,0 +1,8 @@
+#include "unity.h"
+
+#define DETECT_UNITY
+
+int unity1()
+{
+  return 0;
+}

+ 10 - 0
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;
+}