Merge pull request #17191 from anton-potapov:gapi_variant_converting_ctor_assign_op_reworked

modules/gapi/include/opencv2/gapi/gopaque.hpp

@@ -17,6 +17,7 @@
 
 #include <opencv2/gapi/util/variant.hpp>
 #include <opencv2/gapi/util/throw.hpp>
+#include <opencv2/gapi/util/type_traits.hpp>
 #include <opencv2/gapi/own/assert.hpp>
 
 namespace cv
@@ -86,13 +87,13 @@ namespace detail
     template <typename T>
     bool GOpaqueU::holds() const{
         GAPI_Assert(m_hint != nullptr);
-        using U = typename std::decay<T>::type;
+        using U = util::decay_t<T>;
         return dynamic_cast<TypeHint<U>*>(m_hint.get()) != nullptr;
     };
 
     template <typename T>
     void GOpaqueU::specifyType(){
-        m_hint.reset(new TypeHint<typename std::decay<T>::type>);
+        m_hint.reset(new TypeHint<util::decay_t<T>>);
     };
 
     // This class represents a typed object reference.
@@ -220,8 +221,13 @@ namespace detail
     public:
         OpaqueRef() = default;
 
-        template<typename T> explicit OpaqueRef(T&& obj) :
-            m_ref(new OpaqueRefT<typename std::decay<T>::type>(std::forward<T>(obj))) {}
+
+        template<
+            typename T,
+            typename = util::are_different_t<OpaqueRef, T>
+        >
+        explicit OpaqueRef(T&& obj) :
+            m_ref(new OpaqueRefT<util::decay_t<T>>(std::forward<T>(obj))) {}
 
         template<typename T> void reset()
         {
@@ -274,7 +280,7 @@ public:
 private:
     // Host type (or Flat type) - the type this GOpaque is actually
     // specified to.
-    using HT = typename detail::flatten_g<typename std::decay<T>::type>::type;
+    using HT = typename detail::flatten_g<util::decay_t<T>>::type;
 
     static void CTor(detail::OpaqueRef& ref) {
         ref.reset<HT>();

modules/gapi/include/opencv2/gapi/util/type_traits.hpp

@@ -0,0 +1,31 @@
+// This file is part of OpenCV project.
+// It is subject to the license terms in the LICENSE file found in the top-level directory
+// of this distribution and at http://opencv.org/license.html.
+//
+// Copyright (C) 2020 Intel Corporation
+
+
+#ifndef OPENCV_GAPI_UTIL_TYPE_TRAITS_HPP
+#define OPENCV_GAPI_UTIL_TYPE_TRAITS_HPP
+
+#include <type_traits>
+
+namespace cv
+{
+namespace util
+{
+    //these are C++14 parts of type_traits :
+    template< bool B, class T = void >
+    using enable_if_t = typename std::enable_if<B,T>::type;
+
+    template<typename T>
+    using decay_t = typename std::decay<T>::type;
+
+    //this is not part of C++14 but still, of pretty common usage
+    template<class T, class U, class V = void>
+    using are_different_t = enable_if_t< !std::is_same<decay_t<T>, decay_t<U>>::value, V>;
+
+} // namespace cv
+} // namespace util
+
+#endif // OPENCV_GAPI_UTIL_TYPE_TRAITS_HPP

modules/gapi/include/opencv2/gapi/util/variant.hpp

@@ -13,6 +13,7 @@
 
 #include <opencv2/gapi/util/throw.hpp>
 #include <opencv2/gapi/util/util.hpp> // max_of_t
+#include <opencv2/gapi/util/type_traits.hpp>
 
 // A poor man's `variant` implementation, incompletely modeled against C++17 spec.
 namespace cv
@@ -35,15 +36,6 @@ namespace util
             static_assert(std::is_same<Target, First>::value, "Type not found");
             static const constexpr std::size_t value = I;
         };
-
-        template< bool B, class T = void >
-        using enable_if_t = typename std::enable_if<B,T>::type;
-
-        template<class T, class U, class V = void>
-        using are_different_t = enable_if_t<
-                !std::is_same<typename std::decay<T>::type,
-                              typename std::decay<U>::type>::value,
-                 V>;
     }
 
     template<typename Target, typename... Types>
