Fix instantiation of polymorphic types

1eb11dc6146834e48a65061a913ac3cd061ed9e8

9 files changed, 206 insertions, 61 deletions

diff --git a/yt/yt/core/phoenix/context.h b/yt/yt/core/phoenix/context.h
index 4cf8b67bca..a867ffa5fe 100644
--- a/yt/yt/core/phoenix/context.h
+++ b/yt/yt/core/phoenix/context.h
@@ -74,7 +74,7 @@ private:
 
 template <class TSaveContext, class TLoadContext>
 struct ICustomPersistent
-    : public TPolymorphicBase
+    : public virtual TPolymorphicBase
 {
     virtual void Save(TSaveContext& context) const = 0;
     virtual void Load(TLoadContext& context) = 0;
diff --git a/yt/yt/core/phoenix/descriptors-inl.h b/yt/yt/core/phoenix/descriptors-inl.h
new file mode 100644
index 0000000000..0b489d4751
--- /dev/null
+++ b/yt/yt/core/phoenix/descriptors-inl.h
@@ -0,0 +1,38 @@
+#ifndef DESCRIPTORS_INL_H_
+#error "Direct inclusion of this file is not allowed, include descriptors.h"
+// For the sake of sane code completion.
+#include "descriptors.h"
+#endif
+
+#include "polymorphic.h"
+
+#include <yt/yt/core/misc/error.h>
+
+namespace NYT::NPhoenix2 {
+
+////////////////////////////////////////////////////////////////////////////////
+
+template <class T>
+T* TTypeDescriptor::TryConstruct() const
+{
+    if constexpr(NDetail::TPolymorphicTraits<T>::Polymorphic) {
+        return PolymorphicConstructor_ ? dynamic_cast<T*>(PolymorphicConstructor_()) : nullptr;
+    } else {
+        return ConcreteConstructor_ ? static_cast<T*>(ConcreteConstructor_()) : nullptr;
+    }
+}
+
+template <class T>
+T* TTypeDescriptor::ConstructOrThrow() const
+{
+    auto* instance = TryConstruct<T>();
+    if (!instance) {
+        THROW_ERROR_EXCEPTION("Cannot instantiate object of type %v",
+            Name_);
+    }
+    return instance;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
+} // namespace NYT::NPhoenix2
diff --git a/yt/yt/core/phoenix/descriptors.cpp b/yt/yt/core/phoenix/descriptors.cpp
index cbf2fa4613..b961263e7f 100644
--- a/yt/yt/core/phoenix/descriptors.cpp
+++ b/yt/yt/core/phoenix/descriptors.cpp
@@ -102,21 +102,6 @@ std::vector<TTypeTag> TTypeDescriptor::GetBaseTypeTags() const
     return result;
 }
 
-void* TTypeDescriptor::TryConstruct() const
-{
-    return Constructor_ ? Constructor_() : nullptr;
-}
-
-void* TTypeDescriptor::ConstructOrThrow() const
-{
-    auto* instance = TryConstruct();
-    if (!instance) {
-        THROW_ERROR_EXCEPTION("Cannot instantiate object of type %v",
-            Name_);
-    }
-    return instance;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 
 const TUniverseSchemaPtr& TUniverseDescriptor::GetSchema() const
diff --git a/yt/yt/core/phoenix/descriptors.h b/yt/yt/core/phoenix/descriptors.h
index d7ec04d7f2..ea6a803a62 100644
--- a/yt/yt/core/phoenix/descriptors.h
+++ b/yt/yt/core/phoenix/descriptors.h
@@ -61,8 +61,10 @@ public:
 
     std::vector<TTypeTag> GetBaseTypeTags() const;
 
-    void* TryConstruct() const;
-    void* ConstructOrThrow() const;
+    template <class T>
+    T* TryConstruct() const;
+    template <class T>
+    T* ConstructOrThrow() const;
 
 private:
     friend class NDetail::TTypeRegistry;
@@ -74,7 +76,8 @@ private:
     std::vector<std::unique_ptr<TFieldDescriptor>> Fields_;
     std::vector<const TTypeDescriptor*> BaseTypes_;
     bool Template_ = false;
-    TConstructor Constructor_ = nullptr;
+    TPolymorphicConstructor PolymorphicConstructor_ = nullptr;
+    TConcreteConstructor ConcreteConstructor_ = nullptr;
 
     mutable std::once_flag SchemaOnceFlag_;
     mutable TTypeSchemaPtr Schema_;
@@ -112,3 +115,6 @@ private:
 
 } // namespace NYT::NPhoenix2
 
+#define DESCRIPTORS_INL_H_
+#include "descriptors-inl.h"
+#undef DESCRIPTORS_INL_H_
diff --git a/yt/yt/core/phoenix/factory.h b/yt/yt/core/phoenix/factory.h
index a72793a6a0..1a23395c95 100644
--- a/yt/yt/core/phoenix/factory.h
+++ b/yt/yt/core/phoenix/factory.h
@@ -1,6 +1,7 @@
 #pragma once
 
 #include "public.h"
+#include "polymorphic.h"
 
 #include <library/cpp/yt/memory/ref_counted.h>
 #include <library/cpp/yt/memory/new.h>
@@ -14,23 +15,44 @@ namespace NYT::NPhoenix2::NDetail {
 template <class T>
 struct TNonconstructableFactory
 {
-    static constexpr TConstructor Constructor = nullptr;
+    static constexpr TPolymorphicConstructor PolymorphicConstructor = nullptr;
+    static constexpr TConcreteConstructor ConcreteConstructor = nullptr;
+};
+
+template <class T, class TCrtpFactory>
+struct TConstructableFactoryBase
+{
+    static constexpr TPolymorphicConstructor PolymorphicConstructor = [] () -> TPolymorphicBase* {
+        if constexpr(TPolymorphicTraits<T>::Polymorphic) {
+            return TCrtpFactory::Construct();
+        } else {
+            return nullptr;
+        }
+    };
+
+    static constexpr TConcreteConstructor ConcreteConstructor = [] () -> void* {
+        return TCrtpFactory::Construct();
+    };
 };
 
 template <class T>
 struct TSimpleFactory
+    : public TConstructableFactoryBase<T, TSimpleFactory<T>>
 {
-    static constexpr TConstructor Constructor = [] () -> void* {
+    static T* Construct()
+    {
         return new T();
-    };
+    }
 };
 
 template <class T>
 struct TRefCountedFactory
+    : public TConstructableFactoryBase<T, TRefCountedFactory<T>>
 {
-    static constexpr TConstructor Constructor = [] () -> void* {
+    static T* Construct()
+    {
         return NYT::New<T>().Release();
-    };
+    }
 };
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/yt/yt/core/phoenix/public.h b/yt/yt/core/phoenix/public.h
index 34935532df..c8ea7ce526 100644
--- a/yt/yt/core/phoenix/public.h
+++ b/yt/yt/core/phoenix/public.h
@@ -1,7 +1,6 @@
 #pragma once
 
 #include <library/cpp/yt/memory/ref_counted.h>
-#include <library/cpp/yt/memory/intrusive_ptr.h>
 
 #include <library/cpp/yt/misc/strong_typedef.h>
 
@@ -9,14 +8,17 @@ namespace NYT::NPhoenix2 {
 
 ////////////////////////////////////////////////////////////////////////////////
 
-using TConstructor = void* (*)();
-
-////////////////////////////////////////////////////////////////////////////////
-
 class TFieldDescriptor;
 class TTypeDescriptor;
 class TUniverseDescriptor;
 
+struct TPolymorphicBase;
+
+////////////////////////////////////////////////////////////////////////////////
+
+using TPolymorphicConstructor = TPolymorphicBase* (*)();
+using TConcreteConstructor = void* (*)();
+
 ////////////////////////////////////////////////////////////////////////////////
 
 DECLARE_REFCOUNTED_STRUCT(TFieldSchema);
diff --git a/yt/yt/core/phoenix/type_def-inl.h b/yt/yt/core/phoenix/type_def-inl.h
index 5dae193ba8..914cf669ae 100644
--- a/yt/yt/core/phoenix/type_def-inl.h
+++ b/yt/yt/core/phoenix/type_def-inl.h
@@ -141,7 +141,8 @@ public:
         std::vector<const std::type_info*> typeInfos,
         TTypeTag tag,
         bool isTemplate,
-        TConstructor constructor);
+        TPolymorphicConstructor polymorphicConstructor,
+        TConcreteConstructor concreteConstructor);
 
     template <TFieldTag::TUnderlying TagValue, auto Member>
     auto Field(TString name)
@@ -198,7 +199,8 @@ auto MakeTypeSchemaBuilderRegistrar()
         GetTypeInfos<TThis>(),
         TThis::TypeTag,
         Template,
-        TFactoryTraits<TThis>::TFactory::Constructor);
+        TFactoryTraits<TThis>::TFactory::PolymorphicConstructor,
+        TFactoryTraits<TThis>::TFactory::ConcreteConstructor);
 }
 
 template <class TThis, bool Template>
