From ade1518a4494578746a92f4026514eade136e819 Mon Sep 17 00:00:00 2001
From: jolestar <jolestar@gmail.com>
Date: Sun, 25 Aug 2024 18:59:29 +0800
Subject: [PATCH] [moveos_std] refactor sort to remove copy and drop
 requirements from the arguments (#2503)

[moveos_std] refactor sort to remove copy and drop requirements to the arguments
---
 frameworks/moveos-stdlib/doc/sort.md          |  8 +--
 frameworks/moveos-stdlib/sources/sort.move    | 58 ++++---------------
 .../moveos-stdlib/tests/sort_tests.move       | 43 +++++++++++++-
 3 files changed, 56 insertions(+), 53 deletions(-)

diff --git a/frameworks/moveos-stdlib/doc/sort.md b/frameworks/moveos-stdlib/doc/sort.md
index d22961be2..672b06467 100644
--- a/frameworks/moveos-stdlib/doc/sort.md
+++ b/frameworks/moveos-stdlib/doc/sort.md
@@ -24,7 +24,7 @@ Utility functions for sorting vector.
 Sorts a vector using quick sort algorithm.
 
 
-<pre><code><b>public</b> <b>fun</b> <a href="sort.md#0x2_sort_quick_sort">quick_sort</a>&lt;T: <b>copy</b>, drop&gt;(data: &<b>mut</b> <a href="">vector</a>&lt;T&gt;)
+<pre><code><b>public</b> <b>fun</b> <a href="sort.md#0x2_sort_quick_sort">quick_sort</a>&lt;T&gt;(data: &<b>mut</b> <a href="">vector</a>&lt;T&gt;)
 </code></pre>
 
 
@@ -37,7 +37,7 @@ Sorts a vector, returning a new vector with the sorted elements.
 The sort algorithm used is quick sort, it maybe changed in the future.
 
 
-<pre><code><b>public</b> <b>fun</b> <a href="sort.md#0x2_sort">sort</a>&lt;T: <b>copy</b>, drop&gt;(data: &<b>mut</b> <a href="">vector</a>&lt;T&gt;)
+<pre><code><b>public</b> <b>fun</b> <a href="sort.md#0x2_sort">sort</a>&lt;T&gt;(data: &<b>mut</b> <a href="">vector</a>&lt;T&gt;)
 </code></pre>
 
 
@@ -50,7 +50,7 @@ Sorts a vector using a custom comparison function.
 The comparison function should return true if the first element is greater than the second.
 
 
-<pre><code><b>public</b> <b>fun</b> <a href="sort.md#0x2_sort_sort_by_cmp">sort_by_cmp</a>&lt;T: <b>copy</b>, drop&gt;(data: &<b>mut</b> <a href="">vector</a>&lt;T&gt;, cmp: |(&T, &T)|bool)
+<pre><code><b>public</b> <b>fun</b> <a href="sort.md#0x2_sort_sort_by_cmp">sort_by_cmp</a>&lt;T&gt;(data: &<b>mut</b> <a href="">vector</a>&lt;T&gt;, cmp: |(&T, &T)|bool)
 </code></pre>
 
 
@@ -62,5 +62,5 @@ The comparison function should return true if the first element is greater than
 Sorts a vector using a custom key function.
 
 
-<pre><code><b>public</b> <b>fun</b> <a href="sort.md#0x2_sort_sort_by_key">sort_by_key</a>&lt;T: <b>copy</b>, drop, K: <b>copy</b>, drop&gt;(data: &<b>mut</b> <a href="">vector</a>&lt;T&gt;, key: |&T|K)
+<pre><code><b>public</b> <b>fun</b> <a href="sort.md#0x2_sort_sort_by_key">sort_by_key</a>&lt;T, K&gt;(data: &<b>mut</b> <a href="">vector</a>&lt;T&gt;, key: |&T|&K)
 </code></pre>
diff --git a/frameworks/moveos-stdlib/sources/sort.move b/frameworks/moveos-stdlib/sources/sort.move
index e2e0b3fe8..353bfd561 100644
--- a/frameworks/moveos-stdlib/sources/sort.move
+++ b/frameworks/moveos-stdlib/sources/sort.move
@@ -5,7 +5,7 @@ module moveos_std::sort {
     use moveos_std::compare;
     
     /// Sorts a vector using quick sort algorithm.
-    public fun quick_sort<T: copy + drop>(data: &mut vector<T>){
+    public fun quick_sort<T>(data: &mut vector<T>){
         let len = vector::length(data);
         if (len <= 1) {
             return
@@ -13,7 +13,7 @@ module moveos_std::sort {
         quick_sort_helper(data, 0, len - 1);
     }
 
-    fun quick_sort_helper<T: copy + drop>(data: &mut vector<T>, low: u64, high: u64) {
+    fun quick_sort_helper<T>(data: &mut vector<T>, low: u64, high: u64) {
         if (low < high) {
             let p = partition(data, low, high);
             if (p > 0) {
@@ -23,13 +23,15 @@ module moveos_std::sort {
         }
     }
 
-    fun partition<T: copy + drop>(data: &mut vector<T>, low: u64, high: u64): u64 {
-        let pivot = *vector::borrow(data, high);
+    fun partition<T>(data: &mut vector<T>, low: u64, high: u64): u64 {
         let i = low;
         let j = low;
         while (j < high) {
+            //for avoid pivot reference still alive when vector::swap
+            //we need to borrow it in the while loop, not before the loop
+            let pivot = vector::borrow(data, high);
             let value = vector::borrow(data, j);
-            let cmp = compare::compare(value, &pivot);
+            let cmp = compare::compare(value, pivot);
             if (cmp == compare::result_less_than()) {
                 vector::swap(data, i, j);
                 i = i + 1;
@@ -40,7 +42,7 @@ module moveos_std::sort {
         i
     }
 
-    inline fun bubble_sort<T: copy + drop>(data: &mut vector<T>, cmp: |&T, &T|bool) {
+    inline fun bubble_sort<T>(data: &mut vector<T>, cmp: |&T, &T|bool) {
         let len = vector::length(data);
         let swapped = true;
         while(swapped) {
@@ -62,59 +64,23 @@ module moveos_std::sort {
 
     /// Sorts a vector, returning a new vector with the sorted elements.
     /// The sort algorithm used is quick sort, it maybe changed in the future.
-    public fun sort<T: copy + drop>(data: &mut vector<T>){
+    public fun sort<T>(data: &mut vector<T>){
         quick_sort(data)
     }
 
     /// Sorts a vector using a custom comparison function.
     /// The comparison function should return true if the first element is greater than the second.
-    public inline fun sort_by_cmp<T: copy + drop>(data: &mut vector<T>, cmp: |&T, &T|bool){
+    public inline fun sort_by_cmp<T>(data: &mut vector<T>, cmp: |&T, &T|bool){
         bubble_sort(data, |a,b|{cmp(a,b)});
     }
 
     /// Sorts a vector using a custom key function.
-    public inline fun sort_by_key<T: copy + drop, K: copy + drop>(data: &mut vector<T>, key: |&T|K){
+    public inline fun sort_by_key<T, K>(data: &mut vector<T>, key: |&T|&K){
         bubble_sort(data, |a,b|{
             let a_key = key(a);
             let b_key = key(b);
-            compare::compare(&a_key, &b_key) == compare::result_greater_than()
+            compare::compare(a_key, b_key) == compare::result_greater_than()
         });
     }
     
-    #[test]
-    fun test_quick_sort() {
-        let data = vector<u64>[1, 3, 2, 5, 4];
-        quick_sort(&mut data);
-        assert!(vector::length<u64>(&data) == 5, 0);
-        assert!(*vector::borrow(&data, 0) == 1, 0);
-        assert!(*vector::borrow(&data, 1) == 2, 0);
-        assert!(*vector::borrow(&data, 2) == 3, 0);
-        assert!(*vector::borrow(&data, 3) == 4, 0);
-        assert!(*vector::borrow(&data, 4) == 5, 0);
-    }
-
-    #[test]
-    fun test_quick_sort_u128() {
-        let data = vector<u128>[1, 3, 2, 5, 4];
-        quick_sort(&mut data);
-        assert!(vector::length<u128>(&data) == 5, 0);
-        assert!(*vector::borrow(&data, 0) == 1, 0);
-        assert!(*vector::borrow(&data, 1) == 2, 0);
-        assert!(*vector::borrow(&data, 2) == 3, 0);
-        assert!(*vector::borrow(&data, 3) == 4, 0);
-        assert!(*vector::borrow(&data, 4) == 5, 0);
-    }
-
-    #[test]
-    fun test_sort_by_cmp(){
-        let data = vector<u64>[1, 3, 2, 5, 4, 1];
-        sort_by_cmp(&mut data, |a,b|{*a > *b});
-        assert!(vector::length<u64>(&data) == 6, 0);
-        assert!(*vector::borrow(&data, 0) == 1, 0);
-        assert!(*vector::borrow(&data, 1) == 1, 0);
-        assert!(*vector::borrow(&data, 2) == 2, 0);
-        assert!(*vector::borrow(&data, 3) == 3, 0);
-        assert!(*vector::borrow(&data, 4) == 4, 0);
-        assert!(*vector::borrow(&data, 5) == 5, 0);
-    }
 }
\ No newline at end of file
diff --git a/frameworks/moveos-stdlib/tests/sort_tests.move b/frameworks/moveos-stdlib/tests/sort_tests.move
index 2d39ea98a..4498d3fa8 100644
--- a/frameworks/moveos-stdlib/tests/sort_tests.move
+++ b/frameworks/moveos-stdlib/tests/sort_tests.move
@@ -8,12 +8,49 @@ module moveos_std::sort_tests {
     use moveos_std::sort;
 
     #[test_only]
-    struct TestStruct has copy, drop{
+    struct TestStruct has drop{
         value: u64
     }
 
     #[test]
-    fun test_sort_by_cmp(){
+    fun test_quick_sort() {
+        let data = vector<u64>[1, 3, 2, 5, 4];
+        sort::quick_sort(&mut data);
+        assert!(vector::length<u64>(&data) == 5, 0);
+        assert!(*vector::borrow(&data, 0) == 1, 0);
+        assert!(*vector::borrow(&data, 1) == 2, 0);
+        assert!(*vector::borrow(&data, 2) == 3, 0);
+        assert!(*vector::borrow(&data, 3) == 4, 0);
+        assert!(*vector::borrow(&data, 4) == 5, 0);
+    }
+
+    #[test]
+    fun test_quick_sort_u128() {
+        let data = vector<u128>[1, 3, 2, 5, 4];
+        sort::quick_sort(&mut data);
+        assert!(vector::length<u128>(&data) == 5, 0);
+        assert!(*vector::borrow(&data, 0) == 1, 0);
+        assert!(*vector::borrow(&data, 1) == 2, 0);
+        assert!(*vector::borrow(&data, 2) == 3, 0);
+        assert!(*vector::borrow(&data, 3) == 4, 0);
+        assert!(*vector::borrow(&data, 4) == 5, 0);
+    }
+
+    #[test]
+    fun test_sort_by_cmp_u64(){
+        let data = vector<u64>[1, 3, 2, 5, 4, 1];
+        sort::sort_by_cmp(&mut data, |a,b|{*a > *b});
+        assert!(vector::length<u64>(&data) == 6, 0);
+        assert!(*vector::borrow(&data, 0) == 1, 0);
+        assert!(*vector::borrow(&data, 1) == 1, 0);
+        assert!(*vector::borrow(&data, 2) == 2, 0);
+        assert!(*vector::borrow(&data, 3) == 3, 0);
+        assert!(*vector::borrow(&data, 4) == 4, 0);
+        assert!(*vector::borrow(&data, 5) == 5, 0);
+    }
+
+    #[test]
+    fun test_sort_by_cmp_struct(){
         let data = vector<TestStruct>[
             TestStruct{value: 1},
             TestStruct{value: 3},
@@ -49,7 +86,7 @@ module moveos_std::sort_tests {
         ];
         sort::sort_by_key(&mut data, |a|{
             let a: &TestStruct = a;
-            a.value
+            &a.value
         }
         );
         assert!(vector::length(&data) == 6, 0);