Refactor CollectionVector::SortByMapOrder

The current implementation of the SortByMapOrder method is
making use of std::unique_ptr::release to clear the pointer
maintained in the vector during the sorting phase in order
to avoid potential double-free errors, but this is triggering
the bugprone-unused-return-value check in clang-tidy. This
refactor gets rid of the need to call release, making it
programmatically safe to move pointers while sorting, while
maintaining the same O(n) time complexity.

Bug: 213953102
Test: m tidy-art
Test: atest art_dexlayout_tests (especially DexLayoutTest.DexFileOutput)
Change-Id: I525299c347d89171f91e10323d5aa8702e22b8d5

1 files changed, 14 insertions, 6 deletions

diff --git a/dexlayout/dex_ir.h b/dexlayout/dex_ir.h
index 66ca84bbd4..c819c672e3 100644
--- a/dexlayout/dex_ir.h
+++ b/dexlayout/dex_ir.h
@@ -20,7 +20,7 @@
 #define ART_DEXLAYOUT_DEX_IR_H_
 
 #include <stdint.h>
-
+#include <unordered_map>
 #include <vector>
 
 #include "base/iteration_range.h"
@@ -262,13 +262,21 @@ template<class T> class CollectionVector : public CollectionBase {
   // Sort the vector by copying pointers over.
   template <typename MapType>
   void SortByMapOrder(const MapType& map) {
-    auto it = map.begin();
     CHECK_EQ(map.size(), Size());
+
+    // Move all pointers to a temporary map owning the pointers.
+    std::unordered_map<T*, ElementType> pointers_map;
+    pointers_map.reserve(Size());
+    for (std::unique_ptr<T>& element : collection_) {
+      pointers_map[element.get()] = std::move(element);
+    }
+
+    // Move back the pointers to the original vector according to the map order.
+    auto it = map.begin();
     for (size_t i = 0; i < Size(); ++i) {
-      // There are times when the array will temporarily contain the same pointer twice, doing the
-      // release here sure there is no double free errors.
-      collection_[i].release();
-      collection_[i].reset(it->second);
+      auto element_it = pointers_map.find(it->second);
+      DCHECK(element_it != pointers_map.end());
+      collection_[i] = std::move(element_it->second);
       ++it;
     }
   }