Add RAII support with VULKAN_HPP_NO_EXCEPTIONS

[jmacnak]

Google / Android does not use exceptions (see here). However, the RAII classes are very useful!

Could we update the RAII generator to guard the throwing constructors for handle types using:

#ifndef VULKAN_HPP_NO_EXCEPTIONS
  Image(...)
  {
     ...
     if (result != success)
     {
       throwResultException(...);
     }
  }
#endif

guard the throwResultException() function definition, and introduce static "create" functions that return a std::expected<HandleType, vk::Result> using:

  static std::expected<Image, VULKAN_HPP_NAMESPACE::Result> create(...)
  {
    ...
    if (result != success)
    {
      return std::unexpected(result);
    }
    return Image(...);
  }

Bonus points if we can use custom defines such as VULKAN_HPP_RAII_EXPECTED_CLASS /VULKAN_HPP_RAII_UNEXPECTED_CLASS to use custom "expected" implementations when compiling for C++ version < 23 (for example, android::base::expected). If these VULKAN_HPP_RAII_EXPECTED_CLASS /VULKAN_HPP_RAII_UNEXPECTED_CLASS defines are not defined, potentially need to guard the create() functions based on the version.

See https://android-review.googlesource.com/c/device/google/cuttlefish/+/2404731 for a quick attempt at this.

[asuessenbach]

That's an interesting approach, indeed! Thanks a lot for this suggestion.
Unfortunately, that std::expected is a C++23 feature, but it would be a perfect fit for that situation.
What would you think about simply returning a std::pair<vk::raii::Image, vk::Result>? Not as elegant as std::expected, but available on pretty much every platform.

Besides the constructors, there are also all those functions that might throw on error. When introducing VULKAN_HPP_NO_EXCEPTIONS to the raii-classes, those functions would have to be changed to return something plus the result code.

[phire]

I did some experiments on top of @jmacnak's code (see my WIP branch is here. No guarantee it actually runs)

With a careful choice of defines, it's possible to make the it work with both the std::expected interface and something more basic like a std::tuple

// with tuple
#define VULKAN_HPP_RAII_EXPECTED_CLASS std::tuple
#define VULKAN_HPP_RAII_EXPECTED(obj) std::make_tuple(obj, VULKAN_HPP_NAMESPACE::Result::eSuccess )
#define VULKAN_HPP_RAII_UNEXPECTED(type, error) std::make_tuple(type(nullptr), error)
// with std::expect (untested)
#define VULKAN_HPP_RAII_EXPECTED_CLASS std::expect
#define VULKAN_HPP_RAII_EXPECTED(obj) obj
#define VULKAN_HPP_RAII_UNEXPECTED(type, error) std::unexpected(error)

However, I spent some time porting my codebase over using tuples for error returns, and it's less than ideal. Very tempting to use std::tie, which made it very easy for the nullptr constructed RAII objects to escape into the rest of application's codebase. Feels unclean and I'm very tempted to just borrow android's implementation of std::expected.

But an alternative proposal for supporting older versions of C++
What if RAII provided it's own vk::ValueResult (or whatever it's named) that implements a usable subset of the std::expected interface. Make it hard-coded to vk::Result and supply the basic accessors. Then when the headers are compiled with a c++23 compiler (or a supplied alternative implementation of std::expected) they can implement vk::ValueResult as just a type alias and the user will get the full functionality of std::expected, without breaking old code.

[asuessenbach]

What if RAII provided it's own vk::ValueResult (or whatever it's named) that implements a usable subset of the std::expected interface.

Yep, that's the way to go!

[phire]

I might have a go at implementing it and see if I get something workable.

vk::ResultValue already exists, so we might need name it vk::Expected.

I figure keep it out of the vk::raii namespace because it's not really an RAII object, and maybe we can eventually backport it to Vulkan.hpp as a configurable replacement for ResultValue.

[Ahajha]

Interested in this as well!

[Arthapz]

same here

[sharadhr]

C++23 has been finalised, and std::expected support has landed in all three major compilers (g++, Clang, MSVC) and their corresponding libraries.

Perhaps Vulkan-Hpp could consider the other option: simply add overloads for every function/constructor returning vk::Result or any function that could throw, declare them noexcept, and return a value wrapped in a std::expected, instead of re-inventing the wheel.

[sharadhr]

@asuessenbach, any thoughts on this discussion since, or my suggestion?

[asuessenbach]

Well, I think we have waited long enough, 'til std::expected has landed ;)

The main problem with VULKAN_HPP_NO_EXCEPTIONS are the constructors of the vk::raii classes. As they potentially throw, they would have to be guarded with some #ifndef VULKAN_HPP_NO_EXCEPTIONS.

