error message improvement
authorBen Pfaff <blp@cs.stanford.edu>
Sun, 19 Dec 2021 23:57:38 +0000 (15:57 -0800)
committerBen Pfaff <blp@cs.stanford.edu>
Sun, 19 Dec 2021 23:57:38 +0000 (15:57 -0800)
src/language/expressions/parse.c

index 838b211d90cf7e04f028e92f07fcede402f4e470..3b18be72ddf81bce64bfbd9c23be0547259b0b42 100644 (file)
@@ -536,19 +536,19 @@ is_coercible (const struct expr_node *node_, size_t arg_idx)
   return type_coercion__ (NULL, node, arg_idx, false);
 }
 
-/* How to parse an operator. */
+/* How to parse an operator.
+
+   Some operators support both numeric and string operators.  For those,
+   'num_op' and 'str_op' are both nonzero.  Otherwise, only one 'num_op' is
+   nonzero.  (PSPP doesn't have any string-only operators.) */
 struct operator
   {
     enum token_type token;      /* Operator token. */
-    operation_type type;        /* Operation. */
+    operation_type num_op;      /* Operation for numeric operands (or 0). */
+    operation_type str_op;      /* Operation for string operands (or 0). */
   };
 
-/* Attempts to match the current token against the tokens for the
-   OP_CNT operators in OPS[].  If successful, returns true
-   and, if OPERATOR is non-null, sets *OPERATOR to the operator.
-   On failure, returns false and, if OPERATOR is non-null, sets
-   *OPERATOR to a null pointer. */
-static const struct operator *
+static operation_type
 match_operator (struct lexer *lexer, const struct operator ops[], size_t n_ops,
                 const struct expr_node *lhs)
 {
@@ -556,21 +556,18 @@ match_operator (struct lexer *lexer, const struct operator ops[], size_t n_ops,
   for (const struct operator *op = ops; op < ops + n_ops; op++)
     if (lex_token (lexer) == op->token)
       {
-        bool op_is_numeric = operations[op->type].args[0] != OP_string;
-        if (op_is_numeric == lhs_is_numeric)
-          {
-            if (op->token != T_NEG_NUM)
-              lex_get (lexer);
-            return op;
-          }
+        if (op->token != T_NEG_NUM)
+          lex_get (lexer);
+
+        return op->str_op && !lhs_is_numeric ? op->str_op : op->num_op;
       }
-  return NULL;
+  return 0;
 }
 
 static const char *
-operator_name (const struct operator *op)
+operator_name (enum token_type token)
 {
-  return op->token == T_NEG_NUM ? "-" : token_type_to_string (op->token);
+  return token == T_NEG_NUM ? "-" : token_type_to_string (token);
 }
 
 static struct expr_node *
@@ -581,8 +578,9 @@ parse_binary_operators__ (struct lexer *lexer, struct expression *e,
 {
   for (int op_count = 0; ; op_count++)
     {
-      const struct operator *operator = match_operator (lexer, ops, n_ops, lhs);
-      if (!operator)
+      enum token_type token = lex_token (lexer);
+      operation_type optype = match_operator (lexer, ops, n_ops, lhs);
+      if (!optype)
         {
           if (op_count > 1 && chain_warning)
             msg_at (SW, expr_location (e, lhs), "%s", chain_warning);
@@ -594,22 +592,21 @@ parse_binary_operators__ (struct lexer *lexer, struct expression *e,
       if (!rhs)
         return NULL;
 
-      struct expr_node *node = expr_allocate_binary (e, operator->type,
-                                                     lhs, rhs);
+      struct expr_node *node = expr_allocate_binary (e, optype, lhs, rhs);
       bool lhs_ok = type_coercion (e, node, 0);
       bool rhs_ok = type_coercion (e, node, 1);
 
       if (!lhs_ok || !rhs_ok)
         {
-          int n_matches = 0;
+          bool both = false;
           for (size_t i = 0; i < n_ops; i++)
-            if (ops[i].token == operator->token)
-              n_matches++;
+            if (ops[i].token == token)
+              both = ops[i].num_op && ops[i].str_op;
 
-          const char *name = operator_name (operator);
-          if (n_matches > 1)
+          const char *name = operator_name (token);
+          if (both)
             msg_at (SE, expr_location (e, node),
-                    _("The operands of %s must have the same type."), name);
+                    _("Both operands of %s must have the same type."), name);
           else if (operations[node->type].args[0] != OP_string)
             msg_at (SE, expr_location (e, node),
                     _("Both operands of %s must be numeric."), name);
@@ -659,14 +656,14 @@ parse_inverting_unary_operator (struct lexer *lexer, struct expression *e,
   if (!inner || !op_count)
     return inner;
 
-  struct expr_node *outer = expr_allocate_unary (e, op->type, inner);
+  struct expr_node *outer = expr_allocate_unary (e, op->num_op, inner);
   expr_add_location (lexer, e, start_ofs, outer);
 
   if (!type_coercion (e, outer, 0))
     {
       assert (operations[outer->type].args[0] != OP_string);
 
-      const char *name = operator_name (op);
+      const char *name = operator_name (op->token);
       msg_at (SE, expr_location (e, outer),
               _("The unary %s operator requires a numeric operand."), name);
 
@@ -684,7 +681,7 @@ parse_inverting_unary_operator (struct lexer *lexer, struct expression *e,
 static struct expr_node *
 parse_or (struct lexer *lexer, struct expression *e)
 {
-  static const struct operator op = { T_OR, OP_OR };
+  static const struct operator op = { .token = T_OR, .num_op = OP_OR };
   return parse_binary_operators (lexer, e, &op, 1, parse_and, NULL);
 }
 
@@ -692,7 +689,7 @@ parse_or (struct lexer *lexer, struct expression *e)
 static struct expr_node *
 parse_and (struct lexer *lexer, struct expression *e)
 {
-  static const struct operator op = { T_AND, OP_AND };
+  static const struct operator op = { .token = T_AND, .num_op = OP_AND };
 
   return parse_binary_operators (lexer, e, &op, 1, parse_not, NULL);
 }
@@ -701,7 +698,7 @@ parse_and (struct lexer *lexer, struct expression *e)
 static struct expr_node *
 parse_not (struct lexer *lexer, struct expression *e)
 {
-  static const struct operator op = { T_NOT, OP_NOT };
+  static const struct operator op = { .token = T_NOT, .num_op = OP_NOT };
   return parse_inverting_unary_operator (lexer, e, &op, parse_rel);
 }
 
@@ -719,23 +716,13 @@ parse_rel (struct lexer *lexer, struct expression *e)
 
   static const struct operator ops[] =
     {
-      /* Numeric operators. */
-      { T_EQUALS, OP_EQ },
-      { T_EQ, OP_EQ },
-      { T_GE, OP_GE },
-      { T_GT, OP_GT },
-      { T_LE, OP_LE },
-      { T_LT, OP_LT },
-      { T_NE, OP_NE },
-
-      /* String operators. */
-      { T_EQUALS, OP_EQ_STRING },
-      { T_EQ, OP_EQ_STRING },
-      { T_GE, OP_GE_STRING },
-      { T_GT, OP_GT_STRING },
-      { T_LE, OP_LE_STRING },
-      { T_LT, OP_LT_STRING },
-      { T_NE, OP_NE_STRING },
+      { .token = T_EQUALS, .num_op = OP_EQ, .str_op = OP_EQ_STRING },
+      { .token = T_EQ, .num_op = OP_EQ, .str_op = OP_EQ_STRING },
+      { .token = T_GE, .num_op = OP_GE, .str_op = OP_GE_STRING },
+      { .token = T_GT, .num_op = OP_GT, .str_op = OP_GT_STRING },
+      { .token = T_LE, .num_op = OP_LE, .str_op = OP_LE_STRING },
+      { .token = T_LT, .num_op = OP_LT, .str_op = OP_LT_STRING },
+      { .token = T_NE, .num_op = OP_NE, .str_op = OP_NE_STRING },
     };
 
   return parse_binary_operators (lexer, e, ops, sizeof ops / sizeof *ops,
@@ -748,9 +735,9 @@ parse_add (struct lexer *lexer, struct expression *e)
 {
   static const struct operator ops[] =
     {
-      { T_PLUS, OP_ADD },
-      { T_DASH, OP_SUB },
-      { T_NEG_NUM, OP_ADD },
+      { .token = T_PLUS, .num_op = OP_ADD },
+      { .token = T_DASH, .num_op = OP_SUB },
+      { .token = T_NEG_NUM, .num_op = OP_ADD },
     };
 
   return parse_binary_operators (lexer, e, ops, sizeof ops / sizeof *ops,
@@ -763,8 +750,8 @@ parse_mul (struct lexer *lexer, struct expression *e)
 {
   static const struct operator ops[] =
     {
-      { T_ASTERISK, OP_MUL },
-      { T_SLASH, OP_DIV },
+      { .token = T_ASTERISK, .num_op = OP_MUL },
+      { .token = T_SLASH, .num_op = OP_DIV },
     };
 
   return parse_binary_operators (lexer, e, ops, sizeof ops / sizeof *ops,
@@ -775,14 +762,14 @@ parse_mul (struct lexer *lexer, struct expression *e)
 static struct expr_node *
 parse_neg (struct lexer *lexer, struct expression *e)
 {
-  static const struct operator op = { T_DASH, OP_NEG };
+  static const struct operator op = { .token = T_DASH, .num_op = OP_NEG };
   return parse_inverting_unary_operator (lexer, e, &op, parse_exp);
 }
 
 static struct expr_node *
 parse_exp (struct lexer *lexer, struct expression *e)
 {
-  static const struct operator op = { T_EXP, OP_POW };
+  static const struct operator op = { .token = T_EXP, .num_op = OP_POW };
 
   const char *chain_warning =
     _("The exponentiation operator (`**') is left-associative, "