@@ -82,18 +74,20 @@ namespace util
             }
         };
 
-        template<typename T> struct vctr_h {
-            static void help(Memory memory, const void* pval) {
-                new (memory) T(*reinterpret_cast<const T*>(pval));
-            }
-        };
-
         template<typename T> struct mctr_h {
             static void help(Memory memory, void *pval) {
                 new (memory) T(std::move(*reinterpret_cast<T*>(pval)));
             }
         };
 
+        //FIXME: unify with cctr_h and mctr_h
+        template<typename T> struct cnvrt_ctor_h {
+            static void help(Memory memory, void* from) {
+                using util::decay_t;
+                new (memory) decay_t<T>(std::forward<T>(*reinterpret_cast<decay_t<T>*>(from)));
+            }
+        };
+
         template<typename T> struct copy_h {
             static void help(Memory to, const Memory from) {
                 *reinterpret_cast<T*>(to) = *reinterpret_cast<const T*>(from);
@@ -101,8 +95,16 @@ namespace util
         };
 
         template<typename T> struct move_h {
-            static void help(Memory to, const Memory from) {
-                *reinterpret_cast<T*>(to) = std::move(*reinterpret_cast<const T*>(from));
+            static void help(Memory to, Memory from) {
+                *reinterpret_cast<T*>(to) = std::move(*reinterpret_cast<T*>(from));
+            }
+        };
+
+        //FIXME: unify with copy_h and move_h
+        template<typename T> struct cnvrt_assign_h {
+            static void help(Memory to, void* from) {
+                using util::decay_t;
+                *reinterpret_cast<decay_t<T>*>(to) = std::forward<T>(*reinterpret_cast<decay_t<T>*>(from));
             }
         };
 
@@ -128,23 +130,42 @@ namespace util
         };
 
         typedef void (*CCtr) (Memory, const Memory);  // Copy c-tor (variant)
-        typedef void (*VCtr) (Memory, const void*);   // Copy c-tor (value)
         typedef void (*MCtr) (Memory, void*);         // Generic move c-tor
         typedef void (*Copy) (Memory, const Memory);  // Copy assignment
-        typedef void (*Move) (Memory, const Memory);  // Move assignment
+        typedef void (*Move) (Memory, Memory);        // Move assignment
+
         typedef void (*Swap) (Memory, Memory);        // Swap
         typedef void (*Dtor) (Memory);                // Destructor
 
+        using  cnvrt_assgn_t   = void (*) (Memory, void*);  // Converting assignment (via std::forward)
+        using  cnvrt_ctor_t    = void (*) (Memory, void*);  // Converting constructor (via std::forward)
+
         typedef bool (*Equal)(const Memory, const Memory); // Equality test (external)
 
         static constexpr std::array<CCtr, sizeof...(Ts)> cctrs(){ return {{(&cctr_h<Ts>::help)...}};}
-        static constexpr std::array<VCtr, sizeof...(Ts)> vctrs(){ return {{(&vctr_h<Ts>::help)...}};}
         static constexpr std::array<MCtr, sizeof...(Ts)> mctrs(){ return {{(&mctr_h<Ts>::help)...}};}
         static constexpr std::array<Copy, sizeof...(Ts)> cpyrs(){ return {{(&copy_h<Ts>::help)...}};}
         static constexpr std::array<Move, sizeof...(Ts)> mvers(){ return {{(&move_h<Ts>::help)...}};}
         static constexpr std::array<Swap, sizeof...(Ts)> swprs(){ return {{(&swap_h<Ts>::help)...}};}
         static constexpr std::array<Dtor, sizeof...(Ts)> dtors(){ return {{(&dtor_h<Ts>::help)...}};}
 
+        template<bool cond, typename T>
+        struct conditional_ref : std::conditional<cond, typename std::remove_reference<T>::type&, typename std::remove_reference<T>::type > {};
+
+        template<bool cond, typename T>
+        using conditional_ref_t = typename conditional_ref<cond, T>::type;
+
+
+        template<bool is_lvalue_arg>
+        static constexpr std::array<cnvrt_assgn_t, sizeof...(Ts)> cnvrt_assgnrs(){
+            return {{(&cnvrt_assign_h<conditional_ref_t<is_lvalue_arg,Ts>>::help)...}};
+        }
+
+        template<bool is_lvalue_arg>
+        static constexpr std::array<cnvrt_ctor_t, sizeof...(Ts)> cnvrt_ctors(){
+            return {{(&cnvrt_ctor_h<conditional_ref_t<is_lvalue_arg,Ts>>::help)...}};
+        }
+
         std::size_t m_index = 0;
 
     protected:
@@ -162,16 +183,12 @@ namespace util
         variant() noexcept;
         variant(const variant& other);
         variant(variant&& other) noexcept;
-        template<typename T> explicit variant(const T& t);
         // are_different_t is a SFINAE trick to avoid variant(T &&t) with T=variant
         // for some reason, this version is called instead of variant(variant&& o) when
         // variant is used in STL containers (examples: vector assignment).
-        // detail::enable_if_t<! std::is_lvalue_reference<T>::value> is a SFINAE
-        // trick to limit this constructor only to rvalue reference argument
         template<
             typename T,
-            typename = detail::are_different_t<variant, T>,
-            typename = detail::enable_if_t<! std::is_lvalue_reference<T>::value>
+            typename = util::are_different_t<variant, T>
         >
         explicit variant(T&& t);
         // template<class T, class... Args> explicit variant(Args&&... args);
@@ -187,7 +204,7 @@ namespace util
         // SFINAE trick to avoid operator=(T&&) with T=variant<>, see comment above
         template<
             typename T,
-            typename = detail::are_different_t<variant, T>
+            typename = util::are_different_t<variant, T>
         >
         variant& operator=(T&& t) noexcept;
 
@@ -244,19 +261,12 @@ namespace util
     }
 
     template<typename... Ts>
