From c6fe38989c1a8c74b44ee38c9d795a3ffda13d15 Mon Sep 17 00:00:00 2001
From: lisyarus <lisyarus@gmail.com>
Date: Mon, 23 Nov 2020 17:56:35 +0300
Subject: [PATCH] util::function bugfixes & strong exception guarantee

---
 libs/util/include/psemek/util/function.hpp | 77 +++++++++++++++++-----
 1 file changed, 60 insertions(+), 17 deletions(-)

diff --git a/libs/util/include/psemek/util/function.hpp b/libs/util/include/psemek/util/function.hpp
index 666f0539..14d9b523 100644
--- a/libs/util/include/psemek/util/function.hpp
+++ b/libs/util/include/psemek/util/function.hpp
@@ -20,6 +20,22 @@ namespace psemek::util
 	template <typename R, typename ... Args>
 	struct function<R(Args...)>
 	{
+	private:
+		static constexpr std::size_t storage_size = sizeof(void*) * 3;
+		static constexpr std::size_t storage_align = alignof(void*);
+
+		template <typename T>
+		struct uses_static_storage
+			: std::bool_constant<true
+				&& sizeof(T) <= storage_size
+				&& alignof(T) <= storage_align
+				&& ((storage_align % alignof(T)) == 0)
+				&& std::is_nothrow_move_constructible_v<T>
+				>
+		{};
+
+	public:
+
 		using signature = R(Args...);
 
 		function() noexcept = default;
@@ -33,8 +49,18 @@ namespace psemek::util
 		function (function const &) = delete;
 		function & operator = (function const &) = delete;
 
+		~function();
+
+		// operator = (F && f) has strong exception guarantree:
+		// if the assignment throws, the function remains unchanged
+
 		template <typename F>
-		function & operator = (F && f);
+		std::enable_if_t<uses_static_storage<std::decay_t<F>>::value, function &>
+			operator = (F && f);
+
+		template <typename F>
+		std::enable_if_t<!uses_static_storage<std::decay_t<F>>::value, function &>
+			operator = (F && f);
 
 		explicit operator bool() const
 		{
@@ -46,10 +72,9 @@ namespace psemek::util
 
 		void reset();
 
-	private:
-		static constexpr std::size_t storage_size = sizeof(void*) * 3;
-		static constexpr std::size_t storage_align = alignof(void*);
+		void swap(function & other) noexcept;
 
+	private:
 		std::aligned_storage_t<storage_size, storage_align> storage_;
 
 		struct vtable
@@ -78,18 +103,29 @@ namespace psemek::util
 
 	template <typename R, typename ... Args>
 	template <typename F>
-	function<R(Args...)> & function<R(Args...)>::operator = (F && f)
+	std::enable_if_t<function<R(Args...)>::template uses_static_storage<std::decay_t<F>>::value, function<R(Args...)> &>
+		function<R(Args...)>::operator = (F && f)
 	{
 		reset();
 		assign(std::forward<F>(f));
 		return *this;
 	}
 
+	template <typename R, typename ... Args>
+	template <typename F>
+	std::enable_if_t<!function<R(Args...)>::template uses_static_storage<std::decay_t<F>>::value, function<R(Args...)> &>
+		function<R(Args...)>::operator = (F && f)
+	{
+		function(std::forward<F>(f)).swap(*this);
+		return *this;
+	}
+
 	template <typename R, typename ... Args>
 	function<R(Args...)>::function (function && other) noexcept
 	{
 		vtable_ = other.vtable_;
-		vtable_->move(&other.storage_, &storage_);
+		if (vtable_)
+			vtable_->move(&other.storage_, &storage_);
 		other.reset();
 	}
 
@@ -99,12 +135,20 @@ namespace psemek::util
 		if (this == &other)
 			return *this;
 
+		reset();
 		vtable_ = other.vtable_;
-		vtable_->move(&other.storage_, &storage_);
+		if (vtable_)
+			vtable_->move(&other.storage_, &storage_);
 		other.reset();
 		return *this;
 	}
 
+	template <typename R, typename ... Args>
+	function<R(Args...)>::~function()
+	{
+		reset();
+	}
+
 	template <typename R, typename ... Args>
 	template <typename ... Args1>
 	R function<R(Args...)>::operator()(Args1 && ... args) const
@@ -124,27 +168,26 @@ namespace psemek::util
 		vtable_ = nullptr;
 	}
 
+	template <typename R, typename ... Args>
+	void function<R(Args...)>::swap(function & other) noexcept
+	{
+		std::swap(*this, other);
+	}
+
 	template <typename R, typename ... Args>
 	template <typename F>
 	void function<R(Args...)>::assign(F && f)
 	{
 		using T = std::decay_t<F>;
 
-		constexpr bool static_storage = true
-			&& sizeof(T) <= storage_size
-			&& alignof(T) <= storage_align
-			&& ((storage_align % alignof(T)) == 0)
-			&& std::is_nothrow_move_constructible_v<T>
-			;
-
-		if constexpr (static_storage)
+		if constexpr (uses_static_storage<T>::value)
 		{
 			new (reinterpret_cast<T *>(&storage_)) T(std::move(f));
 
 			static vtable m = {
 				[](void * src, void * dst){ new (reinterpret_cast<T*>(dst)) T(std::move(*reinterpret_cast<T*>(src))); },
 				[](void * src){ reinterpret_cast<T*>(src)->~T(); },
-				[](void * src, Args ... args) -> R { return (*reinterpret_cast<T*>(src))(static_cast<Args&&>(args)...); }
+				[](void * src, Args ... args) -> R { return std::invoke(*reinterpret_cast<T*>(src), static_cast<Args&&>(args)...); }
 			};
 
 			vtable_ = &m;
@@ -156,7 +199,7 @@ namespace psemek::util
 			static vtable m = {
 				[](void * src, void * dst){ *reinterpret_cast<T**>(dst) = *reinterpret_cast<T**>(src); *reinterpret_cast<T**>(src) = nullptr; },
 				[](void * src){ delete *reinterpret_cast<T**>(src); },
-				[](void * src, Args ... args) -> R { return (**reinterpret_cast<T**>(src))(static_cast<Args&&>(args)...); }
+				[](void * src, Args ... args) -> R { return std::invoke(**reinterpret_cast<T**>(src), static_cast<Args&&>(args)...); }
 			};
 
 			vtable_ = &m;