From 3bfaa3bb31985596dde140cb7b28083d29626e0d Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Sat, 3 Jul 2021 14:08:24 -0700 Subject: [PATCH] Improve error reporting (but it's mostly untested). --- src/language/lexer/macro.c | 191 ++++++++++++++++++++----------------- src/libpspp/message.c | 52 +++++++++- src/libpspp/message.h | 13 +++ src/ui/terminal/main.c | 2 + 4 files changed, 172 insertions(+), 86 deletions(-) diff --git a/src/language/lexer/macro.c b/src/language/lexer/macro.c index ad6a2be20e..468fdcf1de 100644 --- a/src/language/lexer/macro.c +++ b/src/language/lexer/macro.c @@ -433,6 +433,58 @@ struct macro_expansion_stack int last_line; }; +static void PRINTF_FORMAT (2, 3) +macro_error (const struct macro_expansion_stack *stack, + const char *format, ...) +{ + struct msg_stack **ms = NULL; + size_t allocated_ms = 0; + size_t n_ms = 0; + + for (const struct macro_expansion_stack *p = stack; p; p = p->next) + { + if (n_ms >= allocated_ms) + ms = x2nrealloc (ms, &allocated_ms, sizeof *ms); + + ms[n_ms] = xmalloc (sizeof *ms[n_ms]); + *ms[n_ms] = (struct msg_stack) { + .location = { + .file_name = xstrdup_if_nonnull (p->file_name), + .first_line = p->first_line, + .last_line = p->last_line, + }, + .description = ( + p == stack + /* TRANSLATORS: These strings are used for explaining the context of + an error. The "While expanding" message appears first, followed + by zero or more of the "inside expansion" messages, like this: + + foo.sps:12: While expanding 'innermost', + foo.sps:23: inside expansion of 'next_inner', + foo.sps:34: inside expansion of 'next_inner2', + foo.sps:45: inside expansion of 'outermost', + foo.sps:76: This is the actual error message. */ + ? xasprintf (_("While expanding \"%s\","), p->name) + : xasprintf (_("inside expansion of \"%s\","), p->name)) + }; + n_ms++; + } + + va_list args; + va_start (args, format); + + struct msg *m = xmalloc (sizeof *m); + *m = (struct msg) { + .category = MSG_C_SYNTAX, + .severity = MSG_S_ERROR, + .stack = ms, + .n_stack = n_ms, + .text = xvasprintf (format, args), + }; + msg_emit (m); + + va_end (args); +} enum me_state { @@ -886,7 +938,7 @@ parse_macro_function (struct parse_macro_function_ctx *ctx, if (n_tokens < 2 || tokens[1].token.type != T_LPAREN) { - printf ("`(' expected following %s'\n", function.string); + macro_error (ctx->stack, _("`(' expected following %s"), function.string); return false; } @@ -901,7 +953,9 @@ parse_macro_function (struct parse_macro_function_ctx *ctx, *input_consumed = i + 1; if (args->n < min_args || args->n > max_args) { - printf ("Wrong number of arguments to %s.\n", function.string); + macro_error (ctx->stack, + _("Wrong number of arguments to macro function %s."), + function.string); goto error; } return true; @@ -920,14 +974,16 @@ parse_macro_function (struct parse_macro_function_ctx *ctx, i++; else if (tokens[i].token.type != T_RPAREN) { - printf ("Expecting `,' or `)' in %s invocation.", function.string); + macro_error (ctx->stack, + _("`,' or `)' expected in call to macro function %s."), + function.string); goto error; } } unexpected_end: - printf ("Missing closing parenthesis in arguments to %s.\n", - function.string); + macro_error (ctx->stack, _("Missing `)' in call to macro function %s."), + function.string); /* Fall through. */ error: string_array_destroy (args); @@ -999,7 +1055,9 @@ expand_macro_function (struct parse_macro_function_ctx *ctx, int n; if (!parse_integer (args.strings[0], &n)) { - printf ("argument to !BLANKS must be non-negative integer (not \"%s\")\n", args.strings[0]); + macro_error (ctx->stack, + _("Argument to !BLANKS must be non-negative integer " + "(not \"%s\")"), args.strings[0]); string_array_destroy (&args); return false; } @@ -1057,7 +1115,9 @@ expand_macro_function (struct parse_macro_function_ctx *ctx, int start; if (!parse_integer (args.strings[1], &start) || start < 1) { - printf ("second argument to !SUBSTR must be positive integer (not \"%s\")\n", args.strings[1]); + macro_error (ctx->stack, _("Second argument of !SUBSTR must be " + "positive integer (not \"%s\")"), + args.strings[1]); string_array_destroy (&args); return false; } @@ -1065,7 +1125,9 @@ expand_macro_function (struct parse_macro_function_ctx *ctx, int count = INT_MAX; if (args.n > 2 && (!parse_integer (args.strings[2], &count) || count < 0)) { - printf ("third argument to !SUBSTR must be non-negative integer (not \"%s\")\n", args.strings[1]); + macro_error (ctx->stack, _("Third argument of !SUBSTR must be " + "non-negative integer (not \"%s\")"), + args.strings[1]); string_array_destroy (&args); return false; } @@ -1167,7 +1229,7 @@ macro_evaluate_literal (const struct expr_context *ctx, if (p >= end || p->token.type != T_RPAREN) { free (value); - printf ("expecting ')' in macro expression\n"); + macro_error (ctx->stack, _("Expecting ')' in macro expression.")); return NULL; } p++; @@ -1408,7 +1470,8 @@ macro_evaluate_number (const struct macro_token **tokens, size_t n_tokens, if (mts.n != 1 || !token_is_number (&mts.mts[0].token)) { macro_tokens_print (&mts, stdout); - printf ("expression must evaluate to a number (not %s)\n", s); + macro_error (stack, _("Macro expression must evaluate to " + "a number (not \"%s\")"), s); free (s); macro_tokens_uninit (&mts); return false; @@ -1470,7 +1533,7 @@ macro_expand_if (const struct macro_token *tokens, size_t n_tokens, || p->token.type != T_MACRO_ID || !ss_equals_case (p->token.string, ss_cstr ("!THEN"))) { - printf ("!THEN expected\n"); + macro_error (stack, _("!THEN expected in macro !IF construct.")); return 0; } @@ -1478,7 +1541,8 @@ macro_expand_if (const struct macro_token *tokens, size_t n_tokens, const struct macro_token *end_then = find_ifend_clause (start_then, end); if (!end_then) { - printf ("!ELSE or !IFEND expected\n"); + macro_error (stack, + _("!ELSE or !IFEND expected in macro !IF construct.")); return 0; } @@ -1490,7 +1554,7 @@ macro_expand_if (const struct macro_token *tokens, size_t n_tokens, if (!end_if || !ss_equals_case (end_if->token.string, ss_cstr ("!IFEND"))) { - printf ("!IFEND expected\n"); + macro_error (stack, _("!IFEND expected in macro !IF construct.")); return 0; } } @@ -1534,60 +1598,6 @@ macro_expand_if (const struct macro_token *tokens, size_t n_tokens, return (end_if + 1) - tokens; } -static void PRINTF_FORMAT (2, 3) -macro_error (const struct macro_expansion_stack *stack, - const char *format, ...) -{ - va_list args; - va_start (args, format); - char *s = xvasprintf (format, args); - va_end (args); - - /* foo.sps:12: While expanding 'innermost', - foo.sps:23: inside expansion of 'next_inner', - foo.sps:34: inside expansion of 'next_inner2', - foo.sps:45: inside expansion of 'outermost': - error. */ - struct string header = DS_EMPTY_INITIALIZER; - if (stack->file_name || stack->first_line) - { - if (stack->file_name) - ds_put_format (&header, "%s:", stack->file_name); - if (stack->first_line) - { - if (stack->last_line > stack->first_line) - ds_put_format (&header, "%d-%d:", - stack->first_line, stack->last_line); - else - ds_put_format (&header, "%d", stack->first_line); - } - ds_put_byte (&header, ' '); - } - ds_put_format (&header, "While expanding \"%s\"\n", stack->name); - while ((stack = stack->next) != NULL) - { - if (stack->file_name || stack->first_line) - { - if (stack->file_name) - ds_put_format (&header, "%s:", stack->file_name); - if (stack->first_line) - { - if (stack->last_line > stack->first_line) - ds_put_format (&header, "%d-%d:", - stack->first_line, stack->last_line); - else - ds_put_format (&header, "%d", stack->first_line); - } - ds_put_byte (&header, ' '); - } - ds_put_format (&header, ", inside expansion of \"%s\"\n", stack->name); - } - msg (SE, "%s: %s", ds_cstr (&header), s); - - ds_destroy (&header); - free (s); -} - static size_t macro_parse_let (const struct macro_token *tokens, size_t n_tokens, int nesting_countdown, const struct macro_set *macros, @@ -1604,21 +1614,23 @@ macro_parse_let (const struct macro_token *tokens, size_t n_tokens, if (p >= end || p->token.type != T_MACRO_ID) { - macro_error (stack, "expected macro variable name following !LET"); + macro_error (stack, _("Expected macro variable name following !LET.")); return 0; } const struct substring var_name = p->token.string; if (is_macro_keyword (var_name) || macro_find_parameter_by_name (me->macro, var_name)) { - printf ("cannot use argument name or macro keyword %.*s as !LET variable\n", (int) var_name.length, var_name.string); + macro_error (stack, _("Cannot use argument name or macro keyword " + "\"%.*s\" as !LET variable"), + (int) var_name.length, var_name.string); return 0; } p++; if (p >= end || p->token.type != T_EQUALS) { - printf ("expected = following !LET\n"); + macro_error (stack, _("Expected `=' following !LET")); return 0; } p++; @@ -1634,7 +1646,8 @@ macro_parse_let (const struct macro_token *tokens, size_t n_tokens, } static const struct macro_token * -find_doend (const struct macro_token *p, const struct macro_token *end) +find_doend (const struct macro_expansion_stack *stack, + const struct macro_token *p, const struct macro_token *end) { size_t nesting = 0; for (; p < end; p++) @@ -1651,7 +1664,7 @@ find_doend (const struct macro_token *p, const struct macro_token *end) nesting--; } } - printf ("missing !DOEND\n"); + macro_error (stack, _("Missing !DOEND.")); return NULL; } @@ -1672,14 +1685,16 @@ macro_expand_do (const struct macro_token *tokens, size_t n_tokens, if (p >= end || p->token.type != T_MACRO_ID) { - printf ("expected macro variable name following !DO\n"); + macro_error (stack, _("Expected macro variable name following !DO")); return 0; } const struct substring var_name = p->token.string; if (is_macro_keyword (var_name) || macro_find_parameter_by_name (me->macro, var_name)) { - printf ("cannot use argument name or macro keyword %.*s as !DO variable\n", (int) var_name.length, var_name.string); + macro_error (stack, _("Cannot use argument name or macro " + "keyword \"%.*s\" as !DO variable"), + (int) var_name.length, var_name.string); return 0; } p++; @@ -1703,7 +1718,7 @@ macro_expand_do (const struct macro_token *tokens, size_t n_tokens, SEG_MODE_INTERACTIVE /* XXX */); free (list); - const struct macro_token *do_end = find_doend (p, end); + const struct macro_token *do_end = find_doend (stack, p, end); if (!do_end) { macro_tokens_uninit (&items); @@ -1718,7 +1733,10 @@ macro_expand_do (const struct macro_token *tokens, size_t n_tokens, { if (i >= miterate) { - printf ("exceeded maximum number of iterations %d\n", miterate); + macro_error (stack, _("!DO loop over list exceeded " + "maximum number of iterations %d. " + "(Use SET MITERATE to change the limit.)"), + miterate); break; } string_map_replace_nocopy (vars, ss_xstrdup (var_name), @@ -1743,7 +1761,7 @@ macro_expand_do (const struct macro_token *tokens, size_t n_tokens, if (p >= end || p->token.type != T_MACRO_ID || !ss_equals_case (p->token.string, ss_cstr ("!TO"))) { - printf ("expecting !TO\n"); + macro_error (stack, _("Expected !TO in numerical !DO loop")); return 0; } p++; @@ -1764,12 +1782,12 @@ macro_expand_do (const struct macro_token *tokens, size_t n_tokens, if (by == 0.0) { - printf ("!BY value cannot be zero\n"); + macro_error (stack, _("!BY value cannot be zero.")); return 0; } } - const struct macro_token *do_end = find_doend (p, end); + const struct macro_token *do_end = find_doend (stack, p, end); if (!do_end) return 0; const struct macro_tokens inner = { @@ -1786,8 +1804,11 @@ macro_expand_do (const struct macro_token *tokens, size_t n_tokens, { if (i++ > miterate) { - printf ("exceeded maximum number of iterations %d\n", - miterate); + macro_error (stack, + _("Numerical !DO loop exceeded " + "maximum number of iterations %d. " + "(Use SET MITERATE to change the limit.)"), + miterate); break; } @@ -1808,7 +1829,7 @@ macro_expand_do (const struct macro_token *tokens, size_t n_tokens, } else { - printf ("expecting = or !IN in !DO loop\n"); + macro_error (stack, _("Expected `=' or !IN in !DO loop.")); return 0; } } @@ -1822,7 +1843,8 @@ macro_expand (const struct macro_tokens *mts, { if (nesting_countdown <= 0) { - printf ("maximum nesting level exceeded\n"); + macro_error (stack, _("Maximum nesting level %d exceeded."), + settings_get_mnest ()); for (size_t i = 0; i < mts->n; i++) macro_tokens_add (exp, &mts->mts[i]); return; @@ -1843,7 +1865,6 @@ macro_expand (const struct macro_tokens *mts, if (param) { const struct macro_tokens *arg = me->args[param - me->macro->params]; - //macro_tokens_print (arg, stdout); if (*expand && param->expand_arg) macro_expand (arg, nesting_countdown, macros, NULL, NULL, &(struct macro_expansion_stack) { @@ -1939,7 +1960,7 @@ macro_expand (const struct macro_tokens *mts, if (ss_equals_case (token->string, ss_cstr ("!break"))) { if (!break_) - printf ("!BREAK outside !DO\n"); + macro_error (stack, _("!BREAK outside !DO.")); else { *break_ = true; diff --git a/src/libpspp/message.c b/src/libpspp/message.c index bbba1c5506..f1afc8fe30 100644 --- a/src/libpspp/message.c +++ b/src/libpspp/message.c @@ -105,12 +105,18 @@ msg_set_handler (void (*handler) (const struct msg *, void *aux), void *aux) /* msg_location. */ +void +msg_location_uninit (struct msg_location *loc) +{ + free (loc->file_name); +} + void msg_location_destroy (struct msg_location *loc) { if (loc) { - free (loc->file_name); + msg_location_uninit (loc); free (loc); } } @@ -197,6 +203,31 @@ msg_location_format (const struct msg_location *loc, struct string *s) } } +/* msg_stack */ + +void +msg_stack_destroy (struct msg_stack *stack) +{ + if (stack) + { + msg_location_uninit (&stack->location); + free (stack->description); + free (stack); + } +} + +struct msg_stack * +msg_stack_dup (const struct msg_stack *src) +{ + struct msg_stack *dst = xmalloc (sizeof *src); + *dst = (struct msg_stack) { + .location = src->location, + .description = xstrdup_if_nonnull (src->description), + }; + dst->location.file_name = xstrdup_if_nonnull (dst->location.file_name); + return dst; +} + /* Working with messages. */ const char * @@ -218,10 +249,16 @@ msg_severity_to_string (enum msg_severity severity) struct msg * msg_dup (const struct msg *src) { + struct msg_stack **ms = xmalloc (src->n_stack * sizeof *ms); + for (size_t i = 0; i < src->n_stack; i++) + ms[i] = msg_stack_dup (src->stack[i]); + struct msg *dst = xmalloc (sizeof *dst); *dst = (struct msg) { .category = src->category, .severity = src->severity, + .stack = ms, + .n_stack = src->n_stack, .location = msg_location_dup (src->location), .command_name = xstrdup_if_nonnull (src->command_name), .text = xstrdup (src->text), @@ -239,6 +276,9 @@ msg_destroy (struct msg *m) { if (m) { + for (size_t i = 0; i < m->n_stack; i++) + msg_stack_destroy (m->stack[i]); + free (m->stack); msg_location_destroy (m->location); free (m->text); free (m->command_name); @@ -253,6 +293,16 @@ msg_to_string (const struct msg *m) ds_init_empty (&s); + for (size_t i = 0; i < m->n_stack; i++) + { + const struct msg_stack *ms = m->stack[i]; + if (!msg_location_is_empty (&ms->location)) + { + msg_location_format (&ms->location, &s); + ds_put_cstr (&s, ": "); + } + ds_put_format (&s, "%s\n", ms->description); + } if (m->category != MSG_C_GENERAL && !msg_location_is_empty (m->location)) { msg_location_format (m->location, &s); diff --git a/src/libpspp/message.h b/src/libpspp/message.h index 36aae9c5d1..d403b69653 100644 --- a/src/libpspp/message.h +++ b/src/libpspp/message.h @@ -19,6 +19,7 @@ #include #include +#include #include "libpspp/compiler.h" struct string; @@ -80,18 +81,30 @@ struct msg_location int last_column; /* 1-based exclusive last column (0=none). */ }; +void msg_location_uninit (struct msg_location *); void msg_location_destroy (struct msg_location *); struct msg_location *msg_location_dup (const struct msg_location *); bool msg_location_is_empty (const struct msg_location *); void msg_location_format (const struct msg_location *, struct string *); +struct msg_stack + { + struct msg_location location; + char *description; + }; + +void msg_stack_destroy (struct msg_stack *); +struct msg_stack *msg_stack_dup (const struct msg_stack *); + /* A message. */ struct msg { enum msg_category category; /* Message category. */ enum msg_severity severity; /* Message severity. */ struct msg_location *location; /* Code location. */ + struct msg_stack **stack; + size_t n_stack; char *command_name; /* Name of erroneous command, or NULL. */ char *text; /* Error text. */ }; diff --git a/src/ui/terminal/main.c b/src/ui/terminal/main.c index 1def93fdab..f43bd5dc11 100644 --- a/src/ui/terminal/main.c +++ b/src/ui/terminal/main.c @@ -221,6 +221,8 @@ output_msg (const struct msg *m_, void *lexer_) struct msg m = { .category = m_->category, .severity = m_->severity, + .stack = m_->stack, + .n_stack = m_->n_stack, .location = (m_->location ? m_->location : lexer ? lex_get_lines (lexer, 0, 0) : NULL), -- 2.30.2