-    template<class T>
-    variant<Ts...>::variant(const T& t)
-        : m_index(util::type_list_index<T, Ts...>::value)
-    {
-        (vctrs()[m_index])(memory, &t);
-    }
-
-    template<typename... Ts>
-    template<class T, typename , typename>
+    template<class T, typename>
     variant<Ts...>::variant(T&& t)
-        : m_index(util::type_list_index<typename std::remove_reference<T>::type, Ts...>::value)
+        : m_index(util::type_list_index<util::decay_t<T>, Ts...>::value)
     {
-        (mctrs()[m_index])(memory, &t);
+        const constexpr bool is_lvalue_arg =  std::is_lvalue_reference<T>::value;
+        (cnvrt_ctors<is_lvalue_arg>()[m_index])(memory, const_cast<util::decay_t<T> *>(&t));
     }
 
     template<typename... Ts>
@@ -301,17 +311,25 @@ namespace util
     template<typename T, typename>
     variant<Ts...>& variant<Ts...>::operator=(T&& t) noexcept
     {
-        using decayed_t = typename std::decay<T>::type;
+        using decayed_t = util::decay_t<T>;
         // FIXME: No version with implicit type conversion available!
-        static const constexpr std::size_t t_index =
+        const constexpr std::size_t t_index =
             util::type_list_index<decayed_t, Ts...>::value;
 
-        if (t_index == m_index)
+        const constexpr bool is_lvalue_arg =  std::is_lvalue_reference<T>::value;
+
+        if (t_index != m_index)
+        {
+            (dtors()[m_index])(memory);
+            (cnvrt_ctors<is_lvalue_arg>()[t_index])(memory, &t);
+            m_index = t_index;
+        }
+        else
         {
-            util::get<decayed_t>(*this) = std::forward<T>(t);
-            return *this;
+            (cnvrt_assgnrs<is_lvalue_arg>()[m_index])(memory, &t);
         }
-        else return (*this = variant(std::forward<T>(t)));
+        return *this;
+
     }
 
     template<typename... Ts>

modules/gapi/test/util/variant_tests.cpp