Only the create-functions would need some substantial modifications.
Instead of just calling the constructor of the vk::raii-object, it would need to call the actual vkCreate-function and could look like this:

    VULKAN_HPP_NODISCARD VULKAN_HPP_INLINE VULKAN_HPP_RAII_CREATION_RETURN( VULKAN_HPP_RAII_NAMESPACE::Instance ) Context::createInstance(
      VULKAN_HPP_NAMESPACE::InstanceCreateInfo const &                                createInfo,
      VULKAN_HPP_NAMESPACE::Optional<const VULKAN_HPP_NAMESPACE::AllocationCallbacks> allocator ) const VULKAN_HPP_RAII_NO_EXCEPT
    {
      VkInstance                   instance;
      VULKAN_HPP_NAMESPACE::Result result = static_cast<VULKAN_HPP_NAMESPACE::Result>( vkCreateInstance(
        reinterpret_cast<const VkInstanceCreateInfo *>( &createInfo ), reinterpret_cast<const VkAllocationCallbacks *>( &allocator ), &instance ) );
      if ( result != VULKAN_HPP_NAMESPACE::Result::eSuccess )
      {
#  ifdef VULKAN_HPP_NO_EXCEPTIONS
        return std::unexpected( result );
#  else
        detail::throwResultException( result, "vkCreateInstance" );
#  endif
      }
      return VULKAN_HPP_RAII_NAMESPACE::Instance( *this, instance, allocator );
    }

With some new macros like

#  ifdef VULKAN_HPP_NO_EXCEPTIONS
#    define VULKAN_HPP_RAII_CREATION_RETURN( Type ) std::expected<Type, vk::Result>
#    define VULKAN_HPP_RAII_NO_EXCEPT               noexcept
#  else
#    define VULKAN_HPP_RAII_CREATION_RETURN( Type ) Type
#    define VULKAN_HPP_RAII_NO_EXCEPT
#  endif

There are some functions throwing a LogicError. That would have to be changed to a VULKAN_HPP_ASSERT, just as it's done in the vk-namespace, like this:

#  ifdef VULKAN_HPP_NO_EXCEPTIONS
    VULKAN_HPP_ASSERT( buffers.size() == offsets.size() );
#  else
    if ( buffers.size() != offsets.size() )
    {
      throw LogicError( VULKAN_HPP_NAMESPACE_STRING "::CommandBuffer::bindVertexBuffers: buffers.size() != offsets.size()" );
    }
#  endif /*VULKAN_HPP_NO_EXCEPTIONS*/

Does that sound reasonable?

[sharadhr]

@asuessenbach,

That looks pretty good!

How would the constructors of the various RAII handles, e.g. vk::raii::Device, vk::raii::CommandBuffer, etc. look like? Is it correct to say that vk::raii::createXYZ (and all the corresponding createXYZ functions) will be called by XYZ::XYZ() as well, and simply return the result?

[asuessenbach]

I thought about backing the constructors by the construction functions as well. But I think, it's easier to just keep them as they currently are, calling the actual vkCreate-function and throwing on an error code.
That is VULKAN_HPP_NO_EXCEPTIONS would be available for the raii-classes only if std::expected (or maybe some VULKAN_HPP_EXPECTED_TYPE) is available.

[asuessenbach]

Some more elaborated version of the macros here could look like this:

#if !defined( VULKAN_HPP_RAII_EXPECTED_TYPE )
#  if defined( VULKAN_HPP_RAII_UNEXPECTED ) || defined( VULKAN_HPP_RAII_EXPECTED_INCLUDE )
#    error "vulkan.hpp: VULKAN_HPP_RAII_UNEXPECTED or VULKAN_HPP_RAII_EXPECTED_INCLUDE are externally defined, while VULKAN_HPP_RAII_EXPECTED_TYPE is not!"
#  endif
#  if defined( VULKAN_HPP_NO_EXCEPTIONS )
#    define VULKAN_HPP_RAII_CREATE_NOEXCEPT noexcept
#    if ( 23 <= VULKAN_HPP_CPP_VERSION )
#      define VULKAN_HPP_RAII_EXPECTED_INCLUDE           <expected>
#      define VULKAN_HPP_RAII_EXPECTED_TYPE( Type )      std::expected<Type, vk::Result>
#      define VULKAN_HPP_RAII_UNEXPECTED( Type, result ) std::unexpected( result )
#    elif ( 17 <= VULKAN_HPP_CPP_VERSION )
#      define VULKAN_HPP_RAII_EXPECTED_INCLUDE           <variant>
#      define VULKAN_HPP_RAII_EXPECTED_TYPE( Type )      std::variant<Type, vk::Result>
#      define VULKAN_HPP_RAII_UNEXPECTED( Type, result ) std::variant<Type, vk::Result>( result )
#    else
#      define VULKAN_HPP_RAII_EXPECTED_INCLUDE           <utility>
#      define VULKAN_HPP_RAII_EXPECTED_TYPE( Type )      std::pair<Type, vk::Result>
#      define VULKAN_HPP_RAII_UNEXPECTED( Type, result ) std::pair<Type, vk::Result>( nullptr, result )
#    endif
#  else
#    define VULKAN_HPP_RAII_CREATE_NOEXCEPT
#    define VULKAN_HPP_RAII_EXPECTED_TYPE( Type ) Type
#  endif
#else
#  if !defined( VULKAN_HPP_RAII_UNEXPECTED ) || !defined( VULKAN_HPP_RAII_EXPECTED_INCLUDE )
#    error "vulkan.hpp: VULKAN_HPP_RAII_UNEXPECTED or VULKAN_HPP_RAII_EXPECTED_INCLUDE are not externally defined, while VULKAN_HPP_RAII_EXPECTED_TYPE is!"
#  endif
#endif

