Skip to content

Commit

Permalink
any/meta_any: safe but unspecified state after self move - close #1206
Browse files Browse the repository at this point in the history
Related Work Items: #1
  • Loading branch information
skypjack committed Jan 13, 2025
1 parent 6f5e935 commit 32fbc8a
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 17 deletions.
6 changes: 4 additions & 2 deletions src/entt/core/any.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 5 additions & 3 deletions src/entt/meta/meta.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{});
Expand Down
30 changes: 24 additions & 6 deletions test/entt/core/any.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,11 +282,17 @@ TEST(Any, SBOMoveAssignment) {
ASSERT_EQ(entt::any_cast<int>(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<void>());
ASSERT_EQ(any.data(), nullptr);
}

TEST(Any, SBODirectAssignment) {
Expand Down Expand Up @@ -600,12 +606,18 @@ TEST(Any, NoSBOMoveAssignment) {
ASSERT_EQ(entt::any_cast<fat>(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<void>());
ASSERT_EQ(any.data(), nullptr);
}

TEST(Any, NoSBODirectAssignment) {
Expand Down Expand Up @@ -808,11 +820,17 @@ TEST(Any, VoidMoveAssignment) {
ASSERT_EQ(entt::any_cast<double>(&other), nullptr);
}

ENTT_DEBUG_TEST(AnyDeathTest, VoidSelfMoveAssignment) {
TEST(AnyDeathTest, VoidSelfMoveAssignment) {
entt::any any{std::in_place_type<void>};

// 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<void>());
ASSERT_EQ(any.data(), nullptr);
}

TEST(Any, SBOMoveValidButUnspecifiedState) {
Expand Down
24 changes: 18 additions & 6 deletions test/entt/meta/meta_any.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -908,11 +916,15 @@ TEST_F(MetaAny, VoidMoveAssignment) {
ASSERT_EQ(other, entt::meta_any{std::in_place_type<void>});
}

ENTT_DEBUG_TEST_F(MetaAnyDeathTest, VoidSelfMoveAssignment) {
TEST_F(MetaAnyDeathTest, VoidSelfMoveAssignment) {
entt::meta_any any{std::in_place_type<void>};

// 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) {
Expand Down

0 comments on commit 32fbc8a

Please sign in to comment.