@@ -883,15 +885,15 @@ struct TSerializer
                     YT_VERIFY(streamTag == runtimeTag);
                 }
             } else {
-                if constexpr(TPolymorphicTraits<T>::Polymorphic) {
-                    auto tag = LoadSuspended<TTypeTag>(context);
-                    const auto& descriptor = ITypeRegistry::Get()->GetUniverseDescriptor().GetTypeDescriptorByTagOrThrow(tag);
-                    rawPtr = static_cast<T*>(descriptor.ConstructOrThrow());
-                } else {
-                    using TFactory = typename TFactoryTraits<T>::TFactory;
-                    static_assert(TFactory::Constructor);
-                    rawPtr = static_cast<T*>(TFactory::Constructor());
-                }
+                const auto& descriptor = [&] () -> const TTypeDescriptor& {
+                    if constexpr(TPolymorphicTraits<T>::Polymorphic) {
+                        auto tag = LoadSuspended<TTypeTag>(context);
+                        return ITypeRegistry::Get()->GetUniverseDescriptor().GetTypeDescriptorByTagOrThrow(tag);
+                    } else {
+                        return T::GetTypeDescriptor();
+                    }
+                }();
+                rawPtr = descriptor.template ConstructOrThrow<T>();
                 context.RegisterConstructedObject(rawPtr);
             }
 
