Improve macro error messages.

diff --git a/src/language/lexer/lexer.c b/src/language/lexer/lexer.c
index 67ecfee07d00bb235cfbb38654075966168795f7..1025414a7c4407bf265519af144b255c1d836629 100644 (file)
--- a/src/language/lexer/lexer.c
+++ b/src/language/lexer/lexer.c
@@ -1480,8 +1480,8 @@ lex_source_get_syntax__ (const struct lex_source *src, int n0, int n1)
   return ds_steal_cstr (&s);
 }
 
-static void
-lex_ellipsize__ (struct substring in, char *out, size_t out_size)
+void
+lex_ellipsize (struct substring in, char *out, size_t out_size)
 {
   size_t out_maxlen;
   size_t out_len;
@@ -1555,14 +1555,14 @@ lex_source_error_valist (struct lex_source *src, int n0, int n1,
       /* Get the syntax that caused the error. */
       char *syntax = lex_source_get_syntax__ (src, n0, n1);
       char syntax_cstr[64];
-      lex_ellipsize__ (ss_cstr (syntax), syntax_cstr, sizeof syntax_cstr);
+      lex_ellipsize (ss_cstr (syntax), syntax_cstr, sizeof syntax_cstr);
       free (syntax);
 
       /* Get the macro call(s) that expanded to the syntax that caused the
          error. */
       char call_cstr[64];
       struct substring call = lex_source_get_macro_call (src, n0, n1);
-      lex_ellipsize__ (call, call_cstr, sizeof call_cstr);
+      lex_ellipsize (call, call_cstr, sizeof call_cstr);
 
       if (syntax_cstr[0])
         {

diff --git a/src/language/lexer/lexer.h b/src/language/lexer/lexer.h
index 6aa900e8df70b9bbbb77da7d2c12919ff9ef15cc..1b30015234438d07cc5b263a7daaa0f31f8978d4 100644 (file)
--- a/src/language/lexer/lexer.h
+++ b/src/language/lexer/lexer.h
@@ -186,6 +186,8 @@ void lex_next_error_valist (struct lexer *lexer, int n0, int n1,
                             const char *format, va_list)
   PRINTF_FORMAT (4, 0);
 
+void lex_ellipsize (struct substring in, char *out, size_t out_size);
+
 /* Error handling. */
 enum segmenter_mode lex_get_syntax_mode (const struct lexer *);
 enum lex_error_mode lex_get_error_mode (const struct lexer *);

diff --git a/src/language/lexer/macro.c b/src/language/lexer/macro.c
index 468fdcf1def9f682d17bde936842d6a2b8e1ab27..42ccf4255f41f37b68394c7558d5bdaaa186170d 100644 (file)
--- a/src/language/lexer/macro.c
+++ b/src/language/lexer/macro.c
@@ -23,6 +23,7 @@
 #include <stdlib.h>
 
 #include "data/settings.h"
+#include "language/lexer/lexer.h"
 #include "language/lexer/segment.h"
 #include "language/lexer/scan.h"
 #include "libpspp/assertion.h"
@@ -433,8 +434,9 @@ struct macro_expansion_stack
     int last_line;
   };
 
-static void PRINTF_FORMAT (2, 3)
+static void PRINTF_FORMAT (3, 4)
 macro_error (const struct macro_expansion_stack *stack,
+             const struct macro_token *mt,
              const char *format, ...)
 {
   struct msg_stack **ms = NULL;
@@ -446,6 +448,33 @@ macro_error (const struct macro_expansion_stack *stack,
       if (n_ms >= allocated_ms)
         ms = x2nrealloc (ms, &allocated_ms, sizeof *ms);
 
+      /* 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.  `innermost',
+         `next_inner`, etc., are names of macros, and `foobar' is a piece of
+         PSPP syntax:
+
+         foo.sps:12: At `foobar' in the expansion of 'innermost',
+         foo.sps:23: inside the expansion of 'next_inner',
+         foo.sps:34: inside the expansion of 'next_inner2',
+         foo.sps:45: inside the expansion of 'outermost',
+         foo.sps:76: This is the actual error message. */
+      char *description;
+      if (p == stack)
+        {
+          if (mt && mt->representation.length)
+            {
+              char syntax[64];
+              lex_ellipsize (mt->representation, syntax, sizeof syntax);
+              description = xasprintf (_("At `%s' in the expansion of `%s',"),
+                                       syntax, p->name);
+            }
+          else
+            description = xasprintf (_("In the expansion of `%s',"), p->name);
+        }
+      else
+        description = xasprintf (_("inside the expansion of `%s',"), p->name);
+
       ms[n_ms] = xmalloc (sizeof *ms[n_ms]);
       *ms[n_ms] = (struct msg_stack) {
         .location = {
@@ -453,25 +482,15 @@ macro_error (const struct macro_expansion_stack *stack,
           .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))
+        .description = description,
       };
       n_ms++;
     }
 
   va_list args;
   va_start (args, format);
+  char *s = xvasprintf (format, args);
+  va_end (args);
 
   struct msg *m = xmalloc (sizeof *m);
   *m = (struct msg) {
@@ -479,11 +498,9 @@ macro_error (const struct macro_expansion_stack *stack,
     .severity = MSG_S_ERROR,
     .stack = ms,
     .n_stack = n_ms,
-    .text = xvasprintf (format, args),
+    .text = s,
   };
   msg_emit (m);
-
-  va_end (args);
 }
 
 enum me_state
@@ -938,7 +955,8 @@ parse_macro_function (struct parse_macro_function_ctx *ctx,
 
   if (n_tokens < 2 || tokens[1].token.type != T_LPAREN)
     {
-      macro_error (ctx->stack, _("`(' expected following %s"), function.string);
+      macro_error (ctx->stack, n_tokens > 1 ? &tokens[1] : NULL,
+                   _("`(' expected following %s"), function.string);
       return false;
     }
 
@@ -953,7 +971,7 @@ parse_macro_function (struct parse_macro_function_ctx *ctx,
           *input_consumed = i + 1;
           if (args->n < min_args || args->n > max_args)
             {
-              macro_error (ctx->stack,
+              macro_error (ctx->stack, &tokens[i],
                            _("Wrong number of arguments to macro function %s."),
                            function.string);
               goto error;
@@ -974,7 +992,7 @@ parse_macro_function (struct parse_macro_function_ctx *ctx,
         i++;
       else if (tokens[i].token.type != T_RPAREN)
         {
-          macro_error (ctx->stack,
+          macro_error (ctx->stack, &tokens[i],
                        _("`,' or `)' expected in call to macro function %s."),
                        function.string);
           goto error;
@@ -982,7 +1000,7 @@ parse_macro_function (struct parse_macro_function_ctx *ctx,
     }
 
 unexpected_end:
-  macro_error (ctx->stack, _("Missing `)' in call to macro function %s."),
+  macro_error (ctx->stack, NULL, _("Missing `)' in call to macro function %s."),
                function.string);
   /* Fall through. */
 error:
@@ -1055,7 +1073,7 @@ expand_macro_function (struct parse_macro_function_ctx *ctx,
       int n;
       if (!parse_integer (args.strings[0], &n))
         {
-          macro_error (ctx->stack,
+          macro_error (ctx->stack, NULL,
                        _("Argument to !BLANKS must be non-negative integer "
                          "(not \"%s\")"), args.strings[0]);
           string_array_destroy (&args);
@@ -1115,8 +1133,9 @@ expand_macro_function (struct parse_macro_function_ctx *ctx,
       int start;
       if (!parse_integer (args.strings[1], &start) || start < 1)
         {
-          macro_error (ctx->stack, _("Second argument of !SUBSTR must be "
-                                     "positive integer (not \"%s\")"),
+          macro_error (ctx->stack, NULL,
+                       _("Second argument of !SUBSTR must be "
+                         "positive integer (not \"%s\")"),
                        args.strings[1]);
           string_array_destroy (&args);
           return false;
@@ -1125,8 +1144,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))
         {
-          macro_error (ctx->stack, _("Third argument of !SUBSTR must be "
-                                     "non-negative integer (not \"%s\")"),
+          macro_error (ctx->stack, NULL,
+                       _("Third argument of !SUBSTR must be "
+                         "non-negative integer (not \"%s\")"),
                        args.strings[1]);
           string_array_destroy (&args);
           return false;
@@ -1229,7 +1249,8 @@ macro_evaluate_literal (const struct expr_context *ctx,
       if (p >= end || p->token.type != T_RPAREN)
         {
           free (value);
-          macro_error (ctx->stack, _("Expecting ')' in macro expression."));
+          macro_error (ctx->stack, p < end ? p : NULL,
+                       _("Expecting ')' in macro expression."));
           return NULL;
         }
       p++;
@@ -1470,8 +1491,9 @@ 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);
-      macro_error (stack, _("Macro expression must evaluate to "
-                            "a number (not \"%s\")"), s);
+      macro_error (stack, mts.n > 0 ? &mts.mts[0] : NULL,
+                   _("Macro expression must evaluate to "
+                     "a number (not \"%s\")"), s);
       free (s);
       macro_tokens_uninit (&mts);
       return false;
@@ -1533,7 +1555,8 @@ 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")))
     {
-      macro_error (stack, _("!THEN expected in macro !IF construct."));
+      macro_error (stack, p < end ? p : NULL,
+                   _("!THEN expected in macro !IF construct."));
       return 0;
     }
 
@@ -1541,7 +1564,7 @@ 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)
     {
-      macro_error (stack,
+      macro_error (stack, NULL,
                    _("!ELSE or !IFEND expected in macro !IF construct."));
       return 0;
     }
@@ -1554,7 +1577,8 @@ 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")))
         {
-          macro_error (stack, _("!IFEND expected in macro !IF construct."));
+          macro_error (stack, end_if ? end_if : NULL,
+                       _("!IFEND expected in macro !IF construct."));
           return 0;
         }
     }
@@ -1614,15 +1638,17 @@ 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, p < end ? p : NULL,
+                   _("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))
     {
-      macro_error (stack, _("Cannot use argument name or macro keyword "
-                            "\"%.*s\" as !LET variable"),
+      macro_error (stack, p < end ? p : NULL,
+                   _("Cannot use argument name or macro keyword "
+                     "\"%.*s\" as !LET variable"),
                    (int) var_name.length, var_name.string);
       return 0;
     }
@@ -1630,7 +1656,8 @@ macro_parse_let (const struct macro_token *tokens, size_t n_tokens,
 
   if (p >= end || p->token.type != T_EQUALS)
     {
-      macro_error (stack, _("Expected `=' following !LET"));
+      macro_error (stack, p < end ? p : NULL,
+                   _("Expected `=' following !LET"));
       return 0;
     }
   p++;
@@ -1664,7 +1691,7 @@ find_doend (const struct macro_expansion_stack *stack,
           nesting--;
         }
     }
-  macro_error (stack, _("Missing !DOEND."));
+  macro_error (stack, NULL, _("Missing !DOEND."));
   return NULL;
 }
 
@@ -1685,16 +1712,16 @@ macro_expand_do (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 !DO"));
+      macro_error (stack, p < end ? p : NULL,
+                   _("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))
     {
-      macro_error (stack, _("Cannot use argument name or macro "
-                            "keyword \"%.*s\" as !DO variable"),
-                   (int) var_name.length, var_name.string);
+      macro_error (stack, p, _("Cannot use argument name or macro "
+                               "keyword as !DO variable"));
       return 0;
     }
   p++;
@@ -1733,9 +1760,10 @@ macro_expand_do (const struct macro_token *tokens, size_t n_tokens,
         {
           if (i >= miterate)
             {
-              macro_error (stack, _("!DO loop over list exceeded "
-                                    "maximum number of iterations %d.  "
-                                    "(Use SET MITERATE to change the limit.)"),
+              macro_error (stack, NULL,
+                           _("!DO loop over list exceeded "
+                             "maximum number of iterations %d.  "
+                             "(Use SET MITERATE to change the limit.)"),
                            miterate);
               break;
             }
@@ -1761,7 +1789,8 @@ 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")))
         {
-          macro_error (stack, _("Expected !TO in numerical !DO loop"));
+          macro_error (stack, p < end ? p : NULL,
+                       _("Expected !TO in numerical !DO loop"));
           return 0;
         }
       p++;
@@ -1782,7 +1811,7 @@ macro_expand_do (const struct macro_token *tokens, size_t n_tokens,
 
           if (by == 0.0)
             {
-              macro_error (stack, _("!BY value cannot be zero."));
+              macro_error (stack, NULL, _("!BY value cannot be zero."));
               return 0;
             }
         }
@@ -1804,7 +1833,7 @@ macro_expand_do (const struct macro_token *tokens, size_t n_tokens,
             {
               if (i++ > miterate)
                 {
-                  macro_error (stack,
+                  macro_error (stack, NULL,
                                _("Numerical !DO loop exceeded "
                                  "maximum number of iterations %d.  "
                                  "(Use SET MITERATE to change the limit.)"),
@@ -1829,7 +1858,8 @@ macro_expand_do (const struct macro_token *tokens, size_t n_tokens,
     }
   else
     {
-      macro_error (stack, _("Expected `=' or !IN in !DO loop."));
+      macro_error (stack, p < end ? p : NULL,
+                   _("Expected `=' or !IN in !DO loop."));
       return 0;
     }
 }
@@ -1843,7 +1873,7 @@ macro_expand (const struct macro_tokens *mts,
 {
   if (nesting_countdown <= 0)
     {
-      macro_error (stack, _("Maximum nesting level %d exceeded."),
+      macro_error (stack, NULL, _("Maximum nesting level %d exceeded."),
                    settings_get_mnest ());
       for (size_t i = 0; i < mts->n; i++)
         macro_tokens_add (exp, &mts->mts[i]);
@@ -1960,7 +1990,7 @@ macro_expand (const struct macro_tokens *mts,
       if (ss_equals_case (token->string, ss_cstr ("!break")))
         {
           if (!break_)
-            macro_error (stack, _("!BREAK outside !DO."));
+            macro_error (stack, mt, _("!BREAK outside !DO."));
           else
             {
               *break_ = true;