Changed TFutureCombinerResultHolder to have non-assignable types support in Futures. Also added unit tests to make sure they work

4 files changed, 140 insertions, 8 deletions

diff --git a/yt/yt/core/actions/future-inl.h b/yt/yt/core/actions/future-inl.h
index ed590f85d69..a16b80c0a9c 100644
--- a/yt/yt/core/actions/future-inl.h
+++ b/yt/yt/core/actions/future-inl.h
@@ -1722,9 +1722,79 @@ TFuture<T>* TFutureHolder<T>::operator->() // noexcept
 namespace NDetail {
 
 template <class T>
+concept CLightweight = std::default_initializable<T> &&
+                       std::is_trivially_destructible_v<T> &&
+                       std::is_move_assignable_v<T>;
+
+////////////////////////////////////////////////////////////////////////////////
+
+template <class T>
+class TFutureCombinerResultHolderStorage
+{
+public:
+    constexpr explicit TFutureCombinerResultHolderStorage(size_t size)
+        : Impl_(size)
+    { }
+
+    template <class... Args>
+        requires std::constructible_from<T, Args...>
+    constexpr void ConstructAt(size_t index, Args&&... args)
+    {
+        Impl_[index].emplace(std::forward<Args>(args)...);
+    }
+
+    constexpr std::vector<T> VectorFromThis() &&
+    {
+        std::vector<T> result;
+
+        result.reserve(Impl_.size());
+
+        for (auto& opt : Impl_) {
+            YT_VERIFY(opt.has_value());
+
+            result.push_back(std::move(*opt));
+        }
+
+        return result;
+    }
+
+private:
+    std::vector<std::optional<T>> Impl_;
+};
+
+////////////////////////////////////////////////////////////////////////////////
+
+template <NDetail::CLightweight T>
+class TFutureCombinerResultHolderStorage<T>
+{
+public:
+    constexpr explicit TFutureCombinerResultHolderStorage(size_t size)
+        : Impl_(size)
+    { }
+
+    template <class... Args>
+        requires std::constructible_from<T, Args...>
+    constexpr void ConstructAt(size_t index, Args&&... args)
+    {
+        Impl_[index] = T(std::forward<Args>(args)...);
+    }
+
+    //! This method is inherently unsafe because it assumes
+    //! That you have filled every slot
+    constexpr std::vector<T> VectorFromThis() &&
+    {
+        return std::move(Impl_);
+    }
+
+private:
+    std::vector<T> Impl_;
+};
+
+template <class T>
 class TFutureCombinerResultHolder
 {
 public:
+    using TStorage = TFutureCombinerResultHolderStorage<T>;
     using TResult = std::vector<T>;
 
     explicit TFutureCombinerResultHolder(int size)
@@ -1734,7 +1804,7 @@ public:
     bool TrySetResult(int index, const NYT::TErrorOr<T>& errorOrValue)
     {
         if (errorOrValue.IsOK()) {
-            Result_[index] = errorOrValue.Value();
+            Result_.ConstructAt(index, errorOrValue.Value());
             return true;
         } else {
             return false;
@@ -1743,17 +1813,18 @@ public:
 
     bool TrySetPromise(const TPromise<TResult>& promise)
     {
-        return promise.TrySet(std::move(Result_));
+        return promise.TrySet(std::move(Result_).VectorFromThis());
     }
 
 private:
-    TResult Result_;
+    TStorage Result_;
 };
 
 template <class T>
 class TFutureCombinerResultHolder<TErrorOr<T>>
 {
 public:
+    using TStorage = TFutureCombinerResultHolderStorage<TErrorOr<T>>;
     using TResult = std::vector<TErrorOr<T>>;
 
     explicit TFutureCombinerResultHolder(int size)
@@ -1762,17 +1833,17 @@ public:
 
     bool TrySetResult(int index, const NYT::TErrorOr<T>& errorOrValue)
     {
-        Result_[index] = errorOrValue;
+        Result_.ConstructAt(index, errorOrValue);
         return true;
     }
 
     bool TrySetPromise(const TPromise<TResult>& promise)
     {
-        return promise.TrySet(std::move(Result_));
+        return promise.TrySet(std::move(Result_).VectorFromThis());
     }
 
 private:
-    TResult Result_;
+    TStorage Result_;
 };
 
 template <>
diff --git a/yt/yt/core/actions/unittests/future_ut.cpp b/yt/yt/core/actions/unittests/future_ut.cpp
index ee676f70100..0408fe811f2 100644
--- a/yt/yt/core/actions/unittests/future_ut.cpp
+++ b/yt/yt/core/actions/unittests/future_ut.cpp
@@ -24,6 +24,11 @@ class TFutureTest
     : public ::testing::Test
 { };
 
+struct NonAssignable
+{
+    const int Value = 0;
+};
+
 TEST_F(TFutureTest, NoncopyableGet)
 {
     auto f = MakeFuture<std::unique_ptr<int>>(std::make_unique<int>(1));
@@ -94,6 +99,60 @@ TEST_F(TFutureTest, NoncopyableApply5)
     EXPECT_EQ(2, g.Get().Value());
 }
 
+TEST_F(TFutureTest, NonAssignable1)
+{
+    auto f = MakeFuture<NonAssignable>({
+        .Value = 1
+    });
+
+    auto g = f.ApplyUnique(BIND([] (NonAssignable&& object) {
+        EXPECT_EQ(1, object.Value);
+    }));
+
+    EXPECT_TRUE(g.IsSet());
+    EXPECT_TRUE(g.Get().IsOK());
+}
+
+TEST_F(TFutureTest, NonAssignable2)
+{
+    auto f = MakeFuture<NonAssignable>({
+        .Value = 1
+    });
+
+    std::vector<decltype(f)> futures;
+
+    futures.push_back(f);
+    futures.push_back(f);
+
+    auto g = AllSet(futures).ApplyUnique(BIND([] (std::vector<TErrorOr<NonAssignable>>&& objects) {
+        EXPECT_TRUE(objects.at(0).IsOK());
+        EXPECT_TRUE(objects.at(1).IsOK());
+        EXPECT_EQ(1, objects[0].Value().Value);
+    }));
+
+    EXPECT_TRUE(g.IsSet());
+    EXPECT_TRUE(g.Get().IsOK());
+}
+
+TEST_F(TFutureTest, NonAssignable3)
+{
+    auto f = MakeFuture<NonAssignable>({
+        .Value = 1
+    });
+
+    std::vector<decltype(f)> futures;
+
+    futures.push_back(f);
+    futures.push_back(f);
+
+    auto g = AllSucceeded(futures).ApplyUnique(BIND([] (std::vector<NonAssignable>&& objects) {
+        EXPECT_EQ(1, objects[0].Value);
+    }));
+
+    EXPECT_TRUE(g.IsSet());
+    EXPECT_TRUE(g.Get().IsOK());
+}
+
 TEST_F(TFutureTest, Unsubscribe)
 {
     auto p = NewPromise<int>();
diff --git a/yt/yt/core/misc/error-inl.h b/yt/yt/core/misc/error-inl.h
index 0d07715d68f..0020b9e0d61 100644
--- a/yt/yt/core/misc/error-inl.h
+++ b/yt/yt/core/misc/error-inl.h
@@ -152,6 +152,7 @@ TErrorOr<T>::TErrorOr(const std::exception& ex)
 
 template <class T>
 TErrorOr<T>& TErrorOr<T>::operator = (const TErrorOr<T>& other)
+    requires std::is_copy_assignable_v<T>
 {
     static_cast<TError&>(*this) = static_cast<const TError&>(other);
     Value_ = other.Value_;
@@ -160,6 +161,7 @@ TErrorOr<T>& TErrorOr<T>::operator = (const TErrorOr<T>& other)
 
 template <class T>
 TErrorOr<T>& TErrorOr<T>::operator = (TErrorOr<T>&& other) noexcept
+    requires std::is_nothrow_move_assignable_v<T>
 {
     static_cast<TError&>(*this) = std::move(other);
     Value_ = std::move(other.Value_);
diff --git a/yt/yt/core/misc/error.h b/yt/yt/core/misc/error.h
index a7d4b60d014..033cb3da16f 100644
--- a/yt/yt/core/misc/error.h
+++ b/yt/yt/core/misc/error.h
@@ -338,8 +338,8 @@ public:
     template <class U>
     TErrorOr(TErrorOr<U>&& other) noexcept;
 
-    TErrorOr<T>& operator = (const TErrorOr<T>& other);
-    TErrorOr<T>& operator = (TErrorOr<T>&& other) noexcept;
+    TErrorOr<T>& operator = (const TErrorOr<T>& other) requires std::is_copy_assignable_v<T>;
+    TErrorOr<T>& operator = (TErrorOr<T>&& other) noexcept requires std::is_nothrow_move_assignable_v<T>;
 
     const T& Value() const &;
     T& Value() &;