Improve error reporting (but it's mostly untested).
authorBen Pfaff <blp@cs.stanford.edu>
Sat, 3 Jul 2021 21:08:24 +0000 (14:08 -0700)
committerBen Pfaff <blp@cs.stanford.edu>
Sat, 3 Jul 2021 21:08:24 +0000 (14:08 -0700)
src/language/lexer/macro.c
src/libpspp/message.c
src/libpspp/message.h
src/ui/terminal/main.c

index ad6a2be20edf99372a3439d720816e41c7f8335f..468fdcf1def9f682d17bde936842d6a2b8e1ab27 100644 (file)
@@ -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;
index bbba1c5506e9cdb1dad009d4eeedce06401e7f24..f1afc8fe3050481df753ff4b93bd03ae22fb9d39 100644 (file)
@@ -105,12 +105,18 @@ msg_set_handler (void (*handler) (const struct msg *, void *aux), void *aux)
 \f
 /* 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)
     }
 }
 \f
+/* 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;
+}
+\f
 /* 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);
index 36aae9c5d13137d790493ae6ac102ec5cc3ba600..d403b696539a4eabe07e9c0a5e520a717d6c37f3 100644 (file)
@@ -19,6 +19,7 @@
 
 #include <stdarg.h>
 #include <stdbool.h>
+#include <stddef.h>
 #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. */
   };
index 1def93fdab623ef8fe4bbe96930f689df5977af6..f43bd5dc11556c2ed05fade994654c7c8d989860 100644 (file)
@@ -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),