diff --git a/yt/yt/core/phoenix/type_def.cpp b/yt/yt/core/phoenix/type_def.cpp
index e1f1834940..a9ce100317 100644
--- a/yt/yt/core/phoenix/type_def.cpp
+++ b/yt/yt/core/phoenix/type_def.cpp
@@ -8,13 +8,15 @@ TTypeSchemaBuilderRegistar::TTypeSchemaBuilderRegistar(
     std::vector<const std::type_info*> typeInfos,
     TTypeTag tag,
     bool isTemplate,
-    TConstructor constructor)
+    TPolymorphicConstructor polymorphicConstructor,
+    TConcreteConstructor concreteConstructor)
 {
     TypeDescriptor_->Name_ = CppDemangle(typeInfos[0]->name());
     TypeDescriptor_->TypeInfos_ = std::move(typeInfos);
     TypeDescriptor_->Tag_ = tag;
     TypeDescriptor_->Template_ = isTemplate;
-    TypeDescriptor_->Constructor_ = constructor;
+    TypeDescriptor_->PolymorphicConstructor_ = polymorphicConstructor;
+    TypeDescriptor_->ConcreteConstructor_ = concreteConstructor;
 }
 
 const TTypeDescriptor& TTypeSchemaBuilderRegistar::Finish() &&
