From faceb057755166fac98987b64b732ee781d36722 Mon Sep 17 00:00:00 2001 From: Anton Potapov Date: Thu, 26 Mar 2020 12:08:14 +0300 Subject: [PATCH] G-API utils - make variant converting constructor and assignment operator properly forward it's argument --- .../include/opencv2/gapi/util/variant.hpp | 44 ++++++++++++------- modules/gapi/test/util/variant_tests.cpp | 33 ++++++++++++-- 2 files changed, 58 insertions(+), 19 deletions(-) diff --git a/modules/gapi/include/opencv2/gapi/util/variant.hpp b/modules/gapi/include/opencv2/gapi/util/variant.hpp index ee0590bf7f..22dfb2ea77 100644 --- a/modules/gapi/include/opencv2/gapi/util/variant.hpp +++ b/modules/gapi/include/opencv2/gapi/util/variant.hpp @@ -36,11 +36,14 @@ namespace util static const constexpr std::size_t value = I; }; + template< bool B, class T = void > + using enable_if_t = typename std::enable_if::type; - template using are_different = - std::enable_if::type, - typename std::decay::type>::value, - V>; + template + using are_different_t = enable_if_t< + !std::is_same::type, + typename std::decay::type>::value, + V>; } template @@ -157,10 +160,17 @@ namespace util variant(const variant& other); variant(variant&& other) noexcept; template explicit variant(const T& t); - // are_different is a SFINAE trick to avoid variant(T &&t) with T=variant + // 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) - template explicit variant(T&& t, typename detail::are_different::type = 0); + // variant is used in STL containers (examples: vector assignment). + // detail::enable_if_t::value> is a SFINAE + // trick to limit this constructor only to rvalue reference argument + template< + typename T, + typename = detail::are_different_t, + typename = detail::enable_if_t::value> + > + explicit variant(T&& t); // template explicit variant(Args&&... args); // FIXME: other constructors @@ -172,9 +182,11 @@ namespace util variant& operator=(variant &&rhs) noexcept; // SFINAE trick to avoid operator=(T&&) with T=variant<>, see comment above - template - typename detail::are_different - ::type operator=(T&& t) noexcept; + template< + typename T, + typename = detail::are_different_t + > + variant& operator=(T&& t) noexcept; // Observers std::size_t index() const noexcept; @@ -232,8 +244,8 @@ namespace util } template - template - variant::variant(T&& t, typename detail::are_different::type) + template + variant::variant(T&& t) : m_index(util::type_list_index::type, Ts...>::value) { (mctrs()[m_index])(memory, &t); @@ -278,8 +290,8 @@ namespace util } template - template typename detail::are_different, T, variant&> - ::type variant::operator=(T&& t) noexcept + template + variant& variant::operator=(T&& t) noexcept { using decayed_t = typename std::decay::type; // FIXME: No version with implicit type conversion available! @@ -288,10 +300,10 @@ namespace util if (t_index == m_index) { - util::get(*this) = std::move(t); + util::get(*this) = std::forward(t); return *this; } - else return (*this = variant(std::move(t))); + else return (*this = variant(std::forward(t))); } template diff --git a/modules/gapi/test/util/variant_tests.cpp b/modules/gapi/test/util/variant_tests.cpp index f85b1fbf72..41aa4afa3d 100644 --- a/modules/gapi/test/util/variant_tests.cpp +++ b/modules/gapi/test/util/variant_tests.cpp @@ -33,7 +33,7 @@ TEST(Variant, EmptyCTor) EXPECT_EQ("", util::get(vsi)); } -TEST(Variant, ValueMoveCTor) +TEST(Variant, ConvertingCTorMove) { util::variant vi(42); EXPECT_EQ(0u, vi.index()); @@ -55,16 +55,23 @@ TEST(Variant, ValueMoveCTor) EXPECT_EQ(0u, vsi.index()); EXPECT_EQ("2017", util::get(vsi)); + std::string rvs("2017"); + util::variant vsi3(std::move(rvs)); + EXPECT_EQ(0u, vsi3.index()); + EXPECT_EQ("2017", util::get(vsi3)); + EXPECT_EQ("", rvs) <<"Rvalue source argument was not moved from while should?"; + util::variant vsi2(42); EXPECT_EQ(1u, vsi2.index()); EXPECT_EQ(42, util::get(vsi2)); } -TEST(Variant, ValueCopyCTor) +TEST(Variant, ConvertingCTorCopy) { const int i42 = 42; const int i17 = 2017; const std::string s17 = "2017"; + std::string s17_lvref = s17; util::variant vi(i42); EXPECT_EQ(0u, vi.index()); @@ -82,6 +89,11 @@ TEST(Variant, ValueCopyCTor) EXPECT_EQ(0u, vs.index()); EXPECT_EQ(s17, util::get(vs)); + util::variant vs_lv(s17_lvref); + EXPECT_EQ(0u, vs_lv.index()); + EXPECT_EQ(s17, s17_lvref); + EXPECT_EQ(s17_lvref, util::get(vs_lv)); + util::variant vsi(s17); EXPECT_EQ(0u, vsi.index()); EXPECT_EQ(s17, util::get(vsi)); @@ -151,6 +163,20 @@ TEST(Variant, Assign_ValueUpdate_DiffType) EXPECT_EQ("42", util::get(vis)); } +TEST(Variant, Assign_RValueRef_DiffType) +{ + TestVar vis(42); + + EXPECT_EQ(0u, vis.index()); + EXPECT_EQ(42, util::get(vis)); + + std::string s("42"); + vis = std::move(s); + EXPECT_EQ(1u, vis.index()); + EXPECT_EQ("42", util::get(vis)); + EXPECT_EQ("", s) << "right hand side argument of assignment operation was not moved from while should?";; +} + TEST(Variant, Assign_LValueRef_DiffType) { TestVar vis(42); @@ -162,6 +188,7 @@ TEST(Variant, Assign_LValueRef_DiffType) vis = s; EXPECT_EQ(1u, vis.index()); EXPECT_EQ("42", util::get(vis)); + EXPECT_EQ("42", s) << "right hand side argument of assignment operation was moved from while should not ?"; } TEST(Variant, Assign_ValueUpdate_Const) @@ -198,7 +225,7 @@ TEST(Variant, Assign_ValueUpdate_Const_DiffType) EXPECT_EQ("42", util::get(va)); } -TEST(Variant, Assign_Move) +TEST(Variant, Assign_Move_Variant) { TestVar va(42); TestVar vb(std::string("42"));