overloads array constructors to accept another array.#95
overloads array constructors to accept another array.#95chibash merged 4 commits intocsg-tokyo:mainfrom
Conversation
The nullable parameter was previously ignored and always treated as true.
There was a problem hiding this comment.
Pull request overview
This PR adds overloaded array constructors that accept another array as input, enabling more flexible array initialization patterns. This is particularly useful when working with variables of type any that may contain arrays.
Key changes:
- Added C runtime functions (
gc_copy_intarray,gc_copy_floatarray,gc_copy_bytearray,gc_copy_array) to handle array-to-array copying with type conversion - Extended type checker to support overloaded constructor signatures accepting
Integer,T[], orany[]parameters - Updated code generator to emit appropriate runtime calls based on argument types
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| website/docs/reference/language/built-in-objects.md | Documents the new array constructor overloads with examples |
| microcontroller/core/include/c-runtime.h | Adds function declarations for new copy functions |
| microcontroller/core/src/c-runtime.c | Implements gc_copy_* functions for creating typed arrays from other arrays with type conversion |
| microcontroller/core/test/c-runtime-test.c | Adds comprehensive test coverage for array copy operations including edge cases and error handling |
| lang/src/transpiler/type-checker.ts | Extends type checking for array constructors to handle overloaded signatures with array arguments |
| lang/src/transpiler/code-generator/code-generator.ts | Updates code generation to handle array initialization from arrays |
| lang/src/transpiler/code-generator/c-runtime.ts | Adds arrayFromArray helper to select appropriate runtime function |
| lang/tests/transpiler/code-generator/code-generator2.test.ts | Adds integration tests for array initialization with arrays including error cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
microcontroller/core/src/c-runtime.c
Outdated
| if (is_int_value(src)) | ||
| return gc_new_intarray(value_to_int(src), 0); | ||
| else if (gc_is_instance_of(&intarray_object.clazz, src)) { | ||
| pointer_t srcp = value_to_ptr(src); | ||
| int32_t n = srcp->body[0]; | ||
| destp = gc_new_intarray_base(n); | ||
| for (int32_t i = 1; i <= n; i++) | ||
| destp->body[i] = srcp->body[i]; | ||
| } | ||
| else { | ||
| int32_t n = gc_array_length(safe_value_to_anyarray(false, src)); | ||
| destp = gc_new_intarray_base(n); | ||
| for (int32_t i = 0; i < n; i++) | ||
| destp->body[i + 1] = safe_value_to_int(*gc_array_get(src, i)); | ||
| } | ||
|
|
||
| DELETE_ROOT_SET(rootset) | ||
| return ptr_to_value(destp); |
There was a problem hiding this comment.
Memory leak: When the function returns early at line 1058 (for the integer case), the ROOT_SET allocated at line 1054 is never cleaned up with DELETE_ROOT_SET. The early return bypasses the DELETE_ROOT_SET call at line 1073. This should be fixed by calling DELETE_ROOT_SET before the return, or restructuring the code to ensure cleanup happens in all paths.
| if (is_int_value(src)) | |
| return gc_new_intarray(value_to_int(src), 0); | |
| else if (gc_is_instance_of(&intarray_object.clazz, src)) { | |
| pointer_t srcp = value_to_ptr(src); | |
| int32_t n = srcp->body[0]; | |
| destp = gc_new_intarray_base(n); | |
| for (int32_t i = 1; i <= n; i++) | |
| destp->body[i] = srcp->body[i]; | |
| } | |
| else { | |
| int32_t n = gc_array_length(safe_value_to_anyarray(false, src)); | |
| destp = gc_new_intarray_base(n); | |
| for (int32_t i = 0; i < n; i++) | |
| destp->body[i + 1] = safe_value_to_int(*gc_array_get(src, i)); | |
| } | |
| DELETE_ROOT_SET(rootset) | |
| return ptr_to_value(destp); | |
| value_t result; | |
| if (is_int_value(src)) { | |
| result = gc_new_intarray(value_to_int(src), 0); | |
| goto cleanup; | |
| } | |
| else if (gc_is_instance_of(&intarray_object.clazz, src)) { | |
| pointer_t srcp = value_to_ptr(src); | |
| int32_t n = srcp->body[0]; | |
| destp = gc_new_intarray_base(n); | |
| for (int32_t i = 1; i <= n; i++) | |
| destp->body[i] = srcp->body[i]; | |
| result = ptr_to_value(destp); | |
| } | |
| else { | |
| int32_t n = gc_array_length(safe_value_to_anyarray(false, src)); | |
| destp = gc_new_intarray_base(n); | |
| for (int32_t i = 0; i < n; i++) | |
| destp->body[i + 1] = safe_value_to_int(*gc_array_get(src, i)); | |
| result = ptr_to_value(destp); | |
| } | |
| cleanup: | |
| DELETE_ROOT_SET(rootset) | |
| return result; |
microcontroller/core/src/c-runtime.c
Outdated
| if (is_int_value(src)) | ||
| return gc_new_bytearray(is_boolean, value_to_int(src), 0); |
There was a problem hiding this comment.
Memory leak: When the function returns early at line 1252 (for the integer case), the ROOT_SET allocated at line 1248 is never cleaned up with DELETE_ROOT_SET. The early return bypasses the DELETE_ROOT_SET call at line 1275. This should be fixed by calling DELETE_ROOT_SET before the return, or restructuring the code to ensure cleanup happens in all paths.
| if (is_int_value(src)) | |
| return gc_new_bytearray(is_boolean, value_to_int(src), 0); | |
| if (is_int_value(src)) { | |
| value_t result = gc_new_bytearray(is_boolean, value_to_int(src), 0); | |
| DELETE_ROOT_SET(rootset) | |
| return result; | |
| } |
| } | ||
|
|
||
| value_t gc_copy_intarray(value_t src) { | ||
| ROOT_SET(rootset, 1) |
There was a problem hiding this comment.
Missing semicolon after ROOT_SET macro invocation. The ROOT_SET macro should be followed by a semicolon for consistency with C coding style, even though it may work without it depending on the macro definition.
| } | ||
|
|
||
| value_t gc_copy_bytearray(bool is_boolean, value_t src) { | ||
| ROOT_SET(rootset, 1) |
There was a problem hiding this comment.
Missing semicolon after ROOT_SET macro invocation. The ROOT_SET macro should be followed by a semicolon for consistency with C coding style.
lang/src/transpiler/type-checker.ts
Outdated
| private callExpressionArgOrArray(arg: AST.Node, paramType: StaticType, names: NameTable<Info>) { | ||
| this.visit(arg, names) | ||
| const argType = this.result | ||
| if (isConsistent(argType, paramType) || this.isConsistentOnFirstPass(argType, paramType) | ||
| || isSubtype(argType, paramType) || argType instanceof ArrayType) { | ||
| this.addStaticType(arg, argType) | ||
| return | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation: The closing brace and return statement have incorrect indentation. They should be aligned at the function body level, not indented an extra level.
| private callExpressionArgOrArray(arg: AST.Node, paramType: StaticType, names: NameTable<Info>) { | |
| this.visit(arg, names) | |
| const argType = this.result | |
| if (isConsistent(argType, paramType) || this.isConsistentOnFirstPass(argType, paramType) | |
| || isSubtype(argType, paramType) || argType instanceof ArrayType) { | |
| this.addStaticType(arg, argType) | |
| return | |
| } | |
| private callExpressionArgOrArray(arg: AST.Node, paramType: StaticType, names: NameTable<Info>) { | |
| this.visit(arg, names) | |
| const argType = this.result | |
| if (isConsistent(argType, paramType) || this.isConsistentOnFirstPass(argType, paramType) | |
| || isSubtype(argType, paramType) || argType instanceof ArrayType) { | |
| this.addStaticType(arg, argType) | |
| return | |
| } |
| for (const t of paramTypes) | ||
| if (isConsistent(argType, t) || this.isConsistentOnFirstPass(argType, t) || isSubtype(argType, t)) { | ||
| this.addStaticType(arg, argType) | ||
| return | ||
| } |
There was a problem hiding this comment.
Missing coercion handling: Unlike callExpressionArg which calls addCoercion at line 1188, this method only calls addStaticType without considering if coercion is needed. When a type is consistent but requires coercion (e.g., converting Any to a specific type), this might not be handled correctly. Consider calling addCoercion similar to how it's done in the original callExpressionArg method.
lang/src/transpiler/type-checker.ts
Outdated
| if (isConsistent(argType, paramType) || this.isConsistentOnFirstPass(argType, paramType) | ||
| || isSubtype(argType, paramType) || argType instanceof ArrayType) { | ||
| this.addStaticType(arg, argType) | ||
| return | ||
| } |
There was a problem hiding this comment.
Missing coercion handling: Similar to the issue in callExpressionOLArg, this method only calls addStaticType without considering if coercion is needed. When a type is consistent but requires coercion, this should be handled. Consider calling addCoercion as done in the original callExpressionArg method at line 1188.
| if (isConsistent(argType, paramType) || this.isConsistentOnFirstPass(argType, paramType) | |
| || isSubtype(argType, paramType) || argType instanceof ArrayType) { | |
| this.addStaticType(arg, argType) | |
| return | |
| } | |
| if (isConsistent(argType, paramType) || this.isConsistentOnFirstPass(argType, paramType)) { | |
| this.addCoercion(arg, argType) | |
| return | |
| } | |
| if (isSubtype(argType, paramType) || argType instanceof ArrayType) { | |
| this.addStaticType(arg, argType) | |
| return | |
| } |
| ROOT_SET(rootset, 2); | ||
| // Test 1: Copy from integer value (creates new array with specified size) - boolean array | ||
| value_t arr1 = gc_copy_bytearray(true, int_to_value(5)); | ||
| rootset.values[0] = arr1; | ||
| Assert_equals(gc_bytearray_length(arr1), 5); | ||
| Assert_true(gc_is_boolarray(arr1)); | ||
|
|
||
| // Test 2: Copy from integer value - byte array (Uint8Array) | ||
| value_t arr2 = gc_copy_bytearray(false, int_to_value(5)); | ||
| rootset.values[0] = arr2; | ||
| Assert_equals(gc_bytearray_length(arr2), 5); | ||
| Assert_true(!gc_is_boolarray(arr2)); | ||
|
|
||
| // Test 3: Copy from existing bytearray | ||
| value_t src_bool = gc_make_bytearray(true, 4, 1, 0, 1, 1); | ||
| rootset.values[0] = src_bool; | ||
| Assert_equals(gc_bytearray_length(src_bool), 4); | ||
| value_t arr3 = gc_copy_bytearray(true, src_bool); | ||
| rootset.values[1] = arr3; |
There was a problem hiding this comment.
Inconsistent naming: this test function uses rootset as the ROOT_SET name, while all other test functions in this file use root_set (with an underscore). For consistency, this should be root_set.
| ROOT_SET(rootset, 2); | |
| // Test 1: Copy from integer value (creates new array with specified size) - boolean array | |
| value_t arr1 = gc_copy_bytearray(true, int_to_value(5)); | |
| rootset.values[0] = arr1; | |
| Assert_equals(gc_bytearray_length(arr1), 5); | |
| Assert_true(gc_is_boolarray(arr1)); | |
| // Test 2: Copy from integer value - byte array (Uint8Array) | |
| value_t arr2 = gc_copy_bytearray(false, int_to_value(5)); | |
| rootset.values[0] = arr2; | |
| Assert_equals(gc_bytearray_length(arr2), 5); | |
| Assert_true(!gc_is_boolarray(arr2)); | |
| // Test 3: Copy from existing bytearray | |
| value_t src_bool = gc_make_bytearray(true, 4, 1, 0, 1, 1); | |
| rootset.values[0] = src_bool; | |
| Assert_equals(gc_bytearray_length(src_bool), 4); | |
| value_t arr3 = gc_copy_bytearray(true, src_bool); | |
| rootset.values[1] = arr3; | |
| ROOT_SET(root_set, 2); | |
| // Test 1: Copy from integer value (creates new array with specified size) - boolean array | |
| value_t arr1 = gc_copy_bytearray(true, int_to_value(5)); | |
| root_set.values[0] = arr1; | |
| Assert_equals(gc_bytearray_length(arr1), 5); | |
| Assert_true(gc_is_boolarray(arr1)); | |
| // Test 2: Copy from integer value - byte array (Uint8Array) | |
| value_t arr2 = gc_copy_bytearray(false, int_to_value(5)); | |
| root_set.values[0] = arr2; | |
| Assert_equals(gc_bytearray_length(arr2), 5); | |
| Assert_true(!gc_is_boolarray(arr2)); | |
| // Test 3: Copy from existing bytearray | |
| value_t src_bool = gc_make_bytearray(true, 4, 1, 0, 1, 1); | |
| root_set.values[0] = src_bool; | |
| Assert_equals(gc_bytearray_length(src_bool), 4); | |
| value_t arr3 = gc_copy_bytearray(true, src_bool); | |
| root_set.values[1] = arr3; |
| pointer_t destp; | ||
| if (is_int_value(src)) | ||
| return gc_new_floatarray(value_to_int(src), 0); | ||
| else if (gc_is_instance_of(&floatarray_object.clazz, src)) { | ||
| pointer_t srcp = value_to_ptr(src); | ||
| int32_t n = srcp->body[0]; | ||
| destp = gc_new_floatarray_base(n); | ||
| for (int32_t i = 1; i <= n; i++) | ||
| destp->body[i] = srcp->body[i]; | ||
| } | ||
| else { | ||
| int32_t n = gc_array_length(safe_value_to_anyarray(false, src)); | ||
| destp = gc_new_floatarray_base(n); | ||
| for (int32_t i = 0; i < n; i++) | ||
| *(float*)&destp->body[i + 1] = safe_value_to_float(*gc_array_get(src, i)); | ||
| } | ||
|
|
||
| DELETE_ROOT_SET(rootset) | ||
| return ptr_to_value(destp); |
There was a problem hiding this comment.
Memory leak: When the function returns early at line 1146 (for the integer case), the ROOT_SET allocated at line 1142 is never cleaned up with DELETE_ROOT_SET. The early return bypasses the DELETE_ROOT_SET call at line 1161. This should be fixed by calling DELETE_ROOT_SET before the return, or restructuring the code to ensure cleanup happens in all paths.
| pointer_t destp; | |
| if (is_int_value(src)) | |
| return gc_new_floatarray(value_to_int(src), 0); | |
| else if (gc_is_instance_of(&floatarray_object.clazz, src)) { | |
| pointer_t srcp = value_to_ptr(src); | |
| int32_t n = srcp->body[0]; | |
| destp = gc_new_floatarray_base(n); | |
| for (int32_t i = 1; i <= n; i++) | |
| destp->body[i] = srcp->body[i]; | |
| } | |
| else { | |
| int32_t n = gc_array_length(safe_value_to_anyarray(false, src)); | |
| destp = gc_new_floatarray_base(n); | |
| for (int32_t i = 0; i < n; i++) | |
| *(float*)&destp->body[i + 1] = safe_value_to_float(*gc_array_get(src, i)); | |
| } | |
| DELETE_ROOT_SET(rootset) | |
| return ptr_to_value(destp); | |
| pointer_t destp = NULL; | |
| value_t result; | |
| if (is_int_value(src)) { | |
| result = gc_new_floatarray(value_to_int(src), 0); | |
| } | |
| else if (gc_is_instance_of(&floatarray_object.clazz, src)) { | |
| pointer_t srcp = value_to_ptr(src); | |
| int32_t n = srcp->body[0]; | |
| destp = gc_new_floatarray_base(n); | |
| for (int32_t i = 1; i <= n; i++) | |
| destp->body[i] = srcp->body[i]; | |
| result = ptr_to_value(destp); | |
| } | |
| else { | |
| int32_t n = gc_array_length(safe_value_to_anyarray(false, src)); | |
| destp = gc_new_floatarray_base(n); | |
| for (int32_t i = 0; i < n; i++) | |
| *(float*)&destp->body[i + 1] = safe_value_to_float(*gc_array_get(src, i)); | |
| result = ptr_to_value(destp); | |
| } | |
| DELETE_ROOT_SET(rootset) | |
| return result; |
| destp->body[i + 1] = safe_value_to_int(*gc_array_get(src, i)); | ||
| } | ||
|
|
||
| DELETE_ROOT_SET(rootset) |
There was a problem hiding this comment.
Missing semicolon after DELETE_ROOT_SET macro invocation. The DELETE_ROOT_SET macro should be followed by a semicolon for consistency with C coding style.
| DELETE_ROOT_SET(rootset) | |
| DELETE_ROOT_SET(rootset); |
No description provided.