From: Ben Pfaff Date: Thu, 17 Dec 2020 07:45:30 +0000 (-0800) Subject: expressions: Improve error messages. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ddd596e860006f3064dea11efe8d66d9d73659bd;p=pspp expressions: Improve error messages. Also, introduce tests to trigger the error messages. Prompted by bug #59697. Thanks to Stéphane Aulery for reporting the poor error messages. --- diff --git a/src/language/expressions/generate.pl b/src/language/expressions/generate.pl index 228095db94..6f41d7e842 100644 --- a/src/language/expressions/generate.pl +++ b/src/language/expressions/generate.pl @@ -561,8 +561,8 @@ sub parse_arg { $arg{IDX} = force ('id'); if (match ('*')) { $arg{TIMES} = force ('int'); - die "multiplication factor must be positive" - if $arg{TIMES} < 1; + die "multiplication factor must be two" + if $arg{TIMES} != 2; } else { $arg{TIMES} = 1; } diff --git a/src/language/expressions/parse.c b/src/language/expressions/parse.c index 9064236507..79889082ea 100644 --- a/src/language/expressions/parse.c +++ b/src/language/expressions/parse.c @@ -1096,23 +1096,21 @@ coerce_function_args (struct expression *e, const struct operation *f, static bool validate_function_args (const struct operation *f, int arg_cnt, int min_valid) { + /* Count the function arguments that go into the trailing array (if any). We + know that there must be at least the minimum number because + match_function() already checked. */ int array_arg_cnt = arg_cnt - (f->arg_cnt - 1); - if (array_arg_cnt < f->array_min_elems) - { - msg (SE, _("%s must have at least %d arguments in list."), - f->prototype, f->array_min_elems); - return false; - } + assert (array_arg_cnt >= f->array_min_elems); if ((f->flags & OPF_ARRAY_OPERAND) && array_arg_cnt % f->array_granularity != 0) { - if (f->array_granularity == 2) - msg (SE, _("%s must have an even number of arguments in list."), - f->prototype); - else - msg (SE, _("%s must have multiple of %d arguments in list."), - f->prototype, f->array_granularity); + /* RANGE is the only case we have so far. It has paired arguments with + one initial argument, and that's the only special case we deal with + here. */ + assert (f->array_granularity == 2); + assert (arg_cnt % 2 == 0); + msg (SE, _("%s must have an odd number of arguments."), f->prototype); return false; } @@ -1121,26 +1119,19 @@ validate_function_args (const struct operation *f, int arg_cnt, int min_valid) if (f->array_min_elems == 0) { assert ((f->flags & OPF_MIN_VALID) == 0); - msg (SE, _("%s function does not accept a minimum valid " - "argument count."), f->prototype); + msg (SE, _("%s function cannot accept suffix .%d to specify the " + "minimum number of valid arguments."), + f->prototype, min_valid); return false; } else { assert (f->flags & OPF_MIN_VALID); - if (array_arg_cnt < f->array_min_elems) - { - msg (SE, _("%s requires at least %d valid arguments in list."), - f->prototype, f->array_min_elems); - return false; - } - else if (min_valid > array_arg_cnt) + if (min_valid > array_arg_cnt) { - msg (SE, _("With %s, " - "using minimum valid argument count of %d " - "does not make sense when passing only %d " - "arguments in list."), - f->prototype, min_valid, array_arg_cnt); + msg (SE, _("For %s with %d arguments, at most %d (not %d) may be " + "required to be valid."), + f->prototype, arg_cnt, array_arg_cnt, min_valid); return false; } } diff --git a/tests/language/expressions/evaluate.at b/tests/language/expressions/evaluate.at index d9e532e1d3..ac4d93e704 100644 --- a/tests/language/expressions/evaluate.at +++ b/tests/language/expressions/evaluate.at @@ -15,6 +15,7 @@ dnl You should have received a copy of the GNU General Public License dnl along with this program. If not, see . m4_define([CHECK_EXPR_EVAL], [AT_SETUP([expressions - $1]) + AT_KEYWORDS([expression]) AT_DATA([evaluate.sps], [set mxwarn 1000. set mxerr 1000. @@ -562,11 +563,11 @@ ANY(string, string[, string]...).]], RANGE(number, number, number[, number, number]...) RANGE(string, string, string[, string, string]...).]], [[range(1, 2)], [error], - [error: DEBUG EVALUATE: RANGE(number, number, number[, number, number]...) must have an even number of arguments in list.]], + [error: DEBUG EVALUATE: RANGE(number, number, number[, number, number]...) must have an odd number of arguments.]], [[range(1, 2, 3, 4)], [error], - [error: DEBUG EVALUATE: RANGE(number, number, number[, number, number]...) must have an even number of arguments in list.]], + [error: DEBUG EVALUATE: RANGE(number, number, number[, number, number]...) must have an odd number of arguments.]], [[range(1, 2, 3, 4, 5, 6)], [error], - [error: DEBUG EVALUATE: RANGE(number, number, number[, number, number]...) must have an even number of arguments in list.]], + [error: DEBUG EVALUATE: RANGE(number, number, number[, number, number]...) must have an odd number of arguments.]], [[range('1', 2, 3)], [error], [error: DEBUG EVALUATE: Function invocation range(string, number, number) does not match any known function. Candidates are: RANGE(number, number, number[, number, number]...) @@ -605,11 +606,11 @@ RANGE(string, string, string[, string, string]...).]], RANGE(number, number, number[, number, number]...) RANGE(string, string, string[, string, string]...).]], [[range('1', '2')], [error], - [error: DEBUG EVALUATE: RANGE(string, string, string[, string, string]...) must have an even number of arguments in list.]], + [error: DEBUG EVALUATE: RANGE(string, string, string[, string, string]...) must have an odd number of arguments.]], [[range('1', '2', '3', '4')], [error], - [error: DEBUG EVALUATE: RANGE(string, string, string[, string, string]...) must have an even number of arguments in list.]], + [error: DEBUG EVALUATE: RANGE(string, string, string[, string, string]...) must have an odd number of arguments.]], [[range('1', '2', '3', '4', '5', '6')], [error], - [error: DEBUG EVALUATE: RANGE(string, string, string[, string, string]...) must have an even number of arguments in list.]], + [error: DEBUG EVALUATE: RANGE(string, string, string[, string, string]...) must have an odd number of arguments.]], [[range(1, '2', '3')], [error], [error: DEBUG EVALUATE: Function invocation range(number, string, string) does not match any known function. Candidates are: RANGE(number, number, number[, number, number]...) @@ -635,7 +636,7 @@ MAX(string[, string]...).]], [[max(1, 2, 3, $sysmis)], [3.00]], [[max.4(1, 2, 3, $sysmis)], [sysmis]], [[max.4(1, 2, 3)], [error], - [error: DEBUG EVALUATE: With MAX(number[, number]...), using minimum valid argument count of 4 does not make sense when passing only 3 arguments in list.]], + [error: DEBUG EVALUATE: For MAX(number[, number]...) with 3 arguments, at most 3 (not 4) may be required to be valid.]], [[max("2", "3", "5", "1", "4")], ["5"]], [[max("1", "2")], ["2"]], @@ -653,7 +654,7 @@ MIN(string[, string]...).]], [[min(1, 2, 3, $sysmis)], [1.00]], [[min.4(1, 2, 3, $sysmis)], [sysmis]], [[min.4(1, 2, 3)], [error], - [error: DEBUG EVALUATE: With MIN(number[, number]...), using minimum valid argument count of 4 does not make sense when passing only 3 arguments in list.]], + [error: DEBUG EVALUATE: For MIN(number[, number]...) with 3 arguments, at most 3 (not 4) may be required to be valid.]], [[min("2", "3", "5", "1", "4")], ["1"]], [[min("1", "2")], ["1"]], @@ -669,7 +670,7 @@ CHECK_EXPR_EVAL([cfvar mean median sd sum variance], [[cfvar(1, 2, 3, $sysmis)], [0.50]], [[cfvar.4(1, 2, 3, $sysmis)], [sysmis]], [[cfvar.4(1, 2, 3)], [error], - [error: DEBUG EVALUATE: With CFVAR(number, number[, number]...), using minimum valid argument count of 4 does not make sense when passing only 3 arguments in list.]], + [error: DEBUG EVALUATE: For CFVAR(number, number[, number]...) with 3 arguments, at most 3 (not 4) may be required to be valid.]], [[cfvar('x')], [error], [error: DEBUG EVALUATE: Type mismatch invoking CFVAR(number, number[, number]...) as cfvar(string).]], [[cfvar('x', 1, 2, 3)], [error], @@ -685,7 +686,7 @@ CHECK_EXPR_EVAL([cfvar mean median sd sum variance], [[mean(1, 2, 3, $sysmis)], [2.00]], [[mean.4(1, 2, 3, $sysmis)], [sysmis]], [[mean.4(1, 2, 3)], [error], - [error: DEBUG EVALUATE: With MEAN(number[, number]...), using minimum valid argument count of 4 does not make sense when passing only 3 arguments in list.]], + [error: DEBUG EVALUATE: For MEAN(number[, number]...) with 3 arguments, at most 3 (not 4) may be required to be valid.]], [[median(1, 2, 3, 4, 5)], [3.00]], [[median(2, 3, 4, 5, 1)], [3.00]], @@ -716,7 +717,7 @@ CHECK_EXPR_EVAL([cfvar mean median sd sum variance], [[sd(1, 2, 3, $sysmis)], [1.00]], [[sd.4(1, 2, 3, $sysmis)], [sysmis]], [[sd.4(1, 2, 3)], [error], - [error: DEBUG EVALUATE: With SD(number, number[, number]...), using minimum valid argument count of 4 does not make sense when passing only 3 arguments in list.]], + [error: DEBUG EVALUATE: For SD(number, number[, number]...) with 3 arguments, at most 3 (not 4) may be required to be valid.]], [[sd('x')], [error], [error: DEBUG EVALUATE: Type mismatch invoking SD(number, number[, number]...) as sd(string).]], [[sd('x', 1, 2, 3)], [error], @@ -732,7 +733,7 @@ CHECK_EXPR_EVAL([cfvar mean median sd sum variance], [[sum(1, 2, 3, $sysmis)], [6.00]], [[sum.4(1, 2, 3, $sysmis)], [sysmis]], [[sum.4(1, 2, 3)], [error], - [error: DEBUG EVALUATE: With SUM(number[, number]...), using minimum valid argument count of 4 does not make sense when passing only 3 arguments in list.]], + [error: DEBUG EVALUATE: For SUM(number[, number]...) with 3 arguments, at most 3 (not 4) may be required to be valid.]], [[variance(1, 2, 3, 4, 5)], [2.50]], [[variance(1, $sysmis, 2, 3, $sysmis, 4, 5)], [2.50]], @@ -743,7 +744,7 @@ CHECK_EXPR_EVAL([cfvar mean median sd sum variance], [[variance(1, 2, 3, $sysmis)], [1.00]], [[variance.4(1, 2, 3, $sysmis)], [sysmis]], [[variance.4(1, 2, 3)], [error], - [error: DEBUG EVALUATE: With VARIANCE(number, number[, number]...), using minimum valid argument count of 4 does not make sense when passing only 3 arguments in list.]], + [error: DEBUG EVALUATE: For VARIANCE(number, number[, number]...) with 3 arguments, at most 3 (not 4) may be required to be valid.]], [[variance('x')], [error], [error: DEBUG EVALUATE: Type mismatch invoking VARIANCE(number, number[, number]...) as variance(string).]], [[variance('x', 1, 2, 3)], [error], @@ -1986,6 +1987,14 @@ dnl Tests correctness of generic optimizations in optimize_tree(). [[x ** 1], [5.00], [], [(X = 5.00)]], [[x ** 2], [25.00], [], [(X = 5.00)]]) +CHECK_EXPR_EVAL([negative checks], + [[$nonexistent], [error], [error: DEBUG EVALUATE: Unknown system variable $nonexistent.]], + [[RANGE(1, 2)], [error], [error: DEBUG EVALUATE: RANGE(number, number, number[, number, number]...) must have an odd number of arguments.]], + [[CONCAT.1('a', 'b')], [error], [error: DEBUG EVALUATE: CONCAT(string[, string]...) function cannot accept suffix .1 to specify the minimum number of valid arguments.]], + [[foobar(x)], [error], [error: DEBUG EVALUATE: No function or vector named foobar.]], + [[CONCAT.1('a' b)], [error], [error: DEBUG EVALUATE: Syntax error at `b': expecting `,' or `)'.]], + [[NCDF.CHISQ(1, 2, 3)], [error], [error: DEBUG EVALUATE: NCDF.CHISQ(number, number, number) is not available in this version of PSPP.]]) + AT_SETUP([LAG function]) AT_DATA([lag.sps], [dnl data list /W 1. diff --git a/tests/language/expressions/parse.at b/tests/language/expressions/parse.at index cfa1db997e..e40a4fe873 100644 --- a/tests/language/expressions/parse.at +++ b/tests/language/expressions/parse.at @@ -38,6 +38,72 @@ parse.sps:11: error: Stopping syntax file processing here to avoid a cascade of ]) AT_CLEANUP +AT_SETUP([parsing numeric expression with type mismatch]) +AT_DATA([parse.sps], [dnl +DATA LIST NOTABLE/x 1(A). +IF 'foo'. +]) +AT_CHECK([pspp -O format=csv parse.sps], [1], [dnl +"parse.sps:2: error: IF: Type mismatch: expression has string type, but a numeric value is required here." +]) +AT_CLEANUP + +AT_SETUP([parsing string expression with type mismatch]) +AT_KEYWORDS([expression negative]) +AT_DATA([parse.sps], [dnl +DATA LIST NOTABLE/x 1(A). +COMPUTE x=1. +]) +AT_CHECK([pspp -O format=csv parse.sps], [1], [dnl +"parse.sps:2: error: COMPUTE: Type mismatch: expression has number type, but a string value is required here." +]) +AT_CLEANUP + +AT_SETUP([parse expression with unknown system variable]) +AT_KEYWORDS([expression negative]) +AT_DATA([parse.sps], [dnl +DATA LIST NOTABLE/x 1. +COMPUTE x=$nonexistent. +]) +AT_CHECK([pspp -O format=csv parse.sps], [1], [dnl +parse.sps:2: error: COMPUTE: Unknown system variable $nonexistent. +]) +AT_CLEANUP + +AT_SETUP([parse expression with unknown identifier]) +AT_KEYWORDS([expression negative]) +AT_DATA([parse.sps], [dnl +DATA LIST NOTABLE/x 1. +COMPUTE x=y. +]) +AT_CHECK([pspp -O format=csv parse.sps], [1], [dnl +parse.sps:2: error: COMPUTE: Unknown identifier y. +]) +AT_CLEANUP + +AT_SETUP([parse expression with extension function in compatibility mode]) +AT_KEYWORDS([expression negative]) +AT_DATA([parse.sps], [dnl +DEBUG EVALUATE/ACOS(0)*0. +]) +AT_CHECK([pspp --testing-mode --syntax=compatible -O format=csv parse.sps], [0], [dnl +parse.sps:1: warning: DEBUG EVALUATE: ACOS(number) is a PSPP extension. +0.00 +]) +AT_CLEANUP + +AT_SETUP([LAG expression following TEMPORARY]) +AT_KEYWORDS([expression negative]) +AT_DATA([parse.sps], [dnl +DATA LIST NOTABLE/x 1. +TEMPORARY +COMPUTE y=LAG(x). +]) +AT_CHECK([pspp -O format=csv parse.sps], [1], [dnl +parse.sps:3: error: COMPUTE: LAG(num_variable) may not appear after TEMPORARY. +]) +AT_CLEANUP + AT_SETUP([parse expression with invalid logical expression]) AT_DATA([parse.sps], [dnl INPUT PROGRAM.