Merge topic 'unity-anon-ns'

0fe9c40494 Unity Build: Add option for generating per-file unique id

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

13 files changed, 172 insertions, 3 deletions

diff --git a/Help/manual/cmake-properties.7.rst b/Help/manual/cmake-properties.7.rst
index 99d4c42616..5dbc1a40ae 100644
--- a/Help/manual/cmake-properties.7.rst
+++ b/Help/manual/cmake-properties.7.rst
@@ -354,6 +354,7 @@ Properties on Targets
    /prop_tgt/UNITY_BUILD_CODE_AFTER_INCLUDE
    /prop_tgt/UNITY_BUILD_CODE_BEFORE_INCLUDE
    /prop_tgt/UNITY_BUILD_MODE
+   /prop_tgt/UNITY_BUILD_UNIQUE_ID
    /prop_tgt/VERSION
    /prop_tgt/VISIBILITY_INLINES_HIDDEN
    /prop_tgt/VS_CONFIGURATION_TYPE
diff --git a/Help/manual/cmake-variables.7.rst b/Help/manual/cmake-variables.7.rst
index 4b2aab9409..b6764acdba 100644
--- a/Help/manual/cmake-variables.7.rst
+++ b/Help/manual/cmake-variables.7.rst
@@ -470,6 +470,7 @@ Variables that Control the Build
    /variable/CMAKE_TRY_COMPILE_TARGET_TYPE
    /variable/CMAKE_UNITY_BUILD
    /variable/CMAKE_UNITY_BUILD_BATCH_SIZE
+   /variable/CMAKE_UNITY_BUILD_UNIQUE_ID
    /variable/CMAKE_USE_RELATIVE_PATHS
    /variable/CMAKE_VISIBILITY_INLINES_HIDDEN
    /variable/CMAKE_VS_GLOBALS
diff --git a/Help/prop_tgt/UNITY_BUILD.rst b/Help/prop_tgt/UNITY_BUILD.rst
index 04cede61ef..f827a20a19 100644
--- a/Help/prop_tgt/UNITY_BUILD.rst
+++ b/Help/prop_tgt/UNITY_BUILD.rst
@@ -70,6 +70,11 @@ a number of measures to help address such problems:
   problems with specific files than disabling unity builds for an entire
   target.
 
+* Projects can set :prop_tgt:`UNITY_BUILD_UNIQUE_ID` to cause a valid
+  C-identifier to be generated which is unique per file in a unity
+  build.  This can be used to avoid problems with anonymous namespaces
+  in unity builds.
+
 * The :prop_tgt:`UNITY_BUILD_CODE_BEFORE_INCLUDE` and
   :prop_tgt:`UNITY_BUILD_CODE_AFTER_INCLUDE` target properties can be used
   to inject code into the unity source files before and after every
