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
diff --git a/dexlayout/dex_ir.h b/dexlayout/dex_ir.h
index 66ca84b..c819c67 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 @@
// 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;
}
}