From dd580a15198e13cfcb9468211452b11521b8d5d7 Mon Sep 17 00:00:00 2001 From: Laria Carolin Chabowski Date: Tue, 24 Jan 2023 21:22:22 +0100 Subject: [PATCH] Improve a bunch of error messages --- src/apfl.h | 6 +-- src/context.c | 114 +++++++++++++++++++++++++++++++++++++++++++++---- src/context.h | 17 ++++++++ src/eval.c | 8 ++-- src/format.c | 91 +++++++++++++++++++++++++++++++++++++++ src/format.h | 12 ++++++ src/globals.c | 33 +++----------- src/messages.c | 4 -- src/value.c | 25 +++++++++++ 9 files changed, 262 insertions(+), 48 deletions(-) diff --git a/src/apfl.h b/src/apfl.h index e52f4b6..ddec212 100644 --- a/src/apfl.h +++ b/src/apfl.h @@ -612,6 +612,8 @@ enum apfl_value_type { APFL_VALUE_USERDATA, }; +const char *apfl_type_name(enum apfl_value_type); + typedef int apfl_stackidx; typedef size_t apfl_slotidx; @@ -746,17 +748,13 @@ struct apfl_messages { const char *key_doesnt_exist; const char *value_is_not_a_container; const char *wrong_key_type; - const char *variable_doesnt_exist; const char *not_a_c_function; const char *invalid_slotidx; - const char *not_a_function; const char *wrong_type; const char *io_error; const char *value_doesnt_match; const char *invalid_matcher_state; const char *no_matching_subfunction; - const char *uncomparable; - const char *incompatible_types; }; extern const struct apfl_messages apfl_messages; diff --git a/src/context.c b/src/context.c index faac0c2..3ca7629 100644 --- a/src/context.c +++ b/src/context.c @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -179,6 +180,86 @@ apfl_raise_error_object(apfl_ctx ctx, struct apfl_error error) apfl_raise_error(ctx, -1); } +struct errorfmt_data { + va_list ap; + apfl_ctx ctx; +}; + +static bool +errorfmt_str(void *opaque, struct apfl_format_writer w, struct apfl_string_view arg) +{ + struct errorfmt_data *data = opaque; + + struct apfl_string_view sv; + if (apfl_string_eq(arg, "c")) { + char *cstr = va_arg(data->ap, char *); + sv = apfl_string_view_from(cstr); + } else { + sv = va_arg(data->ap, struct apfl_string_view); + } + + return apfl_format_put_string_view(w, sv); +} + +static bool +errorfmt_type(void *opaque, struct apfl_format_writer w, struct apfl_string_view arg) +{ + struct errorfmt_data *data = opaque; + + enum apfl_value_type type; + if (apfl_string_eq(arg, "stack")) { + apfl_stackidx i = va_arg(data->ap, apfl_stackidx); + struct apfl_value val; + if (!apfl_stack_get(data->ctx, &val, i)) { + FMT_TRY(apfl_format_put_string(w, "{stack:type => invalid stack index}")); + return true; + } + type = apfl_value_type_to_abstract_type(val.type); + } else if (apfl_string_eq(arg, "value")) { + struct apfl_value val = va_arg(data->ap, struct apfl_value); + type = apfl_value_type_to_abstract_type(val.type); + } else { + type = va_arg(data->ap, enum apfl_value_type); + } + + return apfl_format_put_string_view(w, apfl_string_view_from(apfl_type_name(type))); +} + +static const struct format_spec errorfmt_specs[] = { + {"string", errorfmt_str}, + {"type", errorfmt_type}, + {NULL, NULL}, +}; + +APFL_NORETURN void +apfl_raise_errorfmt(apfl_ctx ctx, const char *fmt, ...) +{ + struct errorfmt_data data = { + .ctx = ctx, + }; + va_start(data.ap, fmt); + + struct apfl_string_builder sb = apfl_string_builder_init(ctx->gc.allocator); + struct apfl_format_writer w = apfl_format_string_writer(&sb); + + bool result = apfl_abstract_format(&data, errorfmt_specs, w, apfl_string_view_from(fmt)); + va_end(data.ap); + + if (!result) { + apfl_string_builder_deinit(&sb); + apfl_raise_alloc_error(ctx); + } + + struct apfl_string string = apfl_string_builder_move_string(&sb); + apfl_string_builder_deinit(&sb); + + if (!apfl_move_string_onto_stack(ctx, string)) { + apfl_raise_alloc_error(ctx); + } + + apfl_raise_error(ctx, -1); +} + void apfl_ctx_set_panic_callback(apfl_ctx ctx, apfl_panic_callback cb, void *opaque) { @@ -1001,7 +1082,11 @@ apfl_list_append(apfl_ctx ctx, apfl_stackidx list_index, apfl_stackidx value_ind } if (list_val->type != VALUE_LIST) { - apfl_raise_const_error(ctx, apfl_messages.not_a_list); + apfl_raise_errorfmt( + ctx, + "Can not append to value of type {value:type}, expected list", + *list_val + ); } struct apfl_value value = apfl_stack_must_get(ctx, value_index); @@ -1131,10 +1216,19 @@ apfl_get_member_if_exists( case GET_ITEM_KEY_DOESNT_EXIST: return false; case GET_ITEM_NOT_A_CONTAINER: - apfl_raise_const_error(ctx, apfl_messages.value_is_not_a_container); + apfl_raise_errorfmt( + ctx, + "Can not get member of value of type {value:type}, not a container", + container + ); break; case GET_ITEM_WRONG_KEY_TYPE: - apfl_raise_const_error(ctx, apfl_messages.wrong_key_type); + apfl_raise_errorfmt( + ctx, + "Value of type {value:type} is not a valid key for container type {value:type}", + k, + container + ); break; } @@ -1224,7 +1318,11 @@ apfl_len(apfl_ctx ctx, apfl_stackidx index) case VALUE_FUNC: case VALUE_CFUNC: case VALUE_USERDATA: - apfl_raise_const_error(ctx, apfl_messages.wrong_type); + apfl_raise_errorfmt( + ctx, + "Can not get length of value of type {value:type}", + value + ); return 0; case VALUE_STRING: return value.string->len; @@ -1270,7 +1368,7 @@ apfl_get_string(apfl_ctx ctx, apfl_stackidx index) { struct apfl_string_view sv; if (!get_string_view_of_value(&sv, apfl_stack_must_get(ctx, index))) { - apfl_raise_const_error(ctx, apfl_messages.wrong_type); + apfl_raise_errorfmt(ctx, "Expected string, got {stack:type}", index); } return sv; } @@ -1405,7 +1503,7 @@ apfl_get_number(apfl_ctx ctx, apfl_stackidx index) if (value.type == VALUE_NUMBER) { return value.number; } - apfl_raise_const_error(ctx, apfl_messages.wrong_type); + apfl_raise_errorfmt(ctx, "Expected number, got {value:type}", value); } bool @@ -1436,9 +1534,9 @@ apfl_cmp(apfl_ctx ctx, apfl_stackidx _a, apfl_stackidx _b) case CMP_GT: return 1; case CMP_UNCOMPARABLE: - apfl_raise_const_error(ctx, apfl_messages.uncomparable); + apfl_raise_errorfmt(ctx, "Values of type {value:type} can not be compared", a); case CMP_INCOMPATIBLE_TYPES: - apfl_raise_const_error(ctx, apfl_messages.incompatible_types); + apfl_raise_errorfmt(ctx, "Can not compare values of incompatible types {value:type} and {value:type}", a, b); } assert(false); diff --git a/src/context.h b/src/context.h index 18025ae..4288e39 100644 --- a/src/context.h +++ b/src/context.h @@ -179,6 +179,23 @@ void apfl_stack_clear(apfl_ctx); struct apfl_value *apfl_stack_push_placeholder(apfl_ctx); bool apfl_move_string_onto_stack(apfl_ctx, struct apfl_string); +/* Raise an error with a message formatted according to fmt. + * + * fmt allows placeholders in the form of `{:}`, where `:` + * is optional. Each of these placeholders then takes a vararg. The following + * placeholders are implemented: + * + * - string: Expects a `struct apfl_string_view` parameter and replaces the + * placeholder with that string. With the param `c`, a C `char *` string is + * expected instead. + * - type: Expects an `enum apfl_type` and will replace the placeholder with the + * name of that type. With the param `value` a `struct apfl_value` is expected + * instead and the type of that value is used. With the param `stack` a + * `apfl_stackidx` is expected instead and the type of the value at the given + * stack index is used instead. + */ +APFL_NORETURN void apfl_raise_errorfmt(apfl_ctx ctx, const char *fmt, ...); + struct call_stack_entry *apfl_call_stack_cur_entry(apfl_ctx); struct scope *apfl_closure_scope_for_func(apfl_ctx, struct scopes); diff --git a/src/eval.c b/src/eval.c index 1475eeb..a9e933d 100644 --- a/src/eval.c +++ b/src/eval.c @@ -128,7 +128,7 @@ variable_get(apfl_ctx ctx, struct scopes scopes, struct apfl_string *name, bool return; } - apfl_raise_const_error(ctx, apfl_messages.variable_doesnt_exist); + apfl_raise_errorfmt(ctx, "Variable {string} does not exist.", apfl_string_view_from(*name)); } static bool @@ -437,7 +437,7 @@ call_inner(apfl_ctx ctx, size_t tmproots, apfl_stackidx func_index, apfl_stackid assert(apfl_stack_drop_multi(ctx, 2, (apfl_stackidx[]){func_index, args_index})); if (!VALUE_IS_A(func, APFL_VALUE_FUNC)) { - apfl_raise_const_error(ctx, apfl_messages.not_a_function); + apfl_raise_errorfmt(ctx, "Can only call functions, got a {value:type} instead", func); } if (!VALUE_IS_A(args, APFL_VALUE_LIST)) { apfl_raise_const_error(ctx, apfl_messages.not_a_list); @@ -1036,7 +1036,7 @@ matcher_transfer(apfl_ctx ctx, struct matcher_call_stack_entry *cse) variable_get(ctx, cse->scopes, transfer.var, false); if (apfl_get_type(ctx, -1) != APFL_VALUE_DICT) { - apfl_raise_const_error(ctx, apfl_messages.not_a_dict); + apfl_raise_errorfmt(ctx, "Can not update value of type {stack:type}, expected dict", -1); } @@ -1050,7 +1050,7 @@ matcher_transfer(apfl_ctx ctx, struct matcher_call_stack_entry *cse) apfl_stack_must_push(ctx, apfl_value_set_cow_flag(cse->matcher->values[value_index])); if (apfl_get_member_if_exists(ctx, -2, -1)) { if (apfl_get_type(ctx, -1) != APFL_VALUE_DICT) { - apfl_raise_const_error(ctx, apfl_messages.not_a_dict); + apfl_raise_errorfmt(ctx, "Can not update value of type {stack:type}, expected dict", -1); } } else { apfl_dict_create(ctx); diff --git a/src/format.c b/src/format.c index ff5c7e3..d62cac6 100644 --- a/src/format.c +++ b/src/format.c @@ -125,3 +125,94 @@ apfl_format_put_poiner(struct apfl_format_writer w, void *ptr) assert(len < PUT_POINTER_BUFSIZE); return write(w, buf, len); } + +static bool +search_next_char(struct apfl_string_view sv, size_t *off, char needle) +{ + while (*off < sv.len) { + if (sv.bytes[*off] == needle) { + return true; + } + (*off)++; + } + return false; +} + +static bool +write_leading(struct apfl_format_writer w, struct apfl_string_view fmt, size_t start, size_t off) +{ + assert(start <= off); + + TRY(apfl_format_put_string(w, apfl_string_view_substr(fmt, start, off - start))); + return true; +} + +bool +apfl_abstract_format( + void *opaque, + const struct format_spec *specs, + struct apfl_format_writer w, + struct apfl_string_view fmt +) { + size_t off = 0; + size_t start = 0; + while (search_next_char(fmt, &off, '{')) { + assert(off >= start); + assert(off < fmt.len); + assert(fmt.bytes[off] == '{'); + + TRY(write_leading(w, fmt, start, off)); + + off++; + start = off; + + if (off < fmt.len && fmt.bytes[off] == '{') { + // We got "{{", used to escape "{". Write that and start over. + TRY(apfl_format_put_char(w, '{')); + + off++; + start = off; + + continue; + } + + if (!search_next_char(fmt, &off, '}')) { + // No closing '}', write rest of fmt and abort. + TRY(apfl_format_put_string_view(w, apfl_string_view_offset(fmt, start))); + return true; + } + + struct apfl_string_view placeholder = apfl_string_view_substr(fmt, start, off - start); + struct apfl_string_view arg = { .len = 0, .bytes = NULL }; + + off++; + start = off; + + size_t argsep = 0; + if (search_next_char(placeholder, &argsep, ':')) { + arg = apfl_string_view_trunc(placeholder, argsep); + placeholder = apfl_string_view_offset(placeholder, argsep+1); + } + + const struct format_spec *spec = specs; + for (; spec->name != NULL && spec->callback != NULL; spec++) { + if (apfl_string_eq(spec->name, placeholder)) { + TRY(spec->callback(opaque, w, arg)); + + goto continue_inner; + } + } + + TRY(apfl_format_put_string(w, "{UNKNOWN FORMAT \"")); + TRY(apfl_format_put_string(w, arg)); + TRY(apfl_format_put_string(w, ":")); + TRY(apfl_format_put_string(w, placeholder)); + TRY(apfl_format_put_string(w, "\"}")); + + continue_inner:; + } + + TRY(write_leading(w, fmt, start, off)); + + return true; +} diff --git a/src/format.h b/src/format.h index 9c93183..673d99f 100644 --- a/src/format.h +++ b/src/format.h @@ -11,6 +11,18 @@ extern "C" { #define FMT_TRY(x) do { if (!(x)) return false; } while (0) +struct format_spec { + const char *name; + bool (*callback)(void *, struct apfl_format_writer, struct apfl_string_view arg); +}; + +bool apfl_abstract_format( + void *opaque, + const struct format_spec *specs, + struct apfl_format_writer w, + struct apfl_string_view fmt +); + #ifdef __cplusplus } #endif diff --git a/src/globals.c b/src/globals.c index ab5969e..ba748eb 100644 --- a/src/globals.c +++ b/src/globals.c @@ -231,8 +231,10 @@ disasm(apfl_ctx ctx) apfl_get_list_member_by_index(ctx, 0, 0); apfl_drop(ctx, 0); struct apfl_value value = apfl_stack_must_get(ctx, -1); - if (value.type != VALUE_FUNC) { - apfl_raise_const_error(ctx, apfl_messages.wrong_type); + if (value.type == VALUE_CFUNC) { + apfl_raise_const_error(ctx, "disasm needs a apfl function, got a native function instead"); + } else if (value.type != VALUE_FUNC) { + apfl_raise_errorfmt(ctx, "disasm needs a apfl function, got value of type {value:type} instead", value); } struct function *func = value.func; @@ -289,36 +291,11 @@ len(apfl_ctx ctx) apfl_push_number(ctx, (apfl_number)l); } -static const char * -typestr(enum apfl_value_type type) -{ - switch (type) { - case APFL_VALUE_NIL: - return "nil"; - case APFL_VALUE_BOOLEAN: - return "bool"; - case APFL_VALUE_NUMBER: - return "number"; - case APFL_VALUE_STRING: - return "string"; - case APFL_VALUE_LIST: - return "list"; - case APFL_VALUE_DICT: - return "dict"; - case APFL_VALUE_FUNC: - return "function"; - case APFL_VALUE_USERDATA: - return "userdata"; - } - - return "?"; -} - static void type(apfl_ctx ctx) { ONE_ARG(ctx, "type"); - apfl_push_const_string(ctx, typestr(apfl_get_type(ctx, -1))); + apfl_push_const_string(ctx, apfl_type_name(apfl_get_type(ctx, -1))); } static void diff --git a/src/messages.c b/src/messages.c index ed5dcf9..1589cbc 100644 --- a/src/messages.c +++ b/src/messages.c @@ -11,15 +11,11 @@ const struct apfl_messages apfl_messages = { .key_doesnt_exist = "Key does not exist", .value_is_not_a_container = "Value is not a container (list or dictionary)", .wrong_key_type = "Wrong key type", - .variable_doesnt_exist = "Variable does not exist", .not_a_c_function = "Not a C function", .invalid_slotidx = "Invalid slot index", - .not_a_function = "Not a function", .wrong_type = "Wrong type", .io_error = "I/O error", .value_doesnt_match = "Value does not match", .invalid_matcher_state = "Matcher is in invalid state", .no_matching_subfunction = "No matching subfunction", - .uncomparable = "Value is not comparable", - .incompatible_types = "Incompatible types", }; diff --git a/src/value.c b/src/value.c index 3e271b4..c43d5fb 100644 --- a/src/value.c +++ b/src/value.c @@ -187,6 +187,31 @@ apfl_value_type_to_abstract_type(enum value_type type) return 0; } +const char * +apfl_type_name(enum apfl_value_type type) +{ + switch (type) { + case APFL_VALUE_NIL: + return "nil"; + case APFL_VALUE_BOOLEAN: + return "bool"; + case APFL_VALUE_NUMBER: + return "number"; + case APFL_VALUE_STRING: + return "string"; + case APFL_VALUE_LIST: + return "list"; + case APFL_VALUE_DICT: + return "dict"; + case APFL_VALUE_FUNC: + return "function"; + case APFL_VALUE_USERDATA: + return "userdata"; + } + + return "?"; +} + struct function * apfl_func_new(struct gc *gc, size_t cap, struct scope *scope) {