diff --git a/Help/prop_tgt/UNITY_BUILD_UNIQUE_ID.rst b/Help/prop_tgt/UNITY_BUILD_UNIQUE_ID.rst
new file mode 100644
index 0000000000..e0ee30e537
--- /dev/null
+++ b/Help/prop_tgt/UNITY_BUILD_UNIQUE_ID.rst
@@ -0,0 +1,53 @@
+UNITY_BUILD_UNIQUE_ID
+---------------------
+
+The name of a valid C-identifier which is set to a unique per-file
+value during unity builds.
+
+When this property is populated and when :prop_tgt:`UNITY_BUILD`
+is true, the property value is used to define a compiler definition
+of the specified name. The value of the defined symbol is unspecified,
+but it is unique per file path.
+
+Given:
+
+.. code-block:: cmake
+
+  set_target_properties(myTarget PROPERTIES
+    UNITY_BUILD "ON"
+    UNITY_BUILD_UNIQUE_ID "MY_UNITY_ID"
+  )
+
+the ``MY_UNITY_ID`` symbol is defined to a unique per-file value.
+
+One known use case for this identifier is to disambiguate the
+variables in an anonymous namespace in a limited scope.
+Anonymous namespaces present a problem for unity builds because
+they are used to ensure that certain variables and declarations
+are scoped to a translation unit which is approximated by a
+single source file.  When source files are combined in a unity
+build file, those variables in different files are combined in
+a single translation unit and the names clash.  This property can
+be used to avoid that with code like the following:
+
+.. code-block:: cpp
+
+  // Needed for when unity builds are disabled
+  #ifndef MY_UNITY_ID
+  #define MY_UNITY_ID
+  #endif
+
+  namespace { namespace MY_UNITY_ID {
+    // The name 'i' clashes (or could clash) with other
+    // variables in other anonymous namespaces
+    int i = 42;
+  }}
+
+  int use_var()
+  {
+    return MY_UNITY_ID::i;
+  }
+
+The pseudononymous namespace is used within a truly anonymous namespace.
+On many platforms, this maintains the invariant that the symbols within
+do not get external linkage when performing a unity build.
diff --git a/Help/release/dev/unity-build-anonymous-macros.rst b/Help/release/dev/unity-build-anonymous-macros.rst
new file mode 100644
index 0000000000..ed8fb1a530
--- /dev/null
+++ b/Help/release/dev/unity-build-anonymous-macros.rst
@@ -0,0 +1,7 @@
+unity-build-anonymous-macros
+----------------------------
+
+* The :prop_tgt:`UNITY_BUILD_UNIQUE_ID` target property
+  was added to support generation of an identifier that is
+  unique per source file in unity builds.  It can help to
+  resolve duplicate symbol problems with anonymous namespaces.
diff --git a/Help/variable/CMAKE_UNITY_BUILD_UNIQUE_ID.rst b/Help/variable/CMAKE_UNITY_BUILD_UNIQUE_ID.rst
new file mode 100644
index 0000000000..b30e443cff
--- /dev/null
+++ b/Help/variable/CMAKE_UNITY_BUILD_UNIQUE_ID.rst
@@ -0,0 +1,6 @@
+CMAKE_UNITY_BUILD_UNIQUE_ID
+---------------------------
+
+This variable is used to initialize the :prop_tgt:`UNITY_BUILD_UNIQUE_ID`
+property of targets when they are created.  It specifies the name of the
+unique identifier generated per file in a unity build.
diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx
index d3ede5c424..b329e4b9ba 100644
--- a/Source/cmLocalGenerator.cxx
+++ b/Source/cmLocalGenerator.cxx
@@ -2772,8 +2772,15 @@ inline void RegisterUnitySources(cmGeneratorTarget* target, cmSourceFile* sf,
 inline void IncludeFileInUnitySources(cmGeneratedFileStream& unity_file,
                                       std::string const& sf_full_path,
                                       cmProp beforeInclude,
-                                      cmProp afterInclude)
+                                      cmProp afterInclude, cmProp uniqueIdName)
 {
+
+  if (uniqueIdName && !uniqueIdName->empty()) {
+    unity_file << "#undef " << *uniqueIdName << "\n"
+               << "#define " << *uniqueIdName << " unity_"
+               << cmSystemTools::ComputeStringMD5(sf_full_path) << "\n";
+  }
+
   if (beforeInclude) {
     unity_file << *beforeInclude << "\n";
   }
@@ -2794,6 +2801,8 @@ std::vector<std::string> AddUnityFilesModeAuto(
     batchSize = filtered_sources.size();
   }
 
+  cmProp uniqueIdName = target->GetProperty("UNITY_BUILD_UNIQUE_ID");
+
   std::vector<std::string> unity_files;
   for (size_t itemsLeft = filtered_sources.size(), chunk, batch = 0;
        itemsLeft > 0; itemsLeft -= chunk, ++batch) {
@@ -2817,7 +2826,7 @@ std::vector<std::string> AddUnityFilesModeAuto(
         cmSourceFile* sf = filtered_sources[begin];
         RegisterUnitySources(target, sf, filename);
         IncludeFileInUnitySources(file, sf->ResolveFullPath(), beforeInclude,
-                                  afterInclude);
+                                  afterInclude, uniqueIdName);
       }
     }
     cmSystemTools::MoveFileIfDifferent(filename_tmp, filename);