That is, if you don't externally specify your expected type, some type is selected, depending on your C++-Version. The higher that version, the better that type fits to the task.
That way, you could even use this functionality with C++11, if you're desperate enough ;)

[asuessenbach]

Well, unfortunately the enumerating functions (like vk::raii::Instance::enumeratePhysicalDevices) can't easily be made exception free, as they're returning a std::vector of objects.

I have to admit, I have no good idea how to handle that.

In any case, if VULKAN_HPP_NO_EXCEPTIONS is defined, those functions would need to be removed.
To actually enumerate stuff one could

• let the user directly access the dispatcher and call the actual C-functions on his own; or
• provide a functions that looks exactly like the underlying C-functions, but with vk::raii-namespace arguments.

In both cases, the user would need to do the enumerating boiler-plate code on its own. And as the enumerated classes already have constructors taking the enumerated C-type and don't actually own that resource, it should at least work...
but none of those two approaches really pleases me.
Any other ideas?

[theHamsta]

Well, unfortunately the enumerating functions (like vk::raii::Instance::enumeratePhysicalDevices) can't easily be made exception free, as they're returning a std::vector of objects.

I have to admit, I have no good idea how to handle that.

In any case, if VULKAN_HPP_NO_EXCEPTIONS is defined, those functions would need to be removed. To actually enumerate stuff one could

* let the user directly access the dispatcher and call the actual C-functions on his own; or

* provide a functions that looks exactly like the underlying C-functions, but with vk::raii-namespace arguments.

