From 8a0f7db8ebc2088a881457b26fadae816e21ee72 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Sat, 30 Jul 2022 15:50:06 +0200 Subject: [PATCH] Utils: Make take of SmallString explicit Using a value after the move is undefined. For example, if you move an integer, it is not set to zero after the move. So you have to initialize it again. Adding the take method to SmallString makes this explicit. So if there is a case that you want to use the string again after a move, use take or initialize it yourself. Change-Id: I174116df9639d6a3f63c6f2c2a3bd184852927ce Reviewed-by: Tim Jenssen --- src/libs/utils/smallstring.h | 14 +++++++------- tests/unit/unittest/smallstring-test.cpp | 24 +++++++++++++++++++++++- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/libs/utils/smallstring.h b/src/libs/utils/smallstring.h index 1e5d2cb567f..1cdeea178be 100644 --- a/src/libs/utils/smallstring.h +++ b/src/libs/utils/smallstring.h @@ -195,17 +195,17 @@ public: BasicSmallString &operator=(BasicSmallString &&other) noexcept { - if (this == &other) - return *this; - - this->~BasicSmallString(); - - m_data = std::move(other.m_data); - other.m_data.reset(); + swap(*this, other); return *this; } + BasicSmallString take() noexcept + { + auto tmp = std::move(*this); + + return tmp; + } BasicSmallString clone() const { BasicSmallString clonedString(m_data); diff --git a/tests/unit/unittest/smallstring-test.cpp b/tests/unit/unittest/smallstring-test.cpp index 585efc19743..a45c36918f6 100644 --- a/tests/unit/unittest/smallstring-test.cpp +++ b/tests/unit/unittest/smallstring-test.cpp @@ -1560,7 +1560,7 @@ TEST(SmallString, ShortSmallStringMoveAssignment) copy = std::move(text); - ASSERT_THAT(text, IsEmpty()); + ASSERT_THAT(text, SmallString("more text")); ASSERT_THAT(copy, SmallString("text")); } @@ -1571,6 +1571,28 @@ TEST(SmallString, LongSmallStringMoveAssignment) copy = std::move(text); + ASSERT_THAT(text, SmallString("more text")); + ASSERT_THAT(copy, SmallString("this is a very very very very long text")); +} + +TEST(SmallString, ShortSmallStringTake) +{ + SmallString text("text"); + SmallString copy("more text"); + + copy = text.take(); + + ASSERT_THAT(text, IsEmpty()); + ASSERT_THAT(copy, SmallString("text")); +} + +TEST(SmallString, LongSmallStringTake) +{ + SmallString text("this is a very very very very long text"); + SmallString copy("more text"); + + copy = text.take(); + ASSERT_THAT(text, IsEmpty()); ASSERT_THAT(copy, SmallString("this is a very very very very long text")); }