From e3d8aa9d9873e60ba0e84e814c28f94a92ad0352 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Sun, 19 Dec 2021 15:57:38 -0800 Subject: [PATCH 1/1] error message improvement --- src/language/expressions/parse.c | 101 ++++++++++++++----------------- 1 file changed, 44 insertions(+), 57 deletions(-) diff --git a/src/language/expressions/parse.c b/src/language/expressions/parse.c index 838b211d90..3b18be72dd 100644 --- a/src/language/expressions/parse.c +++ b/src/language/expressions/parse.c @@ -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, " -- 2.30.2