[heap] introduce Move() and unify TakeFrom() for move semantics (#11972)

This change enhances the move semantics for heap-allocated container
classes (`Array`, `Data`, and `String`) by introducing a consistent
pattern.

A new `Move()` method is added to `Heap::Array`, `Heap::Data`, and
`Heap::String`. This method returns an rvalue reference to the object,
making the intent to transfer ownership explicit at the call site.

The existing `TakeFrom()` methods are updated to accept an rvalue
reference and now include a check to prevent self-assignment, which
improves robustness.

For consistency, `Heap::Data::SetFrom(Data&&)` and
`Heap::String::Set(String&&)` are renamed to `TakeFrom()`.

All call sites are updated to use the new `foo.TakeFrom(bar.Move())`
pattern, replacing the more verbose and less clear
`static_cast<...&&>(bar)`.

Unit tests are updated to validate the new `TakeFrom()` and `Move()`
semantics, including tests for self-assignment and moving from a
null (empty) container

src/core/border_router/routing_manager.cpp

@@ -3708,7 +3708,7 @@ Error RoutingManager::RioAdvertiser::AppendRios(RouterAdvert::TxMessage &aRaMess
     const OmrPrefixManager         &omrPrefixManager = Get<RoutingManager>().mOmrPrefixManager;
 
 #if OPENTHREAD_CONFIG_BORDER_ROUTING_USE_HEAP_ENABLE
-    oldPrefixes.TakeFrom(static_cast<RioPrefixArray &&>(mPrefixes));
+    oldPrefixes.TakeFrom(mPrefixes.Move());
 #else
     oldPrefixes = mPrefixes;
 #endif

src/core/common/heap_array.hpp

@@ -153,21 +153,37 @@ public:
     Error ReserveCapacity(IndexType aCapacity) { return Allocate(aCapacity); }
 
     /**
-     * Sets the array by taking the buffer from another given array (using move semantics).
+     * Sets the array by taking ownership of the buffer from another `Array`.
      *
-     * @param[in] aOther    The other `Heap::Array` to take from (rvalue reference).
+     * This method uses move semantics. After the call, `aOther` will be empty and this `Array` will hold the
+     * buffer previously held by `aOther`.
+     *
+     * @param[in] aOther     An rvalue reference to another `Array` to take from.
      */
     void TakeFrom(Array &&aOther)
     {
-        Free();
-        mArray           = aOther.mArray;
-        mLength          = aOther.mLength;
-        mCapacity        = aOther.mCapacity;
-        aOther.mArray    = nullptr;
-        aOther.mLength   = 0;
-        aOther.mCapacity = 0;
+        if (&aOther != this)
+        {
+            Free();
+            mArray           = aOther.mArray;
+            mLength          = aOther.mLength;
+            mCapacity        = aOther.mCapacity;
+            aOther.mArray    = nullptr;
+            aOther.mLength   = 0;
+            aOther.mCapacity = 0;
+        }
     }
 
+    /**
+     * Casts the `Array` to an rvalue reference.
+     *
+     * This method is intended to be used with `TakeFrom()` to explicitly indicate a move operation and transfer of
+     * the underlying buffer.
+     *
+     * @returns An rvalue reference to this `Array`.
+     */
+    Array &&Move(void) { return static_cast<Array &&>(*this); }
+
     /**
      * Overloads the `[]` operator to get the element at a given index.
      *

src/core/common/heap_data.cpp

@@ -72,10 +72,14 @@ exit:
     return error;
 }
 
-void Data::SetFrom(Data &&aData)
+void Data::TakeFrom(Data &&aData)
 {
-    Free();
-    TakeFrom(aData);
+    if (&aData != this)
+    {
+        Free();
+        mData.Init(aData.mData.GetBytes(), aData.GetLength());
+        aData.mData.Init(nullptr, 0);
+    }
 }
 
 bool Data::Matches(const uint8_t *aBuffer, uint16_t aLength) const
@@ -127,11 +131,5 @@ exit:
     return error;
 }
 
-void Data::TakeFrom(Data &aData)
-{
-    mData.Init(aData.mData.GetBytes(), aData.GetLength());
-    aData.mData.Init(nullptr, 0);
-}
-
 } // namespace Heap
 } // namespace ot

src/core/common/heap_data.hpp

@@ -62,7 +62,7 @@ public:
      *
      * @param[in] aData   An rvalue reference to another `Heap::Data` to move from.
      */
-    Data(Data &&aData) { TakeFrom(aData); }
+    Data(Data &&aData) { TakeFrom(aData.Move()); }
 
     /**
      * This is the destructor for `Heap::Data` object.
@@ -131,11 +131,24 @@ public:
     Error SetFrom(const Message &aMessage, uint16_t aOffset, uint16_t aLength);
 
     /**
-     * Sets the `Heap::Data` from another one (move semantics).
+     * Sets the `Heap::Data` by taking ownership of the buffer from another `Heap::Data`.
      *
-     * @param[in] aData   The other `Heap::Data` to set from (rvalue reference).
+     * This method uses move semantics. After the call, `aData` will be null and this `Data` will hold the buffer
+     * previously held by `aData`.
+     *
+     * @param[in] aData     An rvalue reference to another `Heap::Data` to take from.
      */
-    void SetFrom(Data &&aData);
+    void TakeFrom(Data &&aData);
+
+    /**
+     * Casts the `Heap::Data` to an rvalue reference.
+     *
+     * This method is intended to be used with `TakeFrom()` to explicitly indicate a move operation and transfer of
+     * the underlying buffer.
+     *
+     * @returns An rvalue reference to this `Heap::Data`.
+     */
+    Data &&Move(void) { return static_cast<Data &&>(*this); }
 
     /**
      * Appends the bytes from `Heap::Data` to a given message.

src/core/common/heap_string.cpp

@@ -66,16 +66,14 @@ exit:
     return error;
 }
 
-Error String::Set(String &&aString)
+void String::TakeFrom(String &&aString)
 {
-    VerifyOrExit(mStringBuffer != aString.mStringBuffer);
-
-    Heap::Free(mStringBuffer);
-    mStringBuffer         = aString.mStringBuffer;
-    aString.mStringBuffer = nullptr;
-
-exit:
-    return kErrorNone;
+    if (&aString != this)
+    {
+        Heap::Free(mStringBuffer);
+        mStringBuffer         = aString.mStringBuffer;
+        aString.mStringBuffer = nullptr;
+    }
 }
 
 void String::Free(void)

src/core/common/heap_string.hpp

@@ -115,14 +115,24 @@ public:
     Error Set(const String &aString) { return Set(aString.AsCString()); }
 
     /**
-     * Sets the string from another `String`.
+     * Sets the string by taking ownership of the buffer from another `String`.
      *
-     * @param[in] aString     The other `String` to set from (rvalue reference using move semantics).
+     * This method uses move semantics. After the call, `aString` will be null and this `String` will hold the
+     * buffer previously held by `aString`.
      *
-     * @retval kErrorNone     Successfully set the string.
-     * @retval kErrorNoBufs   Failed to allocate buffer for string.
+     * @param[in] aString     An rvalue reference to another `String` to take from.
      */
-    Error Set(String &&aString);
+    void TakeFrom(String &&aString);
+
+    /**
+     * Casts the `String` to an rvalue reference.
+     *
+     * This method is intended to be used with `TakeFrom()` to explicitly indicate a move operation and transfer of
+     * the underlying buffer.
+     *
+     * @returns An rvalue reference to this `String`.
+     */
+    String &&Move(void) { return static_cast<String &&>(*this); }
 
     /**
      * Frees any buffer allocated by the `String`.

src/core/net/mdns.cpp

@@ -1727,7 +1727,7 @@ void Core::LocalHost::HandleEventTimer(void)
         AddressArray &addresses = (addrType == kIp4AddrType) ? mIp4Addresses : mIp6Addresses;
         AddressArray  oldAddresses;
 
-        oldAddresses.TakeFrom(static_cast<AddressArray &&>(addresses));
+        oldAddresses.TakeFrom(addresses.Move());
         addresses.Clear();
 
         // First, add existing addresses (from old list) that did not
@@ -7450,7 +7450,7 @@ Core::RecordCache::NewRecordEntry::NewRecordEntry(const ResourceRecord &aRecord,
     , mCacheFlush(aRecord.GetClass() & kClassCacheFlushFlag)
     , mType(aRecord.GetType())
     , mTtl(aRecord.GetTtl())
-    , mData(static_cast<Heap::Data &&>(aData))
+    , mData(aData.Move())
 {
 }
 
@@ -7467,7 +7467,7 @@ bool Core::RecordCache::NewRecordEntry::Matches(uint16_t aType, const Heap::Data
 Core::RecordCache::RecordEntry::RecordEntry(NewRecordEntry &aNewEntry)
     : mNext(nullptr)
     , mType(aNewEntry.mType)
-    , mData(static_cast<Heap::Data &&>(aNewEntry.mData))
+    , mData(aNewEntry.mData.Move())
 {
     mRecord.RefreshTtl(aNewEntry.mTtl);
 }

src/core/net/srp_advertising_proxy.cpp

@@ -1006,7 +1006,7 @@ void AdvertisingProxy::RegisterService(Service &aService)
 
         IgnoreError(Server::Service::ParseSubTypeServiceName(subTypeName.AsCString(), label, sizeof(label)));
         SuccessOrExit(error = labelString.Set(label));
-        IgnoreError(subTypeHeapStrings.PushBack(static_cast<Heap::String &&>(labelString)));
+        IgnoreError(subTypeHeapStrings.PushBack(labelString.Move()));
         IgnoreError(subTypeLabels.PushBack(subTypeHeapStrings.Back()->AsCString()));
     }
 

tests/unit/test_heap_array.cpp

@@ -326,7 +326,7 @@ void TestHeapArrayOfUint16(void)
 
     VerifyArray(array2, 0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26);
 
-    array2.TakeFrom(static_cast<Heap::Array<uint16_t, 2> &&>(array));
+    array2.TakeFrom(array.Move());
 
     VerifyArray(array);
     VerifyOrQuit(array.GetCapacity() == 0);
@@ -334,6 +334,18 @@ void TestHeapArrayOfUint16(void)
     VerifyArray(array2, 0, 1, 2, 3, 4, 5);
     VerifyOrQuit(array2.GetCapacity() == 10);
 
+    // Test moving from self
+    array2.TakeFrom(array2.Move());
+    VerifyArray(array2, 0, 1, 2, 3, 4, 5);
+    VerifyOrQuit(array2.GetCapacity() == 10);
+
+    // Test moving a null array
+    array2.TakeFrom(array.Move());
+    VerifyArray(array);
+    VerifyOrQuit(array.GetCapacity() == 0);
+    VerifyArray(array2);
+    VerifyOrQuit(array2.GetCapacity() == 0);
+
     printf("\n -- PASS\n");
 }
 
@@ -465,13 +477,25 @@ void TestHeapArray(void)
             SuccessOrQuit(array2.PushBack(Entry(num + 0x20)));
         }
 
-        array2.TakeFrom(static_cast<Heap::Array<Entry, 2> &&>(array));
+        array2.TakeFrom(array.Move());
 
         VerifyOrQuit(array.GetLength() == 0);
         VerifyOrQuit(array.GetCapacity() == 0);
 
         VerifyArray(array2, 0, 1, 2, 3, 4, 5);
         VerifyOrQuit(array2.GetCapacity() == 10);
+
+        // Test moving from self
+        array2.TakeFrom(array2.Move());
+        VerifyArray(array2, 0, 1, 2, 3, 4, 5);
+        VerifyOrQuit(array2.GetCapacity() == 10);
+
+        // Test moving a null array
+        array2.TakeFrom(array.Move());
+        VerifyArray(array);
+        VerifyOrQuit(array.GetCapacity() == 0);
+        VerifyArray(array2);
+        VerifyOrQuit(array2.GetCapacity() == 0);
     }
 
     printf("------------------------------------------------------------------------------------\n");

tests/unit/test_heap_string.cpp

@@ -139,6 +139,27 @@ void TestHeapString(void)
     SuccessOrQuit(str1.Set(GetName()), "Set() with move semantics failed");
     VerifyString("str1", str1, "name");
 
+    printf("------------------------------------------------------------------------------------\n");
+    printf("TakeFrom() and Move()\n\n");
+
+    SuccessOrQuit(str1.Set("string to be moved"));
+    str2.Free();
+    VerifyString("str1", str1, "string to be moved");
+    VerifyString("str2", str2, nullptr);
+
+    str2.TakeFrom(str1.Move());
+    VerifyString("str1", str1, nullptr);
+    VerifyString("str2", str2, "string to be moved");
+
+    // Test moving from self
+    str2.TakeFrom(str2.Move());
+    VerifyString("str2", str2, "string to be moved");
+
+    // Test moving a null string
+    str2.TakeFrom(str1.Move());
+    VerifyString("str1", str1, nullptr);
+    VerifyString("str2", str2, nullptr);
+
     printf("------------------------------------------------------------------------------------\n");
     printf("operator==() with two null string\n\n");
     str1.Free();
@@ -208,6 +229,7 @@ void TestHeapData(void)
     MessagePool   *messagePool;
     Message       *message;
     Heap::Data     data;
+    Heap::Data     data2;
     uint16_t       offset;
     const uint8_t *oldBuffer;
 
@@ -314,9 +336,29 @@ void TestHeapData(void)
 
     printf("------------------------------------------------------------------------------------\n");
     printf("SetFrom() move semantics\n\n");
-    data.SetFrom(GetData());
+    data.TakeFrom(GetData().Move());
     VerifyData(data, &kTestValue, sizeof(kTestValue));
 
+    printf("------------------------------------------------------------------------------------\n");
+    printf("TakeFrom() and Move()\n\n");
+
+    SuccessOrQuit(data.SetFrom(kData1, sizeof(kData1)));
+    VerifyData(data, kData1);
+    VerifyData(data2, nullptr, 0);
+
+    data2.TakeFrom(data.Move());
+    VerifyData(data, nullptr, 0);
+    VerifyData(data2, kData1);
+
+    // Test moving from self
+    data2.TakeFrom(data2.Move());
+    VerifyData(data2, kData1);
+
+    // Test moving a null data
+    data2.TakeFrom(data.Move());
+    VerifyData(data, nullptr, 0);
+    VerifyData(data2, nullptr, 0);
+
     printf("\n -- PASS\n");
 }