Skip to content

Commit

Permalink
[fixup] Addressing reviewer comments, renaming Table::insert to Table…
Browse files Browse the repository at this point in the history
…::add
  • Loading branch information
wrwg committed Mar 29, 2022
1 parent 512ccba commit de66cb7
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 29 deletions.
15 changes: 8 additions & 7 deletions language/extensions/move-table-extension/sources/Table.move
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/// Type of large-scale storage tables.
module Extensions::Table {
/// Type of tables
struct Table<phantom K, phantom V> has store {
struct Table<phantom K, phantom V: store> has store {
handle: u128
}

Expand All @@ -16,10 +16,11 @@ module Extensions::Table {
drop_unchecked_box<K, V, Box<V>>(table)
}

/// Insert the pair (`key`, `val`) to `table`.
/// Aborts if there is already an entry for `key`.
public fun insert<K, V>(table: &mut Table<K, V>, key: &K, val: V) {
insert_box<K, V, Box<V>>(table, key, Box{val})
/// Add a new entry to the table. Aborts if an entry for this
/// key already exists. The entry itself is not stored in the
/// table, and cannot be discovered from it.
public fun add<K, V>(table: &mut Table<K, V>, key: &K, val: V) {
add_box<K, V, Box<V>>(table, key, Box{val})
}

/// Acquire an immutable reference to the value which `key` maps to.
Expand Down Expand Up @@ -48,7 +49,7 @@ module Extensions::Table {
/// Insert the pair (`key`, `default`) first if there is no entry for `key`.
public fun borrow_mut_with_default<K, V: drop>(table: &mut Table<K, V>, key: &K, default: V): &mut V {
if (!contains(table, key)) {
insert(table, key, default)
add(table, key, default)
};
borrow_mut(table, key)
}
Expand Down Expand Up @@ -82,7 +83,7 @@ module Extensions::Table {
// Primitives which take as an additional type parameter `Box<V>`, so the implementation
// can use this to determine serialization layout.
native fun new_table_handle(): u128;
native fun insert_box<K, V, B>(table: &mut Table<K, V>, key: &K, val: Box<V>);
native fun add_box<K, V, B>(table: &mut Table<K, V>, key: &K, val: Box<V>);
native fun borrow_box<K, V, B>(table: &Table<K, V>, key: &K): &Box<V>;
native fun borrow_box_mut<K, V, B>(table: &mut Table<K, V>, key: &K): &mut Box<V>;
native fun length_box<K, V, B>(table: &Table<K, V>): u64;
Expand Down
6 changes: 3 additions & 3 deletions language/extensions/move-table-extension/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ pub fn table_natives(table_addr: AccountAddress) -> NativeFunctionTable {
table_addr,
&[
("Table", "new_table_handle", native_new_table_handle),
("Table", "insert_box", native_insert_box),
("Table", "add_box", native_add_box),
("Table", "length_box", native_length_box),
("Table", "borrow_box", native_borrow_box),
("Table", "borrow_box_mut", native_borrow_box),
Expand Down Expand Up @@ -384,7 +384,7 @@ fn native_new_table_handle(
))
}

fn native_insert_box(
fn native_add_box(
context: &mut NativeContext,
ty_args: Vec<Type>,
mut args: VecDeque<Value>,
Expand Down Expand Up @@ -421,7 +421,7 @@ fn native_length_box(
mut args: VecDeque<Value>,
) -> PartialVMResult<NativeResult> {
assert!(ty_args.len() == 3);
assert!(args.len() == 2);
assert!(args.len() == 1);

let table_context = context.extensions().get::<NativeTableContext>();
let mut table_data = table_context.table_data.borrow_mut();
Expand Down
51 changes: 32 additions & 19 deletions language/extensions/move-table-extension/tests/TableTests.move
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ module Extensions::TableTests {
#[test]
fun simple_read_write() {
let t = T::new<u64, u64>();
T::insert(&mut t, &1, 2);
T::insert(&mut t, &10, 33);
T::add(&mut t, &1, 2);
T::add(&mut t, &10, 33);
assert!(*T::borrow(&t, &1) == 2, 1);
assert!(*T::borrow(&t, &10) == 33, 1);
T::drop_unchecked(t)
Expand All @@ -24,7 +24,7 @@ module Extensions::TableTests {
#[test]
fun simple_update() {
let t = T::new<u64, u64>();
T::insert(&mut t, &1, 2);
T::add(&mut t, &1, 2);
assert!(*T::borrow(&t, &1) == 2, 1);
*T::borrow_mut(&mut t, &1) = 3;
assert!(*T::borrow(&t, &1) == 3, 1);
Expand All @@ -34,32 +34,43 @@ module Extensions::TableTests {
#[test]
fun test_destroy() {
let t = T::new<u64, u64>();
T::insert(&mut t, &1, 2);
T::add(&mut t, &1, 2);
assert!(*T::borrow(&t, &1) == 2, 1);
T::remove(&mut t, &1);
T::destroy_empty(t)
}

#[test]
#[expected_failure(abort_code = 51155)]
fun test_destroy_fail() {
fun test_destroy_fails() {
let t = T::new<u64, u64>();
T::insert(&mut t, &1, 2);
T::add(&mut t, &1, 2);
assert!(*T::borrow(&t, &1) == 2, 1);
T::destroy_empty(t) // expected to fail
}

#[test(s = @0x42)]
#[test]
fun test_length() {
let t = T::new<u64, u64>();
T::add(&mut t, &1, 2);
T::add(&mut t, &2, 2);
assert!(T::length(&t) == 2, 1);
T::remove(&mut t, &1);
assert!(T::length(&t) == 1, 2);
T::drop_unchecked(t)
}

#[test(s = @0x42)]
fun test_primitive(s: signer) acquires S {
let t = T::new<u64, u128>();
assert!(!T::contains(&t, &42), 100);

T::insert(&mut t, &42, 1012);
T::add(&mut t, &42, 1012);
assert!(T::contains(&t, &42), 101);
assert!(!T::contains(&t, &0), 102);
assert!(*T::borrow(&t, &42) == 1012, 103);

T::insert(&mut t, &43, 1013);
T::add(&mut t, &43, 1013);
assert!(T::contains(&t, &42), 104);
assert!(!T::contains(&t, &0), 105);
assert!(T::contains(&t, &43), 106);
Expand All @@ -84,7 +95,7 @@ module Extensions::TableTests {
fun test_vector(s: signer) acquires S {
let t = T::new<u8, vector<address>>();

T::insert(&mut t, &42, Vector::singleton<address>(@0x1012));
T::add(&mut t, &42, Vector::singleton<address>(@0x1012));
assert!(T::contains(&t, &42), 101);
assert!(!T::contains(&t, &0), 102);
assert!(Vector::length(T::borrow(&t, &42)) == 1, 103);
Expand All @@ -111,15 +122,15 @@ module Extensions::TableTests {
let val_1 = 11;
let val_2 = 45;

T::insert(&mut t, &@0xAB, Balance{ value: val_1 });
T::add(&mut t, &@0xAB, Balance{ value: val_1 });
assert!(T::contains(&t, &@0xAB), 101);
assert!(*&T::borrow(&t, &@0xAB).value == val_1, 102);

move_to(&s, S { t });

let global_t = &mut borrow_global_mut<S<address, Balance>>(@0x42).t;

T::insert(global_t, &@0xCD, Balance{ value: val_2 });
T::add(global_t, &@0xCD, Balance{ value: val_2 });
assert!(*&T::borrow(global_t, &@0xAB).value == val_1, 103);
assert!(*&T::borrow(global_t, &@0xCD).value == val_2, 104);

Expand All @@ -142,22 +153,22 @@ module Extensions::TableTests {

// Create two small tables
let t1 = T::new<address, u128>();
T::insert(&mut t1, &@0xAB, val_1);
T::add(&mut t1, &@0xAB, val_1);

let t2 = T::new<address, u128>();
T::insert(&mut t2, &@0xCD, val_2);
T::add(&mut t2, &@0xCD, val_2);

// Insert two small tables into the big table
T::insert(&mut t, &@0x12, t1);
T::insert(&mut t, &@0x34, t2);
T::add(&mut t, &@0x12, t1);
T::add(&mut t, &@0x34, t2);


assert!(T::contains(T::borrow(&t, &@0x12), &@0xAB), 101);
assert!(T::contains(T::borrow(&t, &@0x34), &@0xCD), 102);
assert!(*T::borrow(T::borrow(&t, &@0x12), &@0xAB) == val_1, 103);
assert!(*T::borrow(T::borrow(&t, &@0x34), &@0xCD) == val_2, 104);

T::insert(T::borrow_mut(&mut t, &@0x12), &@0xEF, val_3);
T::add(T::borrow_mut(&mut t, &@0x12), &@0xEF, val_3);
assert!(*T::borrow(T::borrow(&t, &@0x12), &@0xEF) == val_3, 105);
assert!(*T::borrow(T::borrow(&t, &@0x12), &@0xAB) == val_1, 106);

Expand All @@ -174,9 +185,9 @@ module Extensions::TableTests {
let t = T::new<u64, u128>();
assert!(!T::contains(&t, &42), 100);

T::insert(&mut t, &42, 1012);
T::add(&mut t, &42, 1012);
assert!(T::contains(&t, &42), 101);
T::insert(&mut t, &42, 1013); // should fail here since key 42 already exists
T::add(&mut t, &42, 1013); // should fail here since key 42 already exists

move_to(&s, S { t });
}
Expand All @@ -201,4 +212,6 @@ module Extensions::TableTests {
assert!(value == 0, 101);
move_to(&s, S { t });
}


}

0 comments on commit de66cb7

Please sign in to comment.