@@ -2848,6 +2857,8 @@ std::vector<std::string> AddUnityFilesModeGroup(
     }
   }
 
+  cmProp 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,
@@ -2863,7 +2874,7 @@ std::vector<std::string> AddUnityFilesModeGroup(
       for (cmSourceFile* sf : item.second) {
         RegisterUnitySources(target, sf, filename);
         IncludeFileInUnitySources(file, sf->ResolveFullPath(), beforeInclude,
-                                  afterInclude);
+                                  afterInclude, uniqueIdName);
       }
     }
     cmSystemTools::MoveFileIfDifferent(filename_tmp, filename);
diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx
index 778fb89d31..017ab10bfc 100644
--- a/Source/cmTarget.cxx
+++ b/Source/cmTarget.cxx
@@ -380,6 +380,7 @@ cmTarget::cmTarget(std::string const& name, cmStateEnums::TargetType type,
     initProp("VS_JUST_MY_CODE_DEBUGGING");
     initProp("DISABLE_PRECOMPILE_HEADERS");
     initProp("UNITY_BUILD");
+    initProp("UNITY_BUILD_UNIQUE_ID");
     initProp("OPTIMIZE_DEPENDENCIES");
     initPropValue("UNITY_BUILD_BATCH_SIZE", "8");
     initPropValue("UNITY_BUILD_MODE", "BATCH");
diff --git a/Tests/RunCMake/UnityBuild/RunCMakeTest.cmake b/Tests/RunCMake/UnityBuild/RunCMakeTest.cmake
index 9ba3c856db..c00f78b3dc 100644
--- a/Tests/RunCMake/UnityBuild/RunCMakeTest.cmake
+++ b/Tests/RunCMake/UnityBuild/RunCMakeTest.cmake
@@ -1,5 +1,14 @@
 include(RunCMake)
 
+function(run_build name)
+  set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${name}-build)
+  set(RunCMake_TEST_NO_CLEAN 1)
+  run_cmake(${name})
+  run_cmake_command(${name}-build ${CMAKE_COMMAND} --build . --config Debug)
+  unset(RunCMake_TEST_BINARY_DIR)
+  unset(RunCMake_TEST_NO_CLEAN)
+endfunction()
+
 run_cmake(unitybuild_c)
 run_cmake(unitybuild_c_batch)
 run_cmake(unitybuild_c_group)
@@ -15,6 +24,9 @@ run_cmake(unitybuild_c_no_unity_build)
 run_cmake(unitybuild_c_no_unity_build_group)
 run_cmake(unitybuild_order)
 run_cmake(unitybuild_invalid_mode)
+run_build(unitybuild_anon_ns)
+run_build(unitybuild_anon_ns_no_unity_build)
+run_build(unitybuild_anon_ns_group_mode)
 
 function(run_test name)
   set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${name}-build)
