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

@@ -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

source/toy_print.c

@@ -3,15 +3,15 @@
 #include <stdio.h>
 
 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;

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 there's a difference, and neither is null, than a result has (probably) been found
-		if ( (**leftHead == '\0' || **rightHead == '\0') == false) {
+		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) {

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";
+assert ident(hello) == hello, "First class function check failed";

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 <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 
-int main(void) {
+//define a function in one bytecode, invoke it in another
-	printf(TOY_CC_WARN "Test not yet implemented: %s\n" TOY_CC_RESET, __FILE__);
+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;
 }
+
+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;
+}

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;
 }