From 7f90485cff30168944116d4fe4718dcfcd629720 Mon Sep 17 00:00:00 2001 From: Gavin Schulz Date: Sun, 4 Jan 2015 14:42:11 -0500 Subject: [PATCH 1/2] Let the user know the arity of a missing method in the error message. --- src/wren_vm.c | 57 ++++++++++++++++--- test/fiber/error.wren | 4 +- test/fiber/try.wren | 2 +- .../do_not_inherit_static_methods.wren | 2 +- test/method/not_found.wren | 2 +- test/method/not_found_eleven_arguments.wren | 3 + test/method/not_found_multiple_arguments.wren | 3 + test/method/not_found_one_argument.wren | 3 + test/method/static_method_not_found.wren | 2 +- test/super/no_superclass_method.wren | 2 +- 10 files changed, 64 insertions(+), 16 deletions(-) create mode 100644 test/method/not_found_eleven_arguments.wren create mode 100644 test/method/not_found_multiple_arguments.wren create mode 100644 test/method/not_found_one_argument.wren diff --git a/src/wren_vm.c b/src/wren_vm.c index 2a039898..f4f67952 100644 --- a/src/wren_vm.c +++ b/src/wren_vm.c @@ -24,6 +24,39 @@ static void* defaultReallocate(void* memory, size_t oldSize, size_t newSize) return realloc(memory, newSize); } +// Returns the canonical name of a symbol with the argument spaces stripped off. +char* canonicalSymbol(char* symbolName) +{ + // Find the end of the text in the given symbol name. + int pos = strlen(symbolName) - 1; + while (symbolName[pos] == ' ') pos--; + + // Create a new string that contains only the symbol name. + char *canonicalizedSymbol = malloc(pos + 1); + strncpy(canonicalizedSymbol, symbolName, pos + 1); + + return canonicalizedSymbol; +} + +// Extracts the number of arguments from a symbol name. +char* methodArgumentsString(char* symbolName) +{ + // Count the number of spaces to determine number of arguments for this + // symbol. + int pos = strlen(symbolName) - 1; + while (symbolName[pos] == ' ') pos--; + + int args = strlen(symbolName) - pos - 1; + + if (args == 1) return "1 argument"; + + int stringSize = args > 9 ? 13 : 12; + char* argumentString = malloc(stringSize); + snprintf(argumentString, stringSize, "%d arguments", args); + + return argumentString; +} + WrenVM* wrenNewVM(WrenConfiguration* configuration) { WrenReallocateFn reallocate = defaultReallocate; @@ -576,9 +609,10 @@ static bool runInterpreter(WrenVM* vm) if (symbol >= classObj->methods.count) { char message[100]; - snprintf(message, 100, "%s does not implement method '%s'.", + snprintf(message, 100, "%s does not implement method '%s' with %s.", classObj->name->value, - vm->methodNames.data[symbol]); + canonicalSymbol(vm->methodNames.data[symbol]), + methodArgumentsString(vm->methodNames.data[symbol])); RUNTIME_ERROR(message); } @@ -629,9 +663,10 @@ static bool runInterpreter(WrenVM* vm) case METHOD_NONE: { char message[100]; - snprintf(message, 100, "%s does not implement method '%s'.", - classObj->name->value, - vm->methodNames.data[symbol]); + snprintf(message, 100, "%s does not implement method '%s' with %s.", + classObj->name->value, + canonicalSymbol(vm->methodNames.data[symbol]), + methodArgumentsString(vm->methodNames.data[symbol])); RUNTIME_ERROR(message); } } @@ -680,8 +715,10 @@ static bool runInterpreter(WrenVM* vm) if (symbol >= classObj->methods.count) { char message[100]; - snprintf(message, 100, "%s does not implement method '%s'.", - classObj->name->value, vm->methodNames.data[symbol]); + snprintf(message, 100, "%s does not implement method '%s' with %s.", + classObj->name->value, + canonicalSymbol(vm->methodNames.data[symbol]), + methodArgumentsString(vm->methodNames.data[symbol])); RUNTIME_ERROR(message); } @@ -732,8 +769,10 @@ static bool runInterpreter(WrenVM* vm) case METHOD_NONE: { char message[100]; - snprintf(message, 100, "%s does not implement method '%s'.", - classObj->name->value, vm->methodNames.data[symbol]); + snprintf(message, 100, "%s does not implement method '%s' with %s.", + classObj->name->value, + canonicalSymbol(vm->methodNames.data[symbol]), + methodArgumentsString(vm->methodNames.data[symbol])); RUNTIME_ERROR(message); } } diff --git a/test/fiber/error.wren b/test/fiber/error.wren index ccd1e87b..33e2c3c7 100644 --- a/test/fiber/error.wren +++ b/test/fiber/error.wren @@ -3,5 +3,5 @@ var fiber = new Fiber { } IO.print(fiber.error) // expect: null -IO.print(fiber.try) // expect: String does not implement method 'unknown'. -IO.print(fiber.error) // expect: String does not implement method 'unknown'. +IO.print(fiber.try) // expect: String does not implement method 'unknown' with 0 arguments. +IO.print(fiber.error) // expect: String does not implement method 'unknown' with 0 arguments. diff --git a/test/fiber/try.wren b/test/fiber/try.wren index 9abfec89..b4cf68ed 100644 --- a/test/fiber/try.wren +++ b/test/fiber/try.wren @@ -6,5 +6,5 @@ var fiber = new Fiber { IO.print(fiber.try) // expect: before -// expect: Bool does not implement method 'unknownMethod'. +// expect: Bool does not implement method 'unknownMethod' with 0 arguments. IO.print("after try") // expect: after try diff --git a/test/inheritance/do_not_inherit_static_methods.wren b/test/inheritance/do_not_inherit_static_methods.wren index b19f7a21..643efad0 100644 --- a/test/inheritance/do_not_inherit_static_methods.wren +++ b/test/inheritance/do_not_inherit_static_methods.wren @@ -4,4 +4,4 @@ class Foo { class Bar is Foo {} -Bar.methodOnFoo // expect runtime error: Bar metaclass does not implement method 'methodOnFoo'. +Bar.methodOnFoo // expect runtime error: Bar metaclass does not implement method 'methodOnFoo' with 0 arguments. diff --git a/test/method/not_found.wren b/test/method/not_found.wren index 3e26a186..033a7571 100644 --- a/test/method/not_found.wren +++ b/test/method/not_found.wren @@ -1,3 +1,3 @@ class Foo {} -(new Foo).someUnknownMethod // expect runtime error: Foo does not implement method 'someUnknownMethod'. +(new Foo).someUnknownMethod // expect runtime error: Foo does not implement method 'someUnknownMethod' with 0 arguments. diff --git a/test/method/not_found_eleven_arguments.wren b/test/method/not_found_eleven_arguments.wren new file mode 100644 index 00000000..6bdccaa6 --- /dev/null +++ b/test/method/not_found_eleven_arguments.wren @@ -0,0 +1,3 @@ +class Foo {} + +(new Foo).someUnknownMethod(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11) // expect runtime error: Foo does not implement method 'someUnknownMethod' with 11 arguments. \ No newline at end of file diff --git a/test/method/not_found_multiple_arguments.wren b/test/method/not_found_multiple_arguments.wren new file mode 100644 index 00000000..3ff67353 --- /dev/null +++ b/test/method/not_found_multiple_arguments.wren @@ -0,0 +1,3 @@ +class Foo {} + +(new Foo).someUnknownMethod(1, 2) // expect runtime error: Foo does not implement method 'someUnknownMethod' with 2 arguments. \ No newline at end of file diff --git a/test/method/not_found_one_argument.wren b/test/method/not_found_one_argument.wren new file mode 100644 index 00000000..07958bd7 --- /dev/null +++ b/test/method/not_found_one_argument.wren @@ -0,0 +1,3 @@ +class Foo {} + +(new Foo).someUnknownMethod(1) // expect runtime error: Foo does not implement method 'someUnknownMethod' with 1 argument. \ No newline at end of file diff --git a/test/method/static_method_not_found.wren b/test/method/static_method_not_found.wren index cd58d155..39ee8af2 100644 --- a/test/method/static_method_not_found.wren +++ b/test/method/static_method_not_found.wren @@ -1,3 +1,3 @@ class Foo {} -Foo.bar // expect runtime error: Foo metaclass does not implement method 'bar'. \ No newline at end of file +Foo.bar // expect runtime error: Foo metaclass does not implement method 'bar' with 0 arguments. \ No newline at end of file diff --git a/test/super/no_superclass_method.wren b/test/super/no_superclass_method.wren index d1126c95..a3369b96 100644 --- a/test/super/no_superclass_method.wren +++ b/test/super/no_superclass_method.wren @@ -1,7 +1,7 @@ class Base {} class Derived is Base { - foo { super.doesNotExist } // expect runtime error: Base does not implement method 'doesNotExist'. + foo { super.doesNotExist } // expect runtime error: Base does not implement method 'doesNotExist' with 0 arguments. } (new Derived).foo From 15da548f484881c12f1200d5bef1edb57b38cbcd Mon Sep 17 00:00:00 2001 From: Gavin Schulz Date: Thu, 8 Jan 2015 21:30:47 -0800 Subject: [PATCH 2/2] Code review changes. --- src/wren_vm.c | 71 +++++++++++++++++++++------------------------------ 1 file changed, 29 insertions(+), 42 deletions(-) diff --git a/src/wren_vm.c b/src/wren_vm.c index f4f67952..c23f2cca 100644 --- a/src/wren_vm.c +++ b/src/wren_vm.c @@ -24,37 +24,32 @@ static void* defaultReallocate(void* memory, size_t oldSize, size_t newSize) return realloc(memory, newSize); } -// Returns the canonical name of a symbol with the argument spaces stripped off. -char* canonicalSymbol(char* symbolName) -{ - // Find the end of the text in the given symbol name. - int pos = strlen(symbolName) - 1; - while (symbolName[pos] == ' ') pos--; - - // Create a new string that contains only the symbol name. - char *canonicalizedSymbol = malloc(pos + 1); - strncpy(canonicalizedSymbol, symbolName, pos + 1); - - return canonicalizedSymbol; -} - -// Extracts the number of arguments from a symbol name. -char* methodArgumentsString(char* symbolName) +char* methodNotFound(char* className, char* symbolName) { // Count the number of spaces to determine number of arguments for this // symbol. int pos = strlen(symbolName) - 1; while (symbolName[pos] == ' ') pos--; - int args = strlen(symbolName) - pos - 1; + int arguments = strlen(symbolName) - pos - 1; - if (args == 1) return "1 argument"; + // Create a new string that contains only the symbol name. + char *canonicalizedSymbol = malloc(pos + 1); + strncpy(canonicalizedSymbol, symbolName, pos + 1); - int stringSize = args > 9 ? 13 : 12; - char* argumentString = malloc(stringSize); - snprintf(argumentString, stringSize, "%d arguments", args); + int messageLength = strlen(className) + strlen(canonicalizedSymbol) + + (arguments > 9 ? 48 : 47) + (arguments == 1 ? 0 : 1); + char *message = malloc(messageLength); - return argumentString; + snprintf(message, messageLength, "%s does not implement method '%s' with %d argument%s.", + className, + canonicalizedSymbol, + arguments, + arguments == 1 ? "" : "s"); + + free(canonicalizedSymbol); + + return &message[0]; } WrenVM* wrenNewVM(WrenConfiguration* configuration) @@ -608,12 +603,10 @@ static bool runInterpreter(WrenVM* vm) // If the class's method table doesn't include the symbol, bail. if (symbol >= classObj->methods.count) { - char message[100]; - snprintf(message, 100, "%s does not implement method '%s' with %s.", - classObj->name->value, - canonicalSymbol(vm->methodNames.data[symbol]), - methodArgumentsString(vm->methodNames.data[symbol])); + char *message = methodNotFound(classObj->name->value, + vm->methodNames.data[symbol]); RUNTIME_ERROR(message); + free(message); } Method* method = &classObj->methods.data[symbol]; @@ -662,12 +655,10 @@ static bool runInterpreter(WrenVM* vm) case METHOD_NONE: { - char message[100]; - snprintf(message, 100, "%s does not implement method '%s' with %s.", - classObj->name->value, - canonicalSymbol(vm->methodNames.data[symbol]), - methodArgumentsString(vm->methodNames.data[symbol])); + char *message = methodNotFound(classObj->name->value, + vm->methodNames.data[symbol]); RUNTIME_ERROR(message); + free(message); } } DISPATCH(); @@ -714,12 +705,10 @@ static bool runInterpreter(WrenVM* vm) // If the class's method table doesn't include the symbol, bail. if (symbol >= classObj->methods.count) { - char message[100]; - snprintf(message, 100, "%s does not implement method '%s' with %s.", - classObj->name->value, - canonicalSymbol(vm->methodNames.data[symbol]), - methodArgumentsString(vm->methodNames.data[symbol])); + char *message = methodNotFound(classObj->name->value, + vm->methodNames.data[symbol]); RUNTIME_ERROR(message); + free(message); } Method* method = &classObj->methods.data[symbol]; @@ -768,12 +757,10 @@ static bool runInterpreter(WrenVM* vm) case METHOD_NONE: { - char message[100]; - snprintf(message, 100, "%s does not implement method '%s' with %s.", - classObj->name->value, - canonicalSymbol(vm->methodNames.data[symbol]), - methodArgumentsString(vm->methodNames.data[symbol])); + char *message = methodNotFound(classObj->name->value, + vm->methodNames.data[symbol]); RUNTIME_ERROR(message); + free(message); } } DISPATCH();