diff --git a/yt/yt/core/phoenix/unittests/phoenix_ut.cpp b/yt/yt/core/phoenix/unittests/phoenix_ut.cpp
index 8099b7a42a..4159f9042f 100644
--- a/yt/yt/core/phoenix/unittests/phoenix_ut.cpp
+++ b/yt/yt/core/phoenix/unittests/phoenix_ut.cpp
@@ -774,14 +774,14 @@ TEST(TPhoenixTest, TypeDescriptorByTypeInfo)
 TEST(TPhoenixTest, InstantiateSimple)
 {
     const auto& descriptor = ITypeRegistry::Get()->GetUniverseDescriptor().GetTypeDescriptorByTagOrThrow(TPoint::TypeTag);
-    auto* p = static_cast<TPoint*>(descriptor.ConstructOrThrow());
+    auto* p = descriptor.ConstructOrThrow<TPoint>();
     delete p;
 }
 
 TEST(TPhoenixTest, InstantiateRefCounted)
 {
     const auto& descriptor = ITypeRegistry::Get()->GetUniverseDescriptor().GetTypeDescriptorByTagOrThrow(TRefCountedStruct::TypeTag);
-    auto* s = static_cast<TRefCountedStruct*>(descriptor.ConstructOrThrow());
+    auto* s = descriptor.ConstructOrThrow<TRefCountedStruct>();
     EXPECT_EQ(s->GetRefCount(), 1);
     s->Unref();
 }