@@ -59,7 +59,9 @@ TEST(Variant, ConvertingCTorMove)
     util::variant<std::string, int> vsi3(std::move(rvs));
     EXPECT_EQ(0u,     vsi3.index());
     EXPECT_EQ("2017", util::get<std::string>(vsi3));
-    EXPECT_EQ("", rvs) <<"Rvalue source argument was not moved from while should?";
+    //C++ standard state that std::string instance that was moved from stays in valid, but unspecified state.
+    //So the best assumption we can made here is that s is not the same as it was before move.
+    EXPECT_NE("2017", rvs) <<"Rvalue source argument was not moved from while should?";
 
     util::variant<std::string, int> vsi2(42);
     EXPECT_EQ(1u,     vsi2.index());
@@ -174,7 +176,25 @@ TEST(Variant, Assign_RValueRef_DiffType)
     vis = std::move(s);
     EXPECT_EQ(1u, vis.index());
     EXPECT_EQ("42", util::get<std::string>(vis));
-    EXPECT_EQ("", s) << "right hand side argument of assignment operation was not moved from while should?";;
+    //C++ standard state that std::string instance that was moved from stays in valid, but unspecified state.
+    //So the best assumption we can made here is that s is not the same as it was before move.
+    EXPECT_NE("42", s) << "right hand side argument of assignment operation was not moved from while should?";
+}
+
+TEST(Variant, Assign_RValueRef_SameType)
+{
+    TestVar vis(std::string("43"));
+
+    EXPECT_EQ(1u, vis.index());
+    EXPECT_EQ("43", util::get<std::string>(vis));
+
+    std::string s("42");
+    vis = std::move(s);
+    EXPECT_EQ(1u, vis.index());
+    EXPECT_EQ("42", util::get<std::string>(vis));
+    //C++ standard state that std::string instance that was moved from stays in valid, but unspecified state.
+    //So the best assumption we can made here is that s is not the same as it was before move.
+    EXPECT_NE("42", s) << "right hand side argument of assignment operation was not moved from while should?";
 }
 
 TEST(Variant, Assign_LValueRef_DiffType)
@@ -191,7 +211,7 @@ TEST(Variant, Assign_LValueRef_DiffType)
     EXPECT_EQ("42", s) << "right hand side argument of assignment operation was moved from while should not ?";
 }
 
-TEST(Variant, Assign_ValueUpdate_Const)
+TEST(Variant, Assign_ValueUpdate_Const_Variant)
 {
     TestVar va(42);
     const TestVar vb(43);
@@ -208,7 +228,7 @@ TEST(Variant, Assign_ValueUpdate_Const)
     EXPECT_EQ(43, util::get<int>(va));
 }
 
-TEST(Variant, Assign_ValueUpdate_Const_DiffType)
+TEST(Variant, Assign_ValueUpdate_Const_DiffType_Variant)
 {
     TestVar va(42);
     const TestVar vb(std::string("42"));
@@ -229,6 +249,7 @@ TEST(Variant, Assign_Move_Variant)
 {
     TestVar va(42);
     TestVar vb(std::string("42"));
+    TestVar vd(std::string("43"));
     TestVar vc(43);
 
     EXPECT_EQ(0u, va.index());
@@ -240,10 +261,24 @@ TEST(Variant, Assign_Move_Variant)
     EXPECT_EQ(0u, vc.index());
     EXPECT_EQ(43, util::get<int>(vc));
 
+    EXPECT_EQ(1u, vd.index());
+    EXPECT_EQ("43", util::get<std::string>(vd));
+
     va = std::move(vb);
     EXPECT_EQ(1u, va.index());
     EXPECT_EQ("42", util::get<std::string>(va));
 
+    EXPECT_EQ(1u, vb.index());
+    EXPECT_EQ("", util::get<std::string>(vb));
+
+
+    vb = std::move(vd);
+    EXPECT_EQ(1u, vb.index());
+    EXPECT_EQ("43", util::get<std::string>(vb));
+
+    EXPECT_EQ(1u, vd.index());
+    EXPECT_EQ("", util::get<std::string>(vd));
+
     va = std::move(vc);
     EXPECT_EQ(0u, va.index());
     EXPECT_EQ(43, util::get<int>(va));