From f35e9c9f9baf41ea1c9d6dc67c91ad85820655f8 Mon Sep 17 00:00:00 2001 From: Nikolai Siukosev Date: Thu, 18 Jul 2024 16:25:05 +0400 Subject: [PATCH] Add move semantics for non-raii hpp handles. (#1919) --- README.md | 4 ++ VulkanHppGenerator.cpp | 47 ++++++++++-------- snippets/Exchange.hpp | 11 +++++ snippets/includes.hpp | 1 + tests/CMakeLists.txt | 1 + tests/HandlesMoveExchange/CMakeLists.txt | 19 +++++++ .../HandlesMoveExchange.cpp | 49 +++++++++++++++++++ 7 files changed, 112 insertions(+), 20 deletions(-) create mode 100644 snippets/Exchange.hpp create mode 100644 tests/HandlesMoveExchange/CMakeLists.txt create mode 100644 tests/HandlesMoveExchange/HandlesMoveExchange.cpp diff --git a/README.md b/README.md index a17d25d..5bda87c 100644 --- a/README.md +++ b/README.md @@ -892,6 +892,10 @@ When this is not externally defined and `VULKAN_HPP_CPP_VERSION` is at least `23 By default, the member `m_mask` of the `Flags` class template is private. This is to prevent accidentally setting a `Flags` with some inappropriate value. But it also prevents using a `Flags`, or a structure holding a `Flags`, to be used as a non-type template parameter. If you really need that functionality, and accept the reduced security, you can use this define to change the access specifier for `m_mask` from private to public, which allows using a `Flags` as a non-type template parameter. +#### VULKAN_HPP_HANDLES_MOVE_EXCHANGE + +This define can be used to enable `m_handle = exchange( rhs.m_handle, {} )` in move constructors of Vulkan-Hpp handles, which default-initializes the `rhs` underlying value. By default Vulkan-Hpp handles behave like trivial types -- move constructors copying value. + #### VULKAN_HPP_HASH_COMBINE This define can be used to specify your own hash combiner function. In order to determine the hash of a vk-structure, the hashes of the members of that struct are to be combined. This is done by this define, which by default is identical to what the function `boost::hash_combine()` does. It gets the type of the to-be-combined value, the seed, which is the combined value up to that point, and finally the to-be-combined value. This hash calculation determines a "shallow" hash, as it takes the hashes of any pointer in a structure, and not the hash of a pointed-to value. diff --git a/VulkanHppGenerator.cpp b/VulkanHppGenerator.cpp index 906972d..becdec6 100644 --- a/VulkanHppGenerator.cpp +++ b/VulkanHppGenerator.cpp @@ -458,6 +458,7 @@ ${UniqueHandle} ${DispatchLoaderBase} ${DispatchLoaderStatic} +${Exchange} #if !defined( VULKAN_HPP_NO_SMART_HANDLE ) ${ObjectDestroy} ${ObjectFree} @@ -532,6 +533,7 @@ ${DispatchLoaderDynamic} { "DispatchLoaderStatic", generateDispatchLoaderStatic() }, { "DynamicLoader", readSnippet( "DynamicLoader.hpp" ) }, { "Exceptions", readSnippet( "Exceptions.hpp" ) }, + { "Exchange", readSnippet( "Exchange.hpp" ) }, { "headerVersion", m_version }, { "includes", replaceWithMap( readSnippet( "includes.hpp" ), { { "vulkan_h", ( m_api == "vulkan" ) ? "vulkan.h" : "vulkan_sc_core.h" } } ) }, { "licenseHeader", m_vulkanLicenseHeader }, @@ -582,7 +584,7 @@ void VulkanHppGenerator::generateRAIIHppFile() const #define VULKAN_RAII_HPP #include // std::unique_ptr -#include // std::exchange, std::forward +#include // std::forward #include #if !defined( VULKAN_HPP_DISABLE_ENHANCED_MODE ) @@ -590,18 +592,6 @@ namespace VULKAN_HPP_NAMESPACE { namespace VULKAN_HPP_RAII_NAMESPACE { -# if ( 14 <= VULKAN_HPP_CPP_VERSION ) - using std::exchange; -# else - template - VULKAN_HPP_CONSTEXPR_14 VULKAN_HPP_INLINE T exchange( T & obj, U && newValue ) - { - T oldValue = std::move( obj ); - obj = std::forward( newValue ); - return oldValue; - } -# endif - template class CreateReturnType { @@ -5751,6 +5741,7 @@ std::string VulkanHppGenerator::generateCppModuleUsings() const auto const hardCodedEnhancedModeTypes = std::array{ "ArrayProxy", "ArrayProxyNoTemporaries", "StridedArrayProxy", "Optional", "StructureChain" }; auto const hardCodedSmartHandleTypes = std::array{ "ObjectDestroy", "ObjectFree", "ObjectRelease", "PoolFree", "ObjectDestroyShared", "ObjectFreeShared", "ObjectReleaseShared", "PoolFreeShared", "SharedHandle", "UniqueHandle" }; + auto const hardCodedFunctions = std::array{ "exchange" }; auto usings = std::string{ R"( //===================================== //=== HARDCODED TYPEs AND FUNCTIONs === @@ -5794,6 +5785,11 @@ std::string VulkanHppGenerator::generateCppModuleUsings() const const auto [enterNoSmartHandle, leaveNoSmartHandle] = generateProtection( "VULKAN_HPP_NO_SMART_HANDLE", false ); usings += "\n" + enterNoSmartHandle + noSmartHandleUsings + leaveNoSmartHandle + "\n"; + for ( auto const& functionName : hardCodedFunctions ) + { + usings += "\n" + replaceWithMap( usingTemplate, { { "className", std::string{ functionName } } } ); + } + // now generate baseTypes auto baseTypes = std::string{ R"( //================== @@ -5906,7 +5902,6 @@ std::string VulkanHppGenerator::generateCppModuleRaiiUsings() const //=== RAII HARDCODED === //====================== - using VULKAN_HPP_RAII_NAMESPACE::exchange; using VULKAN_HPP_RAII_NAMESPACE::Context; using VULKAN_HPP_RAII_NAMESPACE::ContextDispatcher; using VULKAN_HPP_RAII_NAMESPACE::InstanceDispatcher; @@ -7823,8 +7818,20 @@ ${enter} class ${className} ${className}() VULKAN_HPP_NOEXCEPT {}; // = default - try to workaround a compiler issue ${className}( ${className} const & rhs ) = default; ${className} & operator=( ${className} const & rhs ) = default; + +#if !defined(VULKAN_HPP_HANDLES_MOVE_EXCHANGE) ${className}( ${className} && rhs ) = default; ${className} & operator=( ${className} && rhs ) = default; +#else + ${className}( ${className} && rhs ) VULKAN_HPP_NOEXCEPT + : m_${memberName}( VULKAN_HPP_NAMESPACE::exchange( rhs.m_${memberName}, {} ) ) + {} + ${className} & operator=( ${className} && rhs ) VULKAN_HPP_NOEXCEPT + { + m_${memberName} = VULKAN_HPP_NAMESPACE::exchange( rhs.m_${memberName}, {} ); + return *this; + } +#endif VULKAN_HPP_CONSTEXPR ${className}( std::nullptr_t ) VULKAN_HPP_NOEXCEPT {} @@ -9939,7 +9946,7 @@ std::tuplesecond.params.front().name; clearMembers += "\n m_" + frontName + " = nullptr;"; - moveConstructorInitializerList = "m_" + frontName + "( VULKAN_HPP_NAMESPACE::VULKAN_HPP_RAII_NAMESPACE::exchange( rhs.m_" + frontName + ", {} ) ), "; + moveConstructorInitializerList = "m_" + frontName + "( VULKAN_HPP_NAMESPACE::exchange( rhs.m_" + frontName + ", {} ) ), "; moveAssignmentInstructions = "\n std::swap( m_" + frontName + ", rhs.m_" + frontName + " );"; memberVariables = "\n VULKAN_HPP_NAMESPACE::" + stripPrefix( frontType, "Vk" ) + " m_" + frontName + " = {};"; swapMembers = "\n std::swap( m_" + frontName + ", rhs.m_" + frontName + " );"; releaseMembers += "\n m_" + frontName + " = nullptr;"; } clearMembers += "\n m_" + handleName + " = nullptr;"; - moveConstructorInitializerList += "m_" + handleName + "( VULKAN_HPP_NAMESPACE::VULKAN_HPP_RAII_NAMESPACE::exchange( rhs.m_" + handleName + ", {} ) ), "; + moveConstructorInitializerList += "m_" + handleName + "( VULKAN_HPP_NAMESPACE::exchange( rhs.m_" + handleName + ", {} ) ), "; moveAssignmentInstructions += "\n std::swap( m_" + handleName + ", rhs.m_" + handleName + " );"; memberVariables += "\n " + generateNamespacedType( handle.first ) + " m_" + handleName + " = {};"; swapMembers += "\n std::swap( m_" + handleName + ", rhs.m_" + handleName + " );"; @@ -9985,7 +9992,7 @@ std::tuple + VULKAN_HPP_CONSTEXPR_14 VULKAN_HPP_INLINE T exchange( T & obj, U && newValue ) + { + T oldValue = std::move( obj ); + obj = std::forward( newValue ); + return oldValue; + } +#endif diff --git a/snippets/includes.hpp b/snippets/includes.hpp index a273b49..3433ad0 100644 --- a/snippets/includes.hpp +++ b/snippets/includes.hpp @@ -2,6 +2,7 @@ #include // ArrayWrapperND #include // strnlen #include // std::string +#include // std::exchange #include #include diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9b8446a..d5252e5 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -30,6 +30,7 @@ add_subdirectory( ExtensionInspection ) add_subdirectory( Flags ) add_subdirectory( FormatTraits ) add_subdirectory( Handles ) +add_subdirectory( HandlesMoveExchange ) add_subdirectory( Hash ) add_subdirectory( NoExceptions ) if( ( "cxx_std_23" IN_LIST CMAKE_CXX_COMPILE_FEATURES ) AND NOT ( ( CMAKE_CXX_COMPILER_ID STREQUAL "Clang" ) AND ( CMAKE_CXX_COMPILER_VERSION VERSION_LESS 15.0 ) ) ) diff --git a/tests/HandlesMoveExchange/CMakeLists.txt b/tests/HandlesMoveExchange/CMakeLists.txt new file mode 100644 index 0000000..fdfdee5 --- /dev/null +++ b/tests/HandlesMoveExchange/CMakeLists.txt @@ -0,0 +1,19 @@ +# Copyright(c) 2024, NVIDIA CORPORATION. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +if( NOT VULKAN_HPP_TESTS_BUILD_ONLY_DYNAMIC ) + find_package( Vulkan REQUIRED ) + + vulkan_hpp__setup_test( NAME HandlesMoveExchange LIBRARIES ${Vulkan_LIBRARIES} ) +endif() diff --git a/tests/HandlesMoveExchange/HandlesMoveExchange.cpp b/tests/HandlesMoveExchange/HandlesMoveExchange.cpp new file mode 100644 index 0000000..8a452ce --- /dev/null +++ b/tests/HandlesMoveExchange/HandlesMoveExchange.cpp @@ -0,0 +1,49 @@ +// Copyright(c) 2024, NVIDIA CORPORATION. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#undef VULKAN_HPP_DISPATCH_LOADER_DYNAMIC +#define VULKAN_HPP_DISPATCH_LOADER_DYNAMIC 0 + +#define VULKAN_HPP_HANDLES_MOVE_EXCHANGE + +#include +#include + +int main( int /*argc*/, char ** /*argv*/ ) +{ + try + { + vk::Instance instance; + instance = vk::createInstance( {} ); + assert( instance != nullptr ); + + vk::Instance anotherInstance = std::move( instance ); + assert( instance == nullptr ); + assert( anotherInstance != nullptr ); + + anotherInstance.destroy(); + } + catch ( vk::SystemError const & err ) + { + std::cout << "vk::SystemError: " << err.what() << std::endl; + exit( -1 ); + } + catch ( ... ) + { + std::cout << "unknown error\n"; + exit( -1 ); + } + + return 0; +}