From 32fbc8a51ff7e5b09340f97ac8685bd5024e4fb4 Mon Sep 17 00:00:00 2001 From: Michele Caini Date: Mon, 13 Jan 2025 12:13:06 +0100 Subject: [PATCH] any/meta_any: safe but unspecified state after self move - close #1206 Related Work Items: #1 --- src/entt/core/any.hpp | 6 ++++-- src/entt/meta/meta.hpp | 8 +++++--- test/entt/core/any.cpp | 30 ++++++++++++++++++++++++------ test/entt/meta/meta_any.cpp | 24 ++++++++++++++++++------ 4 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/entt/core/any.hpp b/src/entt/core/any.hpp index 2c1923160..f28f16b4c 100644 --- a/src/entt/core/any.hpp +++ b/src/entt/core/any.hpp @@ -249,12 +249,14 @@ class basic_any { /** * @brief Move assignment operator. + * + * @warning + * Self-moving puts objects in a safe but unspecified state. + * * @param other The instance to move from. * @return This any object. */ basic_any &operator=(basic_any &&other) noexcept { - ENTT_ASSERT(this != &other, "Self move assignment"); - reset(); if(other.mode == any_policy::embedded) { diff --git a/src/entt/meta/meta.hpp b/src/entt/meta/meta.hpp index 2d49c5bb8..016800861 100644 --- a/src/entt/meta/meta.hpp +++ b/src/entt/meta/meta.hpp @@ -357,13 +357,15 @@ class meta_any { /** * @brief Move assignment operator. + * + * @warning + * Self-moving puts objects in a safe but unspecified state. + * * @param other The instance to move from. * @return This meta any object. */ meta_any &operator=(meta_any &&other) noexcept { - ENTT_ASSERT(this != &other, "Self move assignment"); - - release(); + reset(); storage = std::move(other.storage); ctx = other.ctx; node = std::exchange(other.node, internal::meta_type_node{}); diff --git a/test/entt/core/any.cpp b/test/entt/core/any.cpp index f2d3434d1..81660d1c4 100644 --- a/test/entt/core/any.cpp +++ b/test/entt/core/any.cpp @@ -282,11 +282,17 @@ TEST(Any, SBOMoveAssignment) { ASSERT_EQ(entt::any_cast(other), 2); } -ENTT_DEBUG_TEST(AnyDeathTest, SBOSelfMoveAssignment) { +TEST(AnyDeathTest, SBOSelfMoveAssignment) { entt::any any{2}; // avoid warnings due to self-assignment - ASSERT_DEATH(any = std::move(*&any), ""); + any = std::move(*&any); + + ASSERT_FALSE(any); + ASSERT_FALSE(any.owner()); + ASSERT_EQ(any.policy(), entt::any_policy::empty); + ASSERT_EQ(any.type(), entt::type_id()); + ASSERT_EQ(any.data(), nullptr); } TEST(Any, SBODirectAssignment) { @@ -600,12 +606,18 @@ TEST(Any, NoSBOMoveAssignment) { ASSERT_EQ(entt::any_cast(other), instance); } -ENTT_DEBUG_TEST(AnyDeathTest, NoSBOSelfMoveAssignment) { +TEST(AnyDeathTest, NoSBOSelfMoveAssignment) { const fat instance{.1, .2, .3, .4}; entt::any any{instance}; // avoid warnings due to self-assignment - ASSERT_DEATH(any = std::move(*&any), ""); + any = std::move(*&any); + + ASSERT_FALSE(any); + ASSERT_FALSE(any.owner()); + ASSERT_EQ(any.policy(), entt::any_policy::empty); + ASSERT_EQ(any.type(), entt::type_id()); + ASSERT_EQ(any.data(), nullptr); } TEST(Any, NoSBODirectAssignment) { @@ -808,11 +820,17 @@ TEST(Any, VoidMoveAssignment) { ASSERT_EQ(entt::any_cast(&other), nullptr); } -ENTT_DEBUG_TEST(AnyDeathTest, VoidSelfMoveAssignment) { +TEST(AnyDeathTest, VoidSelfMoveAssignment) { entt::any any{std::in_place_type}; // avoid warnings due to self-assignment - ASSERT_DEATH(any = std::move(*&any), ""); + any = std::move(*&any); + + ASSERT_FALSE(any); + ASSERT_FALSE(any.owner()); + ASSERT_EQ(any.policy(), entt::any_policy::empty); + ASSERT_EQ(any.type(), entt::type_id()); + ASSERT_EQ(any.data(), nullptr); } TEST(Any, SBOMoveValidButUnspecifiedState) { diff --git a/test/entt/meta/meta_any.cpp b/test/entt/meta/meta_any.cpp index 38e3d9cb1..8d6aca2de 100644 --- a/test/entt/meta/meta_any.cpp +++ b/test/entt/meta/meta_any.cpp @@ -356,11 +356,15 @@ TEST_F(MetaAny, SBOMoveAssignment) { ASSERT_NE(other, entt::meta_any{0}); } -ENTT_DEBUG_TEST_F(MetaAnyDeathTest, SBOSelfMoveAssignment) { +TEST_F(MetaAnyDeathTest, SBOSelfMoveAssignment) { entt::meta_any any{3}; // avoid warnings due to self-assignment - ASSERT_DEATH(any = std::move(*&any), ""); + any = std::move(*&any); + + ASSERT_FALSE(any); + ASSERT_FALSE(any.type()); + ASSERT_EQ(any.base().data(), nullptr); } TEST_F(MetaAny, SBODirectAssignment) { @@ -685,12 +689,16 @@ TEST_F(MetaAny, NoSBOMoveAssignment) { ASSERT_NE(other, fat{}); } -ENTT_DEBUG_TEST_F(MetaAnyDeathTest, NoSBOSelfMoveAssignment) { +TEST_F(MetaAnyDeathTest, NoSBOSelfMoveAssignment) { const fat instance{.1, .2, .3, .4}; entt::meta_any any{instance}; // avoid warnings due to self-assignment - ASSERT_DEATH(any = std::move(*&any), ""); + any = std::move(*&any); + + ASSERT_FALSE(any); + ASSERT_FALSE(any.type()); + ASSERT_EQ(any.base().data(), nullptr); } TEST_F(MetaAny, NoSBODirectAssignment) { @@ -908,11 +916,15 @@ TEST_F(MetaAny, VoidMoveAssignment) { ASSERT_EQ(other, entt::meta_any{std::in_place_type}); } -ENTT_DEBUG_TEST_F(MetaAnyDeathTest, VoidSelfMoveAssignment) { +TEST_F(MetaAnyDeathTest, VoidSelfMoveAssignment) { entt::meta_any any{std::in_place_type}; // avoid warnings due to self-assignment - ASSERT_DEATH(any = std::move(*&any), ""); + any = std::move(*&any); + + ASSERT_FALSE(any); + ASSERT_FALSE(any.type()); + ASSERT_EQ(any.base().data(), nullptr); } TEST_F(MetaAny, SBOMoveInvalidate) {