In both cases, the user would need to do the enumerating boiler-plate code on its own. And as the enumerated classes already have constructors taking the enumerated C-type and don't actually own that resource, it should at least work... but none of those two approaches really pleases me. Any other ideas?

    class PhysicalDevices : public std::vector<VULKAN_HPP_NAMESPACE::VULKAN_HPP_RAII_NAMESPACE::PhysicalDevice>
    {
    public:
      PhysicalDevices( VULKAN_HPP_NAMESPACE::VULKAN_HPP_RAII_NAMESPACE::Instance const & instance )
      {
        VULKAN_HPP_NAMESPACE::VULKAN_HPP_RAII_NAMESPACE::InstanceDispatcher const * dispatcher = instance.getDispatcher();
        std::vector<VkPhysicalDevice>                                               physicalDevices;
        uint32_t                                                                    physicalDeviceCount;
        VULKAN_HPP_NAMESPACE::Result                                                result;
        do
        {
          result = static_cast<VULKAN_HPP_NAMESPACE::Result>(
            dispatcher->vkEnumeratePhysicalDevices( static_cast<VkInstance>( *instance ), &physicalDeviceCount, nullptr ) );
          if ( ( result == VULKAN_HPP_NAMESPACE::Result::eSuccess ) && physicalDeviceCount )
          {
            physicalDevices.resize( physicalDeviceCount );
            result = static_cast<VULKAN_HPP_NAMESPACE::Result>(
              dispatcher->vkEnumeratePhysicalDevices( static_cast<VkInstance>( *instance ), &physicalDeviceCount, physicalDevices.data() ) );
          }
        } while ( result == VULKAN_HPP_NAMESPACE::Result::eIncomplete );
        if ( result == VULKAN_HPP_NAMESPACE::Result::eSuccess )
        {
          VULKAN_HPP_ASSERT( physicalDeviceCount <= physicalDevices.size() );
          this->reserve( physicalDeviceCount );
          for ( auto const & physicalDevice : physicalDevices )
          {
            this->emplace_back( instance, physicalDevice );
          }
        }
        else
        {
          detail::throwResultException( result, "vkEnumeratePhysicalDevices" );
        }
      }

At least how the the raii::PhysicalDevice constructor is written now it could be converted into fallible constructor with a THROW macro which either expands into detail::throwResultException or return UNEXPECTED(result). Constructors are not allowed to fail unless with an exception, but fallible constructors could be regular static functions for the noexcept case e.g. static auto vk::raii::PhysicalDevice::new(...) -> expected<vk::raii::PhysicalDevice> with the same semanics as their throw constructor counterparts

EDIT: sorry, this was about returning expected<std::vector<raii::PhysicalDevice>>, but I think this would also be possible with an error check macro to forward unexpected if not expected. Is the problem that std::vector functions are not noexcept?

[theHamsta]

https://youtu.be/SJlyMV_oTpY?si=oAVrnMMS9VGgQhc6&t=927 presents an approach to wrap a C library with exceptions or std::expected, nonstd::unexpected depending on user choice using the same implementation for the exception and non-exception path

[asuessenbach]

Is the problem that std::vector functions are not noexcept?

Yes, exactly. noexcept means no std::vector.

[asuessenbach]

... so, it seams there are different opinions available on using std::vector within a noexcept-function.
@jmacnak, @phire, @Ahajha, @sharadhr: any statement on this issue?
Personally, I would not like to mark a creation function that uses std::vector as noexcept.
Would just not self-throwing (and not marking with noexcept) be enough? Or would some non-throwing allocator be needed?

[Ahajha]

In a lot of environments (mine included), memory allocation is just a hard failure/a case we don't consider, so noexcept would be fine. Though I know that's not going to be a universally accepted solution.

What does the standard library do with exceptions disabled do? Does it also hard-error on creation?

One option is to add yet another option which is to either ignore allocation errors, or to propagate them as part of the expected.

[sharadhr]

Another solution, though it means we can't quite use vk::raii::PhysicalDevices:

1. For every enumeration enumerateThings supported by vk::raii::Instance, contain a pThings and a pThingCount variable within the Instance class, initially set to nullptr and 0 respectively.
2. When a given enumerateThings function is first called, set up those variables (with possible error conditions).
3. Return a std::expected<std::span<const Thing>> containing either the span, or the error (since we're using C++23, std::span ought to be supported) using the std::span::span(T*, size_type) constructor.
4. Returning spans this way should be OK, since the pointer and integer will stick around until the containing vk::raii::Instance is deleted.

Although this not declared noexcept, it is defined to 'Throws: Nothing'. De jure we shouldn't call a function that is not declared noexcept from one that is, but de facto, the constructor doesn't throw anything anyway, which we could take advantage of.

There is a paper to subsume these narrow-contract functions under noexcept as well: P1656R1.

As for PhysicalDevices... If we are forced to use std::vector, I think we should catch any exception from std::vector and re-contain it in the E of the std::expected. This still maintains the noexcept.

[asuessenbach]

@Ahajha

What does the standard library do with exceptions disabled do? Does it also hard-error on creation?

I think, it throws. But as nobody will catch it, it will ultimately call abort().

One option is to add yet another option which is to either ignore allocation errors, or to propagate them as part of the expected.

I don't think, stl-exceptions could always be mapped to vk::Result error codes.

@sharadhr
Your approach sounds interesting. But it sounds a bit too fragile, especially with regard to memory ownership.
Maybe, those who can't use std::vector would need to get the Dispatcher out of the vk::raii::Instance and do the enumerating on their own, using some non-throwing memory allocator they need to have anyways.

That is, just not marking vk::raii::Instance::enumeratePhysicalDevices as noexcept might be both the easiest and best thing to do.

[sharadhr]

Have we also considered using a private constructor for the plural types, and using a factory function? This could solve the issue nicely.

[asuessenbach]

Have we also considered using a private constructor for the plural types, and using a factory function? This could solve the issue nicely.

I think, I don't get what you mean... could you provide some pseudo-code to describe?

[sharadhr]

Have we also considered using a private constructor for the plural types, and using a factory function? This could solve the issue nicely.

I think, I don't get what you mean... could you provide some pseudo-code to describe?

So, something like...

struct PhysicalDevices
{
  std::vector<VULKAN_HPP_NAMESPACE::VULKAN_HPP_RAII_NAMESPACE::PhysicalDevice> physicalDevices{};
  std::expected<std::vector<VULKAN_HPP_NAMESPACE::VULKAN_HPP_RAII_NAMESPACE::PhysicalDevice>, VULKAN_HPP_NAMESPACE::Result> Create( VULKAN_HPP_NAMESPACE::VULKAN_HPP_RAII_NAMESPACE::Instance const & instance );

private:
  PhysicalDevices( VULKAN_HPP_NAMESPACE::VULKAN_HPP_RAII_NAMESPACE::Instance const & instance )
  {
    VULKAN_HPP_NAMESPACE::VULKAN_HPP_RAII_NAMESPACE::InstanceDispatcher const * dispatcher = instance.getDispatcher();
    VkPhysicalDevice*                                                           pPhysicalDevices;
    uint32_t                                                                    physicalDeviceCount;
    VULKAN_HPP_NAMESPACE::ArrayProxyNoTemporaries<VkPhysicalDevice>             physicalDevicesSpan;
    VULKAN_HPP_NAMESPACE::Result                                                result;

    do
    {
      result = static_cast<VULKAN_HPP_NAMESPACE::Result>(
        dispatcher->vkEnumeratePhysicalDevices( static_cast<VkInstance>( *instance ), &physicalDeviceCount, pPhysicalDevices ) );
      if ( ( result == VULKAN_HPP_NAMESPACE::Result::eSuccess ) && physicalDeviceCount > 0 )
      {
        physicalDevicesSpan = ArrayProxyNoTemporaries( physicalDeviceCount, pPhysicalDevices );
        break;
      }
    } while ( result == VULKAN_HPP_NAMESPACE::Result::eIncomplete );

    if ( result == VULKAN_HPP_NAMESPACE::Result::eSuccess )
    {
      // do we still need this assert?
      VULKAN_HPP_ASSERT( physicalDeviceCount <= physicalDevicesSpan.size() );
      try
      {
        for ( auto const & physicalDevice : physicalDevicesSpan )
        {
          physicalDevices.emplace_back( instance, physicalDevice );
        }
      }
      catch(const std::exception& e)
      {
        // or something else from vk::Result? eOutOfHostMemory?
        detail::throwResultException( VULKAN_HPP_NAMESPACE::Result::eErrorInitializationFailed, e.what() );
      }
    }
    else
    {
      detail::throwResultException( result, "vkEnumeratePhysicalDevices" );
    }
  }

  public:
    PhysicalDevices( std::nullptr_t ) {}

    // note: don't delete the default constructor!
    PhysicalDevices( PhysicalDevices const & )             = delete;
    PhysicalDevices( PhysicalDevices && rhs )              = default;
    PhysicalDevices & operator=( PhysicalDevices const & ) = delete;
    PhysicalDevices & operator=( PhysicalDevices && rhs )  = default;

    std::expected<std::vector<VULKAN_HPP_NAMESPACE::VULKAN_HPP_RAII_NAMESPACE::PhysicalDevice>, VULKAN_HPP_NAMESPACE::Result> Create( VULKAN_HPP_NAMESPACE::VULKAN_HPP_RAII_NAMESPACE::Instance const & instance )
    {
      try
      {
        this = PhysicalDevices( instance );
        return physicalDevices;
      }
      // probably need a more elegant way to handle
      catch(const VULKAN_HPP_NAMESPACE::SystemError& e)
      {
        return std::unexpected( e );
      }
    }
};

Then, rewrite vk::raii::Instance::enumeratePhysicalDevices as:

VULKAN_HPP_NODISCARD VULKAN_HPP_INLINE std::expected<std::vector<VULKAN_HPP_RAII_NAMESPACE::PhysicalDevice>, vk::Result> Instance::enumeratePhysicalDevices() const noexcept
{
  return VULKAN_HPP_RAII_NAMESPACE::PhysicalDevices( ).Create( *this );
}

[asuessenbach]

@sharadhr, @Ahajha: would you mind to check if #1742 actually resolves this issue?
It's a solution available with C++23 with support for std::expected. No attempt to support it with older versions or other types (yet).

[keryell]

Looking at this interesting discussion as we are considering std::expected too in the SYCL SC standard discussion.
But looking at the code examples above, why so many macros instead of using namespace alias, constexpr functions, templated type aliases, etc.?

[sharadhr]

why so many macros instead of using namespace alias, constexpr functions, templated type aliases, etc.

My understanding is that this is fairly common in library code. Macros are the most straightforward way to provide similar functionality across various C++ versions. They also make generated code easier to deal with, from what I've seen.

Templated types can potentially explode compile times, and Vulkan-Hpp is already huge as it is (I've literally never seen a larger header file, we are approaching 100K lines).

We actually do have constexpr functions, but these are for the users to use.