From 3666eae0132af365fc9612a1dbf5ae9fe3f9006d Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Wed, 3 Aug 2016 22:42:31 -0700 Subject: [PATCH] Tweak new list constructors. - Remove List.new(_). I was convinced by the issue discussion that using it is probably a bad idea. We don't want to encourage more nulls in the world than there are already are. So let's see if we can live without it and just have List.filled(). That way users think about what they're creating a list *of*. - Added some more tests. - Correctly handle being given a negative size. --- src/vm/wren_core.c | 55 ++++++++---------------- test/core/list/filled.wren | 8 ++++ test/core/list/filled_size_negative.wren | 1 + test/core/list/filled_size_not_int.wren | 1 + test/core/list/filled_size_not_num.wren | 1 + test/core/list/new_extra_args.wren | 8 ---- 6 files changed, 29 insertions(+), 45 deletions(-) create mode 100644 test/core/list/filled.wren create mode 100644 test/core/list/filled_size_negative.wren create mode 100644 test/core/list/filled_size_not_int.wren create mode 100644 test/core/list/filled_size_not_num.wren delete mode 100644 test/core/list/new_extra_args.wren diff --git a/src/vm/wren_core.c b/src/vm/wren_core.c index 28f3cafc..cf4c0eee 100644 --- a/src/vm/wren_core.c +++ b/src/vm/wren_core.c @@ -234,46 +234,28 @@ DEF_PRIMITIVE(fn_toString) RETURN_VAL(CONST_STRING(vm, "")); } +// Creates a new list of size args[1], with all elements initialized to args[2]. +DEF_PRIMITIVE(list_filled) +{ + if (!validateInt(vm, args[1], "Size")) return false; + if (AS_NUM(args[1]) < 0) RETURN_ERROR("Size cannot be negative."); + + uint32_t size = (uint32_t)AS_NUM(args[1]); + ObjList* list = wrenNewList(vm, size); + + for (uint32_t i = 0; i < size; i++) + { + list->elements.data[i] = args[2]; + } + + RETURN_OBJ(list); +} + DEF_PRIMITIVE(list_new) { RETURN_OBJ(wrenNewList(vm, 0)); } -//creates a new list of size args[1], with all elements initilized to null -DEF_PRIMITIVE(list_newWithSize) -{ - if(!validateInt(vm, args[1], "Size")) return false; - - uint32_t size = (uint32_t)AS_NUM(args[1]); - ObjList* list = wrenNewList(vm, size); - - for(uint32_t i=0; i < size; i++) - { - list->elements.data[i] = NULL_VAL; - } - - RETURN_OBJ(list); -} - - -//creates a new list of size args[1], with all elements initilized to args[2] -DEF_PRIMITIVE(list_newWithSizeDefault) -{ - if(!validateInt(vm, args[1], "Size")) return false; - - uint32_t size = (uint32_t)AS_NUM(args[1]); - - ObjList* list = wrenNewList(vm, size); - - for(uint32_t i=0; i < size; i++) - { - list->elements.data[i] = args[2]; - } - - RETURN_OBJ(list); -} - - DEF_PRIMITIVE(list_add) { wrenValueBufferWrite(vm, &AS_LIST(args[0])->elements, args[1]); @@ -1308,9 +1290,8 @@ void wrenInitializeCore(WrenVM* vm) PRIMITIVE(vm->stringClass, "toString", string_toString); vm->listClass = AS_CLASS(wrenFindVariable(vm, coreModule, "List")); + PRIMITIVE(vm->listClass->obj.classObj, "filled(_,_)", list_filled); PRIMITIVE(vm->listClass->obj.classObj, "new()", list_new); - PRIMITIVE(vm->listClass->obj.classObj, "new(_)", list_newWithSize); - PRIMITIVE(vm->listClass->obj.classObj, "filled(_,_)", list_newWithSizeDefault); PRIMITIVE(vm->listClass, "[_]", list_subscript); PRIMITIVE(vm->listClass, "[_]=(_)", list_subscriptSetter); PRIMITIVE(vm->listClass, "add(_)", list_add); diff --git a/test/core/list/filled.wren b/test/core/list/filled.wren new file mode 100644 index 00000000..e8e5cdda --- /dev/null +++ b/test/core/list/filled.wren @@ -0,0 +1,8 @@ +var list = List.filled(3, "value") +System.print(list.count) // expect: 3 +System.print(list) // expect: [value, value, value] + +// Can create an empty list. +list = List.filled(0, "value") +System.print(list.count) // expect: 0 +System.print(list) // expect: [] diff --git a/test/core/list/filled_size_negative.wren b/test/core/list/filled_size_negative.wren new file mode 100644 index 00000000..acdeb38a --- /dev/null +++ b/test/core/list/filled_size_negative.wren @@ -0,0 +1 @@ +List.filled(-1, null) // expect runtime error: Size cannot be negative. diff --git a/test/core/list/filled_size_not_int.wren b/test/core/list/filled_size_not_int.wren new file mode 100644 index 00000000..989e7953 --- /dev/null +++ b/test/core/list/filled_size_not_int.wren @@ -0,0 +1 @@ +List.filled(1.2, null) // expect runtime error: Size must be an integer. diff --git a/test/core/list/filled_size_not_num.wren b/test/core/list/filled_size_not_num.wren new file mode 100644 index 00000000..93fd4534 --- /dev/null +++ b/test/core/list/filled_size_not_num.wren @@ -0,0 +1 @@ +List.filled("not num", null) // expect runtime error: Size must be a number. diff --git a/test/core/list/new_extra_args.wren b/test/core/list/new_extra_args.wren deleted file mode 100644 index 1fc236e9..00000000 --- a/test/core/list/new_extra_args.wren +++ /dev/null @@ -1,8 +0,0 @@ -var list = List.new(5) - -System.print(list.count) // expect: 5 -System.print(list) // expect: [null, null, null, null, null] - -var list2 = List.filled(5, 2) -System.print(list2.count) // expect: 5 -System.print(list2) // expect: [2, 2, 2, 2, 2]