Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support of noexcept-path for vk::raii classes #1742

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

asuessenbach
Copy link
Contributor

@asuessenbach asuessenbach commented Nov 30, 2023

Resolves #1498.

@asuessenbach asuessenbach force-pushed the noexcept branch 8 times, most recently from 6fbb733 to 249070f Compare November 30, 2023 12:45
@theHamsta
Copy link
Contributor

theHamsta commented Dec 1, 2023

I think the failures for clang-13 are because the compiler does not properly support c++23 yet and requires the typename before the template (which might become obsolete with c++23).

Other failures with newer versions of clang are because the compiler doesn't support __cpp_lib_expected because it also does not fully support concepts yet (the gcc header tests for __cpp_concepts). theHamsta#21 seems to compile almost fine https://github.com/theHamsta/Vulkan-Hpp/actions/runs/7063965682/job/19231032347?pr=21. Vulkan-Hpp headers seem to miss <memory> for unique ptr in the raii headers. clang++-13 compiles locally with these changes fine on my machine.

Overwriting VULKAN_HPP_EXPECTED/VULKAN_HPP_UNEXPECTED might not be possible with older c++ standard as template type deducation is not that advanced D:\a\Vulkan-Hpp\Vulkan-Hpp\vulkan/vulkan_raii.hpp(17380,16): error C2955: 'tl::unexpected': use of class template requires template argument list (they might need tl::make_expected, std::make_pair etc.).

@asuessenbach asuessenbach force-pushed the noexcept branch 6 times, most recently from 5923659 to 26a50d4 Compare December 4, 2023 13:24
@@ -265,3 +266,20 @@ namespace VULKAN_HPP_NAMESPACE
# define VULKAN_HPP_DEFAULT_ARGUMENT_NULLPTR_ASSIGNMENT = nullptr
# define VULKAN_HPP_DEFAULT_DISPATCHER_ASSIGNMENT = VULKAN_HPP_DEFAULT_DISPATCHER
#endif

#if !defined( VULKAN_HPP_EXPECTED ) && ( 23 <= VULKAN_HPP_CPP_VERSION ) && ( __has_include( <expected>))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use the feature test macro __cpp_lib_expected. Clang has <expected> via GCC, but cannot use the feature due to missing full support for concepts.

@asuessenbach asuessenbach merged commit 197017e into KhronosGroup:main Jan 2, 2024
102 checks passed
@asuessenbach asuessenbach deleted the noexcept branch January 2, 2024 12:47
@sharadhr
Copy link
Contributor

sharadhr commented Jan 2, 2024

Ah, seems this has been merged. I saw this discussion on Reddit, and one comment in particular seems relevant:

If you're calling a private constructor without using this pattern you are likely incurring an extra copy or move constructor. The basic problem is that you want to return an object of type std::expected<T, E> from the public static function. In order to construct the return object, the constructor of std::expected<T, E> needs to initialize the object of type T in its storage, ideally by forwarding the arguments you provide to construct the T object in place. But std::expected<T, E> can't call private constructors, so the passkey idiom is useful to make a constructor that any callsite can invoke, but that takes a passkey parameter that only members of the class T can create.

It seems one possible way around this is to either use the passkey idiom, or declare std::expected as a friend of the classes we want to protect. I suppose we might choose the latter, since we want to still allow users to directly construct vk::raii::Things...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add RAII support with VULKAN_HPP_NO_EXCEPTIONS
3 participants