diff --git a/Tests/RunCMake/UnityBuild/unitybuild_anon_ns.cmake b/Tests/RunCMake/UnityBuild/unitybuild_anon_ns.cmake
new file mode 100644
index 0000000000..6f4878f0c3
--- /dev/null
+++ b/Tests/RunCMake/UnityBuild/unitybuild_anon_ns.cmake
@@ -0,0 +1,11 @@
+project(unitybuild_anon_ns CXX)
+
+include(${CMAKE_CURRENT_SOURCE_DIR}/unitybuild_anon_ns_test_files.cmake)
+
+write_unity_build_anon_ns_test_files(srcs)
+
+add_library(tgt SHARED ${srcs})
+
+set_target_properties(tgt PROPERTIES UNITY_BUILD ON)
+
+set_target_properties(tgt PROPERTIES UNITY_BUILD_UNIQUE_ID MY_ANON_ID)
diff --git a/Tests/RunCMake/UnityBuild/unitybuild_anon_ns_group_mode.cmake b/Tests/RunCMake/UnityBuild/unitybuild_anon_ns_group_mode.cmake
new file mode 100644
index 0000000000..a91fc745d6
--- /dev/null
+++ b/Tests/RunCMake/UnityBuild/unitybuild_anon_ns_group_mode.cmake
@@ -0,0 +1,19 @@
+project(unitybuild_anon_ns_group_mode CXX)
+
+include(${CMAKE_CURRENT_SOURCE_DIR}/unitybuild_anon_ns_test_files.cmake)
+
+write_unity_build_anon_ns_test_files(srcs)
+
+add_library(tgt SHARED ${srcs})
+
+set_target_properties(tgt PROPERTIES UNITY_BUILD ON
+                                     UNITY_BUILD_MODE GROUP)
+
+set_target_properties(tgt PROPERTIES UNITY_BUILD_UNIQUE_ID MY_ANON_ID)
+
+set_source_files_properties(s1.cpp s2.cpp s5.cpp s7.cpp s8.cpp
+                            PROPERTIES UNITY_GROUP "a"
+                            )
+set_source_files_properties(s3.cpp s4.cpp s6.cpp
+                            PROPERTIES UNITY_GROUP "b"
+                            )
diff --git a/Tests/RunCMake/UnityBuild/unitybuild_anon_ns_no_unity_build.cmake b/Tests/RunCMake/UnityBuild/unitybuild_anon_ns_no_unity_build.cmake
new file mode 100644
index 0000000000..9f0a9e1d66
--- /dev/null
+++ b/Tests/RunCMake/UnityBuild/unitybuild_anon_ns_no_unity_build.cmake
@@ -0,0 +1,11 @@
+project(unitybuild_anon_ns_no_unity_build CXX)
+
+include(${CMAKE_CURRENT_SOURCE_DIR}/unitybuild_anon_ns_test_files.cmake)
+
+write_unity_build_anon_ns_test_files(srcs)
+
+add_library(tgt SHARED ${srcs})
+
+set_target_properties(tgt PROPERTIES UNITY_BUILD OFF)
+
+set_target_properties(tgt PROPERTIES UNITY_BUILD_UNIQUE_ID MY_ANON_ID)
diff --git a/Tests/RunCMake/UnityBuild/unitybuild_anon_ns_test_files.cmake b/Tests/RunCMake/UnityBuild/unitybuild_anon_ns_test_files.cmake
new file mode 100644
index 0000000000..06a600e2ee
--- /dev/null
+++ b/Tests/RunCMake/UnityBuild/unitybuild_anon_ns_test_files.cmake
@@ -0,0 +1,31 @@
+
+function(write_unity_build_anon_ns_test_files OUTVAR)
+  set(srcs)
+  foreach(s RANGE 1 8)
+    set(src "${CMAKE_CURRENT_BINARY_DIR}/s${s}.cpp")
+    file(WRITE "${src}" "
+#ifndef CONFIG_H
+#define CONFIG_H
+#define MY_ANON_NAMESPACE MY_ANON_ID
+#define MY_ANON(Name) MY_ANON_NAMESPACE::Name
+#define MY_ANON_USING_NAMESPACE using namespace MY_ANON_NAMESPACE
+#endif
+
+namespace { namespace MY_ANON_NAMESPACE {
+  int i = ${s};
+}}
+int use_plain_${s}() {
+  return MY_ANON_NAMESPACE::i;
+}
+int func_like_macro_${s}() {
+  return MY_ANON(i);
+}
+int using_macro_${s}() {
+  MY_ANON_USING_NAMESPACE;
+  return i;
+}
+")
+    list(APPEND srcs "${src}")
+  endforeach()
+  set(${OUTVAR} ${srcs} PARENT_SCOPE)
+endfunction()