From 0fe9c40494be0e15e6603a245f181bc74d8cf481 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sat, 23 May 2020 17:16:49 +0100 Subject: Unity Build: Add option for generating per-file unique id Fixes: #21477 --- Help/manual/cmake-properties.7.rst | 1 + Help/manual/cmake-variables.7.rst | 1 + Help/prop_tgt/UNITY_BUILD.rst | 5 ++ Help/prop_tgt/UNITY_BUILD_UNIQUE_ID.rst | 53 ++++++++++++++++++++++ Help/release/dev/unity-build-anonymous-macros.rst | 7 +++ Help/variable/CMAKE_UNITY_BUILD_UNIQUE_ID.rst | 6 +++ Source/cmLocalGenerator.cxx | 17 +++++-- Source/cmTarget.cxx | 1 + Tests/RunCMake/UnityBuild/RunCMakeTest.cmake | 12 +++++ Tests/RunCMake/UnityBuild/unitybuild_anon_ns.cmake | 11 +++++ .../UnityBuild/unitybuild_anon_ns_group_mode.cmake | 19 ++++++++ .../unitybuild_anon_ns_no_unity_build.cmake | 11 +++++ .../UnityBuild/unitybuild_anon_ns_test_files.cmake | 31 +++++++++++++ 13 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 Help/prop_tgt/UNITY_BUILD_UNIQUE_ID.rst create mode 100644 Help/release/dev/unity-build-anonymous-macros.rst create mode 100644 Help/variable/CMAKE_UNITY_BUILD_UNIQUE_ID.rst create mode 100644 Tests/RunCMake/UnityBuild/unitybuild_anon_ns.cmake create mode 100644 Tests/RunCMake/UnityBuild/unitybuild_anon_ns_group_mode.cmake create mode 100644 Tests/RunCMake/UnityBuild/unitybuild_anon_ns_no_unity_build.cmake create mode 100644 Tests/RunCMake/UnityBuild/unitybuild_anon_ns_test_files.cmake diff --git a/Help/manual/cmake-properties.7.rst b/Help/manual/cmake-properties.7.rst index 33af9f7..96c186f 100644 --- a/Help/manual/cmake-properties.7.rst +++ b/Help/manual/cmake-properties.7.rst @@ -353,6 +353,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 f898ae9..838aa31 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 04cede6..f827a20 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 0000000..e0ee30e --- /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 0000000..ed8fb1a --- /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 0000000..b30e443 --- /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 9f9d725..be28522 100644 --- a/Source/cmLocalGenerator.cxx +++ b/Source/cmLocalGenerator.cxx @@ -2753,8 +2753,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"; } @@ -2775,6 +2782,8 @@ std::vector AddUnityFilesModeAuto( batchSize = filtered_sources.size(); } + cmProp uniqueIdName = target->GetProperty("UNITY_BUILD_UNIQUE_ID"); + std::vector unity_files; for (size_t itemsLeft = filtered_sources.size(), chunk, batch = 0; itemsLeft > 0; itemsLeft -= chunk, ++batch) { @@ -2798,7 +2807,7 @@ std::vector AddUnityFilesModeAuto( cmSourceFile* sf = filtered_sources[begin]; RegisterUnitySources(target, sf, filename); IncludeFileInUnitySources(file, sf->ResolveFullPath(), beforeInclude, - afterInclude); + afterInclude, uniqueIdName); } } cmSystemTools::MoveFileIfDifferent(filename_tmp, filename); @@ -2829,6 +2838,8 @@ std::vector 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, @@ -2844,7 +2855,7 @@ std::vector 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 9db5dc6..4bb8737 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -379,6 +379,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 9ba3c85..c00f78b 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 0000000..6f4878f --- /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 0000000..a91fc74 --- /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 0000000..9f0a9e1 --- /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 0000000..06a600e --- /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() -- cgit v0.12