@@ -789,15 +789,61 @@ TEST(TPhoenixTest, InstantiateRefCounted)
 TEST(TPhoenixTest, InstantiateNonconstructable)
 {
     const auto& descriptor = ITypeRegistry::Get()->GetUniverseDescriptor().GetTypeDescriptorByTagOrThrow(TAbstractStruct::TypeTag);
-    EXPECT_EQ(descriptor.TryConstruct(), nullptr);
+    EXPECT_EQ(descriptor.TryConstruct<TAbstractStruct>(), nullptr);
     EXPECT_THROW_MESSAGE_HAS_SUBSTR(
-        descriptor.ConstructOrThrow(),
+        descriptor.ConstructOrThrow<TAbstractStruct>(),
         std::exception,
         "Cannot instantiate");
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 
+namespace NInstantiatePolymorphic {
+
+struct TBase
+    : public TPolymorphicBase
+{
+    int A;
+
+    PHOENIX_DECLARE_TYPE(TBase, 0xbfad62ab);
+};
+
+void TBase::RegisterMetadata(auto&& registrar)
+{
+    registrar.template Field<1, &TThis::A>("a");
+}
+
+PHOENIX_DEFINE_TYPE(TBase);
+
+struct TDerived
+    : public virtual TBase
+{
+    PHOENIX_DECLARE_TYPE(TDerived, 0x623bdf71);
+};
+
+void TDerived::RegisterMetadata(auto&& registrar)
+{
+    registrar.template BaseType<TBase>();
+}
+
+PHOENIX_DEFINE_TYPE(TDerived);
+
+} // namespace NInstantiatePolymorphic
+
+TEST(TPhoenixTest, InstantiatePolymorphic)
+{
+    using namespace NInstantiatePolymorphic;
+
+    const auto& descriptor = ITypeRegistry::Get()->GetUniverseDescriptor().GetTypeDescriptorByTagOrThrow(TDerived::TypeTag);
+    auto* b = descriptor.ConstructOrThrow<TBase>();
+    auto* d = dynamic_cast<TDerived*>(b);
+    d->A = 123;
+    EXPECT_EQ(d->A, 123);
+    delete b;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
 namespace NIntrusivePtr {
 
 struct A;
@@ -1241,34 +1287,52 @@ TEST(TPhoenixTest, PolymorphicRawPtr)
 
 namespace NPolymorphicIntrusivePtr {
 
-struct TBase
-    : public TRefCounted
-    , public IPersistent
+struct TBase1
+    : public virtual TRefCounted
+    , public virtual IPersistent
 {
-    int X = 0;
+    int X1 = 0;
 
-    PHOENIX_DECLARE_POLYMORPHIC_TYPE(TBase, 0x149f8345);
+    PHOENIX_DECLARE_POLYMORPHIC_TYPE(TBase1, 0x149f8345);
 };
 
-void TBase::RegisterMetadata(auto&& registrar)
+void TBase1::RegisterMetadata(auto&& registrar)
 {
-    registrar.template Field<1, &TThis::X>("x");
+    registrar.template Field<1, &TThis::X1>("x1");
 }
 
-PHOENIX_DEFINE_TYPE(TBase);
+PHOENIX_DEFINE_TYPE(TBase1);
+
+struct TBase2
+    : public virtual TRefCounted
+    , public virtual IPersistent
+{
+    int X2 = 0;
+
+    PHOENIX_DECLARE_POLYMORPHIC_TYPE(TBase2, 0x185ec0d);
+};
+
+void TBase2::RegisterMetadata(auto&& registrar)
+{
+    registrar.template Field<1, &TThis::X2>("x2");
+}
+
+PHOENIX_DEFINE_TYPE(TBase2);
 
 struct TDerived
-    : public TBase
+    : public TBase1
+    , public TBase2
 {
-    int Y = 0;
-    TIntrusivePtr<TBase> Z;
+    int Y;
+    TIntrusivePtr<TBase2> Z;
 
     PHOENIX_DECLARE_POLYMORPHIC_TYPE(TDerived, 0x57818795);
 };
 
 void TDerived::RegisterMetadata(auto&& registrar)
 {
-    registrar.template BaseType<TBase>();
+    registrar.template BaseType<TBase1>();
+    registrar.template BaseType<TBase2>();
     registrar.template Field<1, &TThis::Y>("y");
     registrar.template Field<2, &TThis::Z>("z");
 }
@@ -1282,13 +1346,13 @@ TEST(TPhoenixTest, PolymorphicIntrusivePtr)
     using namespace NPolymorphicIntrusivePtr;
 
     auto obj1 = New<TDerived>();
-    obj1->X = 123;
+    obj1->X1= 123;
     obj1->Y = 456;
     obj1->Z = obj1.Get();
 
     auto obj2 = New<TDerived>();
-    InplaceDeserialize(obj2, Serialize(TIntrusivePtr<TBase>(obj1)));
-    EXPECT_EQ(obj2->X, 123);
+    InplaceDeserialize(obj2, Serialize(TIntrusivePtr<TBase1>(obj1)));
+    EXPECT_EQ(obj2->X1, 123);
     EXPECT_EQ(obj2->Y, 456);
     EXPECT_EQ(obj2->Z, obj2);
 
@@ -1297,6 +1361,30 @@ TEST(TPhoenixTest, PolymorphicIntrusivePtr)
     obj2->Z.Reset();
 }
 
+TEST(TPhoenixTest, PolymorphicMultipleInheritance)
+{
+    using namespace NPolymorphicIntrusivePtr;
+
+    auto obj1 = New<TDerived>();
+    obj1->X1 = 11;
+    obj1->X2 = 12;
+    obj1->Y = 13;
+
+    auto obj2 = New<TDerived>();
+    obj2->X1 = 21;
+    obj2->X2 = 22;
+    obj2->Y = 23;
+    obj2->Z = obj1;
+
+    auto obj3 = New<TDerived>();
+    auto x = Serialize(TIntrusivePtr<TBase1>(obj2));
+    InplaceDeserialize(obj3, x);
+    EXPECT_EQ(obj3->X1, 21);
+    EXPECT_EQ(obj3->X2, 22);
+    EXPECT_EQ(obj3->Y, 23);
+    EXPECT_EQ(obj3->Z->X2, 12);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 
 namespace NOpaque {