Merge topic 'unity-build-per-config'

129e3c6540 Unity Build: Fix per-config sources in multi-config generators
ea289314ef VS: Fix pre-VS15.8 unity build exclusion of per-config sources
53990059da cmLocalGenerator: Add dedicated types to hold unity source info
de6e362a88 cmLocalGenerator: Clarify name of method to write unity source include lines
3017b3e7d4 cmLocalGenerator: Simplify unity source copy-if-different logic
0b56f92576 cmLocalGenerator: De-duplicate unity source file generation

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !6720

Source/cmLocalGenerator.cxx

@@ -37,6 +37,7 @@
 #include "cmInstallTargetGenerator.h"
 #include "cmLinkLineComputer.h"
 #include "cmMakefile.h"
+#include "cmRange.h"
 #include "cmRulePlaceholderExpander.h"
 #include "cmSourceFile.h"
 #include "cmSourceFileLocation.h"
@@ -2806,10 +2807,47 @@ inline void RegisterUnitySources(cmGeneratorTarget* target, cmSourceFile* sf,
 }
 }
 
-void cmLocalGenerator::IncludeFileInUnitySources(
-  cmGeneratedFileStream& unity_file, std::string const& sf_full_path,
-  cmValue beforeInclude, cmValue afterInclude, cmValue uniqueIdName) const
+cmLocalGenerator::UnitySource cmLocalGenerator::WriteUnitySource(
+  cmGeneratorTarget* target, std::vector<std::string> const& configs,
+  cmRange<std::vector<UnityBatchedSource>::const_iterator> sources,
+  cmValue beforeInclude, cmValue afterInclude, std::string filename) const
 {
+  cmValue uniqueIdName = target->GetProperty("UNITY_BUILD_UNIQUE_ID");
+  cmGeneratedFileStream file(
+    filename, false, target->GetGlobalGenerator()->GetMakefileEncoding());
+  file.SetCopyIfDifferent(true);
+  file << "/* generated by CMake */\n\n";
+
+  bool perConfig = false;
+  for (UnityBatchedSource const& ubs : sources) {
+    cm::optional<std::string> cond;
+    if (ubs.Configs.size() != configs.size()) {
+      perConfig = true;
+      cond = std::string();
+      cm::string_view sep;
+      for (size_t ci : ubs.Configs) {
+        cond = cmStrCat(*cond, sep, "defined(CMAKE_UNITY_CONFIG_",
+                        cmSystemTools::UpperCase(configs[ci]), ")");
+        sep = " || "_s;
+      }
+    }
+    RegisterUnitySources(target, ubs.Source, filename);
+    WriteUnitySourceInclude(file, cond, ubs.Source->ResolveFullPath(),
+                            beforeInclude, afterInclude, uniqueIdName);
+  }
+
+  return UnitySource(std::move(filename), perConfig);
+}
+
+void cmLocalGenerator::WriteUnitySourceInclude(
+  std::ostream& unity_file, cm::optional<std::string> const& cond,
+  std::string const& sf_full_path, cmValue beforeInclude, cmValue afterInclude,
+  cmValue uniqueIdName) const
+{
+  if (cond) {
+    unity_file << "#if " << *cond << "\n";
+  }
+
   if (cmNonempty(uniqueIdName)) {
     std::string pathToHash;
     auto PathEqOrSubDir = [](std::string const& a, std::string const& b) {
@@ -2844,21 +2882,25 @@ void cmLocalGenerator::IncludeFileInUnitySources(
   if (afterInclude) {
     unity_file << *afterInclude << "\n";
   }
+  if (cond) {
+    unity_file << "#endif\n";
+  }
   unity_file << "\n";
 }
 
-std::vector<std::string> cmLocalGenerator::AddUnityFilesModeAuto(
+std::vector<cmLocalGenerator::UnitySource>
+cmLocalGenerator::AddUnityFilesModeAuto(
   cmGeneratorTarget* target, std::string const& lang,
-  std::vector<cmSourceFile*> const& filtered_sources, cmValue beforeInclude,
-  cmValue afterInclude, std::string const& filename_base, size_t batchSize)
+  std::vector<std::string> const& configs,
+  std::vector<UnityBatchedSource> const& filtered_sources,
+  cmValue beforeInclude, cmValue afterInclude,
+  std::string const& filename_base, size_t batchSize)
 {
   if (batchSize == 0) {
     batchSize = filtered_sources.size();
   }
 
-  cmValue uniqueIdName = target->GetProperty("UNITY_BUILD_UNIQUE_ID");
-
-  std::vector<std::string> unity_files;
+  std::vector<UnitySource> unity_files;
   for (size_t itemsLeft = filtered_sources.size(), chunk, batch = 0;
        itemsLeft > 0; itemsLeft -= chunk, ++batch) {
 
@@ -2866,74 +2908,48 @@ std::vector<std::string> cmLocalGenerator::AddUnityFilesModeAuto(
 
     std::string filename = cmStrCat(filename_base, "unity_", batch,
                                     (lang == "C") ? "_c.c" : "_cxx.cxx");
-
-    const std::string filename_tmp = cmStrCat(filename, ".tmp");
-    {
-      size_t begin = batch * batchSize;
-      size_t end = begin + chunk;
-
-      cmGeneratedFileStream file(
-        filename_tmp, false,
-        target->GetGlobalGenerator()->GetMakefileEncoding());
-      file << "/* generated by CMake */\n\n";
-
-      for (; begin != end; ++begin) {
-        cmSourceFile* sf = filtered_sources[begin];
-        RegisterUnitySources(target, sf, filename);
-        IncludeFileInUnitySources(file, sf->ResolveFullPath(), beforeInclude,
-                                  afterInclude, uniqueIdName);
-      }
-    }
-    cmSystemTools::MoveFileIfDifferent(filename_tmp, filename);
-    unity_files.emplace_back(std::move(filename));
+    auto const begin = filtered_sources.begin() + batch * batchSize;
+    auto const end = begin + chunk;
+    unity_files.emplace_back(this->WriteUnitySource(
+      target, configs, cmMakeRange(begin, end), beforeInclude, afterInclude,
+      std::move(filename)));
   }
   return unity_files;
 }
 
-std::vector<std::string> cmLocalGenerator::AddUnityFilesModeGroup(
+std::vector<cmLocalGenerator::UnitySource>
+cmLocalGenerator::AddUnityFilesModeGroup(
   cmGeneratorTarget* target, std::string const& lang,
-  std::vector<cmSourceFile*> const& filtered_sources, cmValue beforeInclude,
-  cmValue afterInclude, std::string const& filename_base)
+  std::vector<std::string> const& configs,
+  std::vector<UnityBatchedSource> const& filtered_sources,
+  cmValue beforeInclude, cmValue afterInclude,
+  std::string const& filename_base)
 {
-  std::vector<std::string> unity_files;
+  std::vector<UnitySource> unity_files;
 
   // sources organized by group name. Drop any source
   // without a group
-  std::unordered_map<std::string, std::vector<cmSourceFile*>> explicit_mapping;
-  for (cmSourceFile* sf : filtered_sources) {
-    if (cmValue value = sf->GetProperty("UNITY_GROUP")) {
+  std::unordered_map<std::string, std::vector<UnityBatchedSource>>
+    explicit_mapping;
+  for (UnityBatchedSource const& ubs : filtered_sources) {
+    if (cmValue value = ubs.Source->GetProperty("UNITY_GROUP")) {
       auto i = explicit_mapping.find(*value);
       if (i == explicit_mapping.end()) {
-        std::vector<cmSourceFile*> sources{ sf };
-        explicit_mapping.emplace(*value, sources);
+        std::vector<UnityBatchedSource> sources{ ubs };
+        explicit_mapping.emplace(*value, std::move(sources));
       } else {
-        i->second.emplace_back(sf);
+        i->second.emplace_back(ubs);
       }
     }
   }
 
-  cmValue uniqueIdName = target->GetProperty("UNITY_BUILD_UNIQUE_ID");
-
   for (auto const& item : explicit_mapping) {
     auto const& name = item.first;
     std::string filename = cmStrCat(filename_base, "unity_", name,
                                     (lang == "C") ? "_c.c" : "_cxx.cxx");
-
-    const std::string filename_tmp = cmStrCat(filename, ".tmp");
-    {
-      cmGeneratedFileStream file(
-        filename_tmp, false,
-        target->GetGlobalGenerator()->GetMakefileEncoding());
-      file << "/* generated by CMake */\n\n";
-
-      for (cmSourceFile* sf : item.second) {
-        RegisterUnitySources(target, sf, filename);
-        IncludeFileInUnitySources(file, sf->ResolveFullPath(), beforeInclude,
-                                  afterInclude, uniqueIdName);
-      }
-    }
-    cmSystemTools::MoveFileIfDifferent(filename_tmp, filename);
-    unity_files.emplace_back(std::move(filename));
+    unity_files.emplace_back(this->WriteUnitySource(
+      target, configs, cmMakeRange(item.second), beforeInclude, afterInclude,
+      std::move(filename)));
   }
 
   return unity_files;
@@ -2945,20 +2961,33 @@ void cmLocalGenerator::AddUnityBuild(cmGeneratorTarget* target)
     return;
   }
 
-  // FIXME: Handle all configurations in multi-config generators.
-  std::string config;
-  if (!this->GetGlobalGenerator()->IsMultiConfig()) {
-    config = this->Makefile->GetSafeDefinition("CMAKE_BUILD_TYPE");
+  std::vector<UnityBatchedSource> unitySources;
+
+  std::vector<std::string> configs =
+    this->Makefile->GetGeneratorConfigs(cmMakefile::IncludeEmptyConfig);
+
+  std::map<cmSourceFile const*, size_t> index;
+
+  for (size_t ci = 0; ci < configs.size(); ++ci) {
+    // FIXME: Refactor collection of sources to not evaluate object libraries.
+    std::vector<cmSourceFile*> sources;
+    target->GetSourceFiles(sources, configs[ci]);
+    for (cmSourceFile* sf : sources) {
+      auto mi = index.find(sf);
+      if (mi == index.end()) {
+        unitySources.emplace_back(sf);
+        std::map<cmSourceFile const*, size_t>::value_type entry(
+          sf, unitySources.size() - 1);
+        mi = index.insert(entry).first;
+      }
+      unitySources[mi->second].Configs.emplace_back(ci);
+    }
   }
 
   std::string filename_base =
     cmStrCat(this->GetCurrentBinaryDirectory(), "/CMakeFiles/",
              target->GetName(), ".dir/Unity/");
 
-  // FIXME: Refactor collection of sources to not evaluate object libraries.
-  std::vector<cmSourceFile*> sources;
-  target->GetSourceFiles(sources, config);
-
   cmValue batchSizeString = target->GetProperty("UNITY_BUILD_BATCH_SIZE");
   const size_t unityBatchSize = batchSizeString
     ? static_cast<size_t>(std::atoi(batchSizeString->c_str()))
@@ -2970,9 +2999,11 @@ void cmLocalGenerator::AddUnityBuild(cmGeneratorTarget* target)
   cmValue unityMode = target->GetProperty("UNITY_BUILD_MODE");
 
   for (std::string lang : { "C", "CXX" }) {
-    std::vector<cmSourceFile*> filtered_sources;
-    std::copy_if(sources.begin(), sources.end(),
-                 std::back_inserter(filtered_sources), [&](cmSourceFile* sf) {
+    std::vector<UnityBatchedSource> filtered_sources;
+    std::copy_if(unitySources.begin(), unitySources.end(),
+                 std::back_inserter(filtered_sources),
+                 [&](UnityBatchedSource const& ubs) -> bool {
+                   cmSourceFile* sf = ubs.Source;
                    return sf->GetLanguage() == lang &&
                      !sf->GetPropertyAsBool("SKIP_UNITY_BUILD_INCLUSION") &&
                      !sf->GetPropertyAsBool("HEADER_FILE_ONLY") &&
@@ -2982,15 +3013,15 @@ void cmLocalGenerator::AddUnityBuild(cmGeneratorTarget* target)
                      !sf->GetProperty("INCLUDE_DIRECTORIES");
                  });
 
-    std::vector<std::string> unity_files;
+    std::vector<UnitySource> unity_files;
     if (!unityMode || *unityMode == "BATCH") {
-      unity_files =
-        AddUnityFilesModeAuto(target, lang, filtered_sources, beforeInclude,
-                              afterInclude, filename_base, unityBatchSize);
+      unity_files = AddUnityFilesModeAuto(
+        target, lang, configs, filtered_sources, beforeInclude, afterInclude,
+        filename_base, unityBatchSize);
     } else if (unityMode && *unityMode == "GROUP") {
       unity_files =
-        AddUnityFilesModeGroup(target, lang, filtered_sources, beforeInclude,
-                               afterInclude, filename_base);
+        AddUnityFilesModeGroup(target, lang, configs, filtered_sources,
+                               beforeInclude, afterInclude, filename_base);
     } else {
       // unity mode is set to an unsupported value
       std::string e("Invalid UNITY_BUILD_MODE value of " + *unityMode +
@@ -2999,11 +3030,15 @@ void cmLocalGenerator::AddUnityBuild(cmGeneratorTarget* target)
       this->IssueMessage(MessageType::FATAL_ERROR, e);
     }
 
-    for (auto const& file : unity_files) {
-      auto* unity = this->GetMakefile()->GetOrCreateSource(file);
-      target->AddSource(file, true);
+    for (UnitySource const& file : unity_files) {
+      auto* unity = this->GetMakefile()->GetOrCreateSource(file.Path);
+      target->AddSource(file.Path, true);
       unity->SetProperty("SKIP_UNITY_BUILD_INCLUSION", "ON");
-      unity->SetProperty("UNITY_SOURCE_FILE", file);
+      unity->SetProperty("UNITY_SOURCE_FILE", file.Path);
+      if (file.PerConfig) {
+        unity->SetProperty("COMPILE_DEFINITIONS",
+                           "CMAKE_UNITY_CONFIG_$<UPPER_CASE:$<CONFIG>>");
+      }
     }
   }
 }

Source/cmLocalGenerator.h

@@ -11,8 +11,11 @@
 #include <set>
 #include <string>
 #include <unordered_map>
+#include <utility>
 #include <vector>
 
+#include <cm/optional>
+
 #include <cm3p/kwiml/int.h>
 
 #include "cmCustomCommandTypes.h"
@@ -28,7 +31,6 @@ class cmComputeLinkInformation;
 class cmCustomCommand;
 class cmCustomCommandGenerator;
 class cmCustomCommandLines;
-class cmGeneratedFileStream;
 class cmGeneratorTarget;
 class cmGlobalGenerator;
 class cmImplicitDependsList;
@@ -40,6 +42,9 @@ class cmState;
 class cmTarget;
 class cmake;
 
+template <typename Iter>
+class cmRange;
+
 /** Flag if byproducts shall also be considered.  */
 enum class cmSourceOutputKind
 {
@@ -657,18 +662,48 @@ private:
                          const std::string& ReuseFrom,
                          cmGeneratorTarget* reuseTarget,
                          std::vector<std::string> const& extensions);
-  void IncludeFileInUnitySources(cmGeneratedFileStream& unity_file,
-                                 std::string const& sf_full_path,
-                                 cmValue beforeInclude, cmValue afterInclude,
-                                 cmValue uniqueIdName) const;
-  std::vector<std::string> AddUnityFilesModeAuto(
+
+  struct UnityBatchedSource
+  {
+    cmSourceFile* Source = nullptr;
+    std::vector<size_t> Configs;
+    UnityBatchedSource(cmSourceFile* sf)
+      : Source(sf)
+    {
+    }
+  };
+  struct UnitySource
+  {
+    std::string Path;
+    bool PerConfig = false;
+    UnitySource(std::string path, bool perConfig)
+      : Path(std::move(path))
+      , PerConfig(perConfig)
+    {
+    }
+  };
+
+  UnitySource WriteUnitySource(
+    cmGeneratorTarget* target, std::vector<std::string> const& configs,
+    cmRange<std::vector<UnityBatchedSource>::const_iterator> sources,
+    cmValue beforeInclude, cmValue afterInclude, std::string filename) const;
+  void WriteUnitySourceInclude(std::ostream& unity_file,
+                               cm::optional<std::string> const& cond,
+                               std::string const& sf_full_path,
+                               cmValue beforeInclude, cmValue afterInclude,
+                               cmValue uniqueIdName) const;
+  std::vector<UnitySource> AddUnityFilesModeAuto(
     cmGeneratorTarget* target, std::string const& lang,
-    std::vector<cmSourceFile*> const& filtered_sources, cmValue beforeInclude,
-    cmValue afterInclude, std::string const& filename_base, size_t batchSize);
-  std::vector<std::string> AddUnityFilesModeGroup(
+    std::vector<std::string> const& configs,
+    std::vector<UnityBatchedSource> const& filtered_sources,
+    cmValue beforeInclude, cmValue afterInclude,
+    std::string const& filename_base, size_t batchSize);
+  std::vector<UnitySource> AddUnityFilesModeGroup(
     cmGeneratorTarget* target, std::string const& lang,
-    std::vector<cmSourceFile*> const& filtered_sources, cmValue beforeInclude,
-    cmValue afterInclude, std::string const& filename_base);
+    std::vector<std::string> const& configs,
+    std::vector<UnityBatchedSource> const& filtered_sources,
+    cmValue beforeInclude, cmValue afterInclude,
+    std::string const& filename_base);
 };
 
 #if !defined(CMAKE_BOOTSTRAP)

Source/cmVisualStudio10TargetGenerator.cxx

@@ -2351,7 +2351,7 @@ void cmVisualStudio10TargetGenerator::WriteAllSources(Elem& e0)
           // Visual Studio versions prior to 2017 15.8 do not know about unity
           // builds, thus we exclude the files already part of unity sources.
           if (!si.Source->GetPropertyAsBool("SKIP_UNITY_BUILD_INCLUSION")) {
-            exclude_configs = si.Configs;
+            exclude_configs = all_configs;
           }
         }
       }

Tests/RunCMake/UnityBuild/RunCMakeTest.cmake

@@ -26,6 +26,22 @@ run_build(unitybuild_anon_ns)
 run_build(unitybuild_anon_ns_no_unity_build)
 run_build(unitybuild_anon_ns_group_mode)
 
+function(run_per_config name)
+  set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${name}-build)
+  run_cmake(${name})
+  set(RunCMake_TEST_NO_CLEAN 1)
+  if(RunCMake_GENERATOR_IS_MULTI_CONFIG)
+    run_cmake_command(${name}-build-debug ${CMAKE_COMMAND} --build . --config Debug)
+    run_cmake_command(${name}-build-release ${CMAKE_COMMAND} --build . --config Release)
+  else()
+    run_cmake_command(${name}-build ${CMAKE_COMMAND} --build .)
+  endif()
+endfunction()
+
+if(NOT RunCMake_GENERATOR STREQUAL "Xcode")
+  run_per_config(per_config_c)
+endif()
+
 function(run_test name)
   set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${name}-build)
   run_cmake(${name})

Tests/RunCMake/UnityBuild/per_config_c.c

@@ -0,0 +1,16 @@
+#ifdef CFG_DEBUG
+extern void per_config_c_debug(void);
+#endif
+#ifdef CFG_OTHER
+extern void per_config_c_other(void);
+#endif
+int main(void)
+{
+#ifdef CFG_DEBUG
+  per_config_c_debug();
+#endif
+#ifdef CFG_OTHER
+  per_config_c_other();
+#endif
+  return 0;
+}

Tests/RunCMake/UnityBuild/per_config_c.cmake

@@ -0,0 +1,12 @@
+enable_language(C)
+
+add_executable(per_config_c per_config_c.c
+  "$<$<CONFIG:Debug>:per_config_c_debug.c>"
+  "$<$<NOT:$<CONFIG:Debug>>:per_config_c_other.c>"
+  )
+
+set_target_properties(per_config_c PROPERTIES UNITY_BUILD ON)
+target_compile_definitions(per_config_c PRIVATE
+  "$<$<CONFIG:Debug>:CFG_DEBUG>"
+  "$<$<NOT:$<CONFIG:Debug>>:CFG_OTHER>"
+  )

Tests/RunCMake/UnityBuild/per_config_c_debug.c

@@ -0,0 +1,9 @@
+#ifndef CFG_DEBUG
+#  error "per_config_c_debug built without CFG_DEBUG"
+#endif
+#ifdef CFG_OTHER
+#  error "per_config_c_debug built with CFG_OTHER"
+#endif
+void per_config_c_debug(void)
+{
+}

Tests/RunCMake/UnityBuild/per_config_c_other.c

@@ -0,0 +1,9 @@
+#ifdef CFG_DEBUG
+#  error "per_config_c_other built with CFG_DEBUG"
+#endif
+#ifndef CFG_OTHER
+#  error "per_config_c_other built without CFG_OTHER"
+#endif
+void per_config_c_other(void)
+{
+}