From b718b3509781cc80592c7db35b555d3096e3a1fe Mon Sep 17 00:00:00 2001 From: Kayne Ruse Date: Sun, 26 Apr 2026 22:52:24 +1000 Subject: [PATCH] FIX: Substrings had corner-cases with incorrect results, read more I found this while writing unit tests for Toy_Function, where one (native) function was named 'identity' and another (custom) was named 'ident' to avoid a naming clash. The rename didn't resolve the clash, so after some digging, I found that strings compared to substrings would return a match, despite being different. This took some awkward corner-case handling, as it turns out 'deepCompareUtil' only returns zero when no differences have been found, not when a match has been found. I also added checks for this to Toy_String's unit test, with the parameters checked in both orders i.e. (a,b) and (b,a), because paranoia is your friend. The rope pattern is powerful, but also gives you enough rope to hang yourself. --- source/toy_attributes.h | 4 +- source/toy_print.c | 6 +- source/toy_string.c | 22 ++- tests/scripts/test_first_class_functions.toy | 4 +- tests/units/test_function.c | 142 ++++++++++++++++++- tests/units/test_string.c | 142 +++++++++++++++++++ 6 files changed, 306 insertions(+), 14 deletions(-) diff --git a/source/toy_attributes.h b/source/toy_attributes.h index 4d37d8e..29502a3 100644 --- a/source/toy_attributes.h +++ b/source/toy_attributes.h @@ -13,10 +13,10 @@ Toy_Value handleTableAttributes(Toy_VM* vm, Toy_Value compound, Toy_Value attrib // [x] array.length // [x] array.pushBack(x) // [x] array.popBack() -// [ ] array.forEach(fn) // fn(x) -> void +// [x] array.forEach(fn) // fn(x) -> void // [ ] array.sort(fn) // fn(a,b) -> int // [x] table.length // [x] table.insert(x, y) // [x] table.hasKey(x) // [x] table.remove(x) -// [ ] table.forEach(fn) // fn(x) -> void +// [ ] table.forEach(fn) // fn(x,y) -> void diff --git a/source/toy_print.c b/source/toy_print.c index c5a049b..4d02d10 100644 --- a/source/toy_print.c +++ b/source/toy_print.c @@ -3,15 +3,15 @@ #include static void outDefault(const char* msg) { - fprintf(stdout, "%s", msg); + fprintf(stdout, "%s\n", msg); } static void errDefault(const char* msg) { - fprintf(stderr, "%s", msg); + fprintf(stderr, "%s\n", msg); } static void assertDefault(const char* msg) { - fprintf(stderr, "%s", msg); + fprintf(stderr, "%s\n", msg); } static Toy_callbackType printCallback = outDefault; diff --git a/source/toy_string.c b/source/toy_string.c index 59a6dfc..cb8091f 100644 --- a/source/toy_string.c +++ b/source/toy_string.c @@ -193,13 +193,13 @@ static int deepCompareUtil(Toy_String* left, Toy_String* right, const char** lef (*rightHead)++; } - //if both are not null, then it's a real result - if ( (**leftHead == '\0' || **rightHead == '\0') == false) { + //if there's a difference, and neither is null, than a result has (probably) been found + if ((**leftHead != '\0' && **rightHead != '\0' && (**leftHead != **rightHead))) { result = *(const unsigned char*)(*leftHead) - *(const unsigned char*)(*rightHead); } } - //if either are a null character, return 0 to check the next node + //returning 0 means no difference found yet return result; } @@ -209,11 +209,25 @@ int Toy_compareStrings(Toy_String* left, Toy_String* right) { return left->info.length - right->info.length; } + //BUGFIX: If both args are leaves, and one is a substring of the other, then deepCompareUtil() will return a wrong result + if (left->info.type == TOY_STRING_LEAF && right->info.type == TOY_STRING_LEAF) { + unsigned int maxLength = left->info.length > right->info.length ? left->info.length : right->info.length; + return strncmp(left->leaf.data, right->leaf.data, maxLength); + } + //util pointers const char* leftHead = NULL; const char* rightHead = NULL; - return deepCompareUtil(left, right, &leftHead, &rightHead); + int result = deepCompareUtil(left, right, &leftHead, &rightHead); + + //BUGFIX: deepCompareUtil() doesn't handle substrings correctly + if (result == 0 && leftHead != NULL && rightHead != NULL) { + return (int)(*leftHead - *rightHead); + } + else { + return result; + } } static unsigned int hashCString(const char* string) { diff --git a/tests/scripts/test_first_class_functions.toy b/tests/scripts/test_first_class_functions.toy index 4471b18..3ccff2a 100644 --- a/tests/scripts/test_first_class_functions.toy +++ b/tests/scripts/test_first_class_functions.toy @@ -3,8 +3,8 @@ fn hello() { print "Hello world"; } -fn identity(x) { +fn ident(x) { return x; } -assert identity(hello) == hello, "First class function check failed"; \ No newline at end of file +assert ident(hello) == hello, "First class function check failed"; \ No newline at end of file diff --git a/tests/units/test_function.c b/tests/units/test_function.c index 303b58a..37979d4 100644 --- a/tests/units/test_function.c +++ b/tests/units/test_function.c @@ -1,9 +1,145 @@ +#include "toy_vm.h" #include "toy_console_colors.h" +#include "toy_lexer.h" +#include "toy_parser.h" +#include "toy_compiler.h" +#include "toy_print.h" + #include +#include #include -int main(void) { - printf(TOY_CC_WARN "Test not yet implemented: %s\n" TOY_CC_RESET, __FILE__); +//define a function in one bytecode, invoke it in another +const char* sourceAlpha = "\n\ +//using the classic closure approach\n\ +fn makeCounter() {\n\ + var counter: Int = 0;\n\ +\ + fn increment() {\n\ + return ++counter;\n\ + }\n\ +\n\ + return increment;\n\ +}\n\ +"; + +const char* sourceBeta = "\n\ +var tally = makeCounter();\n\ +\n\ +while (true) {\n\ + var result = tally();\n\ +\n\ + print result;\n\ +\n\ + if (result >= 10) {\n\ + return result;\n\ + }\n\ +}\n\ +"; + +//utils +unsigned char* makeCodeFromSource(Toy_Bucket** bucketHandle, const char* source) { + Toy_Lexer lexer; + Toy_bindLexer(&lexer, source); + + Toy_Parser parser; + Toy_bindParser(&parser, &lexer); + + Toy_Ast* ast = Toy_scanParser(bucketHandle, &parser); + return Toy_compileToBytecode(ast); +} + +//tests +int test_functions_from_bytecodes(void) { + //do the thing + { + //setup + Toy_Bucket* bucket = Toy_allocateBucket(TOY_BUCKET_IDEAL); + Toy_String* makeCounterString = Toy_createStringLength(&bucket, "makeCounter", 11); + + Toy_VM vm; + Toy_initVM(&vm); + + //run alpha + unsigned char* alpha = makeCodeFromSource(&bucket, sourceAlpha); + Toy_bindVM(&vm, alpha, NULL); + Toy_runVM(&vm); + + //check for the function was declared + if (Toy_isDeclaredScope(vm.scope, makeCounterString) != true) { + fprintf(stderr, TOY_CC_ERROR "ERROR: Failed to create the function '%s' from source\n" TOY_CC_RESET, makeCounterString->leaf.data); + Toy_freeString(makeCounterString); + Toy_freeVM(&vm); + Toy_freeBucket(&bucket); + free(alpha); + return -1; + } + + //get the function and clean up + Toy_Value fnValue = Toy_copyValue(&bucket, *Toy_accessScopeAsPointer(vm.scope, makeCounterString)); + Toy_resetVM(&vm, false, true); + + //run beta + unsigned char* beta = makeCodeFromSource(&bucket, sourceBeta); + Toy_bindVM(&vm, beta, NULL); + Toy_declareScope(vm.scope, makeCounterString, TOY_VALUE_ANY, fnValue, true); + Toy_runVM(&vm); + + //examine the results + if (vm.stack->count != 1 || + TOY_VALUE_IS_INTEGER(vm.stack->data[0]) != true || + TOY_VALUE_AS_INTEGER(vm.stack->data[0]) != 10 + ) + { + fprintf(stderr, TOY_CC_ERROR "ERROR: Unexpected result found in function results\n" TOY_CC_RESET); + Toy_freeString(makeCounterString); + Toy_freeVM(&vm); + Toy_freeBucket(&bucket); + free(alpha); + free(beta); + return -1; + } + + Toy_resetVM(&vm, false, true); + + //cleanup + Toy_freeValue(fnValue); + Toy_freeString(makeCounterString); + Toy_freeVM(&vm); + Toy_freeBucket(&bucket); + free(alpha); + free(beta); + } + + //all good return 0; -} \ No newline at end of file +} + +int test_functions_from_callbacks(void) { + printf(TOY_CC_WARN "WIP test not yet implemented: %s\n" TOY_CC_RESET, __FILE__); + return 0; +} + +int main(void) { + //run each test set, returning the total errors given + int total = 0, res = 0; + + { + res = test_functions_from_bytecodes(); + if (res == 0) { + printf(TOY_CC_NOTICE "All good\n" TOY_CC_RESET); + } + total += res; + } + + { + res = test_functions_from_callbacks(); + if (res == 0) { + printf(TOY_CC_NOTICE "All good\n" TOY_CC_RESET); + } + total += res; + } + + return total; +} diff --git a/tests/units/test_string.c b/tests/units/test_string.c index 710959c..0bb587c 100644 --- a/tests/units/test_string.c +++ b/tests/units/test_string.c @@ -619,6 +619,148 @@ int test_string_equality(void) { Toy_freeBucket(&bucket); } + //substring non-equality (no concats) + { + //setup + Toy_Bucket* bucket = Toy_allocateBucket(1024); + Toy_String* left = Toy_createStringLength(&bucket, "identity", 8); + Toy_String* right = Toy_createStringLength(&bucket, "ident", 5); + + int result = 0; //for print the errors + + //check mismatch + if ((result = Toy_compareStrings(left, right)) == 0) + { + char* leftBuffer = Toy_getStringRaw(left); + char* rightBuffer = Toy_getStringRaw(right); + fprintf(stderr, TOY_CC_ERROR "ERROR: String equality '%s' != '%s' is incorrect, found %s\n" TOY_CC_RESET, leftBuffer, rightBuffer, result < 0 ? "<" : result == 0 ? "==" : ">"); + free(leftBuffer); + free(rightBuffer); + Toy_freeBucket(&bucket); + return -1; + } + + //cleanup + Toy_freeBucket(&bucket); + } + + //substring non-equality (with matching and non-matching concats) + { + //setup + Toy_Bucket* bucket = Toy_allocateBucket(1024); + Toy_String* identity = Toy_createStringLength(&bucket, "identity", 8); + Toy_String* ident = Toy_createStringLength(&bucket, "ident", 5); + + Toy_String* matchingIdentity = Toy_concatStrings(&bucket, + Toy_createStringLength(&bucket, "ident", 5), + Toy_createStringLength(&bucket, "ity", 3) + ); + + Toy_String* stolenIdentity = Toy_concatStrings(&bucket, + Toy_createStringLength(&bucket, "id", 2), + Toy_createStringLength(&bucket, "entity", 6) + ); + + int result = 0; //for print the errors + + //ensure the concats match the base + if ((result = Toy_compareStrings(identity, matchingIdentity)) != 0) + { + char* leftBuffer = Toy_getStringRaw(identity); + char* rightBuffer = Toy_getStringRaw(matchingIdentity); + fprintf(stderr, TOY_CC_ERROR "ERROR: Substring concat non-equality failed early on line %d with '%s' and '%s' is incorrect, found %s\n" TOY_CC_RESET, __LINE__, leftBuffer, rightBuffer, result < 0 ? "<" : result == 0 ? "==" : ">"); + free(leftBuffer); + free(rightBuffer); + Toy_freeBucket(&bucket); + return -1; + } + + if ((result = Toy_compareStrings(identity, stolenIdentity)) != 0) + { + char* leftBuffer = Toy_getStringRaw(identity); + char* rightBuffer = Toy_getStringRaw(stolenIdentity); + fprintf(stderr, TOY_CC_ERROR "ERROR: Substring concat non-equality failed early on line %d with '%s' and '%s' is incorrect, found %s\n" TOY_CC_RESET, __LINE__, leftBuffer, rightBuffer, result < 0 ? "<" : result == 0 ? "==" : ">"); + free(leftBuffer); + free(rightBuffer); + Toy_freeBucket(&bucket); + return -1; + } + + //ensure both concats are a mismatch for 'ident' + if ((result = Toy_compareStrings(ident, matchingIdentity)) == 0) + { + char* leftBuffer = Toy_getStringRaw(ident); + char* rightBuffer = Toy_getStringRaw(matchingIdentity); + fprintf(stderr, TOY_CC_ERROR "ERROR: Substring concat non-equality failed on line %d with '%s' and '%s' is incorrect, found %s\n" TOY_CC_RESET, __LINE__, leftBuffer, rightBuffer, result < 0 ? "<" : result == 0 ? "==" : ">"); + free(leftBuffer); + free(rightBuffer); + Toy_freeBucket(&bucket); + return -1; + } + + if ((result = Toy_compareStrings(ident, stolenIdentity)) == 0) + { + char* leftBuffer = Toy_getStringRaw(ident); + char* rightBuffer = Toy_getStringRaw(stolenIdentity); + fprintf(stderr, TOY_CC_ERROR "ERROR: Substring concat non-equality failed on line %d with '%s' and '%s' is incorrect, found %s\n" TOY_CC_RESET, __LINE__, leftBuffer, rightBuffer, result < 0 ? "<" : result == 0 ? "==" : ">"); + free(leftBuffer); + free(rightBuffer); + Toy_freeBucket(&bucket); + return -1; + } + + //repeat these tests with the parameters swapped, just to be safe + if ((result = Toy_compareStrings(matchingIdentity, identity)) != 0) + { + char* leftBuffer = Toy_getStringRaw(matchingIdentity); + char* rightBuffer = Toy_getStringRaw(identity); + fprintf(stderr, TOY_CC_ERROR "ERROR: Substring concat non-equality failed early on line %d with '%s' and '%s' is incorrect, found %s\n" TOY_CC_RESET, __LINE__, leftBuffer, rightBuffer, result < 0 ? "<" : result == 0 ? "==" : ">"); + free(leftBuffer); + free(rightBuffer); + Toy_freeBucket(&bucket); + return -1; + } + + if ((result = Toy_compareStrings(stolenIdentity, identity)) != 0) + { + char* leftBuffer = Toy_getStringRaw(stolenIdentity); + char* rightBuffer = Toy_getStringRaw(identity); + fprintf(stderr, TOY_CC_ERROR "ERROR: Substring concat non-equality failed early on line %d with '%s' and '%s' is incorrect, found %s\n" TOY_CC_RESET, __LINE__, leftBuffer, rightBuffer, result < 0 ? "<" : result == 0 ? "==" : ">"); + free(leftBuffer); + free(rightBuffer); + Toy_freeBucket(&bucket); + return -1; + } + + if ((result = Toy_compareStrings(matchingIdentity, ident)) == 0) + { + char* leftBuffer = Toy_getStringRaw(matchingIdentity); + char* rightBuffer = Toy_getStringRaw(ident); + fprintf(stderr, TOY_CC_ERROR "ERROR: Substring concat non-equality failed on line %d with '%s' and '%s' is incorrect, found %s\n" TOY_CC_RESET, __LINE__, leftBuffer, rightBuffer, result < 0 ? "<" : result == 0 ? "==" : ">"); + free(leftBuffer); + free(rightBuffer); + Toy_freeBucket(&bucket); + return -1; + } + + if ((result = Toy_compareStrings(stolenIdentity, ident)) == 0) + { + char* leftBuffer = Toy_getStringRaw(stolenIdentity); + char* rightBuffer = Toy_getStringRaw(ident); + fprintf(stderr, TOY_CC_ERROR "ERROR: Substring concat non-equality failed on line %d with '%s' and '%s' is incorrect, found %s\n" TOY_CC_RESET, __LINE__, leftBuffer, rightBuffer, result < 0 ? "<" : result == 0 ? "==" : ">"); + free(leftBuffer); + free(rightBuffer); + Toy_freeBucket(&bucket); + return -1; + } + + + + //cleanup + Toy_freeBucket(&bucket); + } + + return 0; }