From 00d5ccd2b3072493b84dbf85b40467ef0a2c1df8 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Sun, 11 Sep 2022 22:10:40 -0700 Subject: [PATCH] Improve error messages for format specifiers. --- src/data/format.c | 96 +++++++++++++----------- src/data/format.h | 2 + src/data/por-file-reader.c | 42 +++++------ src/language/data-io/data-list.c | 9 ++- src/language/data-io/get-data.c | 48 +++++++----- src/language/data-io/placement-parser.c | 50 +++++++----- src/language/data-io/print.c | 72 +++++++++--------- src/language/dictionary/formats.c | 37 ++++----- src/language/dictionary/numeric.c | 33 ++++---- src/language/dictionary/vector.c | 10 ++- src/language/expressions/evaluate.c | 13 +++- src/language/expressions/parse.c | 6 -- src/language/stats/ctables.c | 17 ++++- src/language/stats/matrix.c | 35 ++++++++- src/language/tests/format-guesser-test.c | 2 - src/language/utilities/set.c | 9 ++- src/output/spv/spv.c | 2 - src/ui/gui/var-type-dialog.c | 2 - tests/language/dictionary/formats.at | 16 +++- tests/language/stats/ctables.at | 6 +- tests/language/stats/matrix.at | 8 ++ tests/language/utilities/set.at | 4 +- 22 files changed, 319 insertions(+), 200 deletions(-) diff --git a/src/data/format.c b/src/data/format.c index 8c1c467398..e999636f03 100644 --- a/src/data/format.c +++ b/src/data/format.c @@ -451,12 +451,23 @@ fmt_check__ (const struct fmt_spec *spec, enum fmt_use use) return NULL; } +char * +fmt_check_input__ (const struct fmt_spec *spec) +{ + return fmt_check__ (spec, FMT_FOR_INPUT); +} + +char * +fmt_check_output__ (const struct fmt_spec *spec) +{ + return fmt_check__ (spec, FMT_FOR_OUTPUT); +} + static bool -fmt_emit_and_free_error (char *error) +error_to_bool (char *error) { if (error) { - msg (SE, "%s", error); free (error); return false; } @@ -464,25 +475,21 @@ fmt_emit_and_free_error (char *error) return true; } -/* Checks whether SPEC is valid for USE and returns nonzero if so. Otherwise, - emits an error message for the current source location and returns zero. */ +/* Returns true if SPEC is valid for USE, false otherwise. */ bool fmt_check (const struct fmt_spec *spec, enum fmt_use use) { - return fmt_emit_and_free_error (fmt_check__ (spec, use)); + return error_to_bool (fmt_check__ (spec, use)); } -/* Checks whether SPEC is valid as an input format and returns - nonzero if so. Otherwise, emits an error message and returns - zero. */ +/* Returns true if SPEC is valid as an input format, otherwise false. */ bool fmt_check_input (const struct fmt_spec *spec) { return fmt_check (spec, FMT_FOR_INPUT); } -/* Checks whether SPEC is valid as an output format and returns - true if so. Otherwise, emits an error message and returns false. */ +/* Returnst true SPEC is valid as an output format, false otherwise. */ bool fmt_check_output (const struct fmt_spec *spec) { @@ -491,7 +498,7 @@ fmt_check_output (const struct fmt_spec *spec) /* Checks that FORMAT is appropriate for a variable of the given VAR_TYPE and returns NULL if so. Otherwise returns a malloc()'d error message that the - calelr must eventually free(). */ + caller must eventually free(). */ char * fmt_check_type_compat__ (const struct fmt_spec *format, enum val_type var_type) { @@ -499,21 +506,23 @@ fmt_check_type_compat__ (const struct fmt_spec *format, enum val_type var_type) if ((var_type == VAL_STRING) != (fmt_is_string (format->type) != 0)) { char str[FMT_STRING_LEN_MAX + 1]; - return xasprintf (_("%s variables are not compatible with %s format %s."), - var_type == VAL_STRING ? _("String") : _("Numeric"), - var_type == VAL_STRING ? _("numeric") : _("string"), - fmt_to_string (format, str)); + fmt_to_string (format, str); + if (var_type == VAL_STRING) + return xasprintf (_("String variables are not compatible with " + "numeric format %s."), str); + else + return xasprintf (_("Numeric variables are not compatible with " + "string format %s."), str); } return NULL; } -/* Checks that FORMAT is appropriate for a variable of the given - VAR_TYPE and returns true if so. Otherwise returns false and - emits an error message. */ +/* Returns FORMAT is appropriate for a variable of the given + VAR_TYPE and returns true if so, otherwise false. */ bool fmt_check_type_compat (const struct fmt_spec *format, enum val_type var_type) { - return fmt_emit_and_free_error (fmt_check_type_compat__ (format, var_type)); + return error_to_bool (fmt_check_type_compat__ (format, var_type)); } /* Checks that FORMAT is appropriate for a variable of the given WIDTH and @@ -533,28 +542,33 @@ fmt_check_width_compat__ (const struct fmt_spec *format, const char *varname, char format_str[FMT_STRING_LEN_MAX + 1]; fmt_to_string (format, format_str); + char better_str[FMT_STRING_LEN_MAX + 1]; + if (format->type == FMT_A) + snprintf (better_str, sizeof better_str, "A%d", width); + else + snprintf (better_str, sizeof better_str, "AHEX%d", width * 2); + if (varname) return xasprintf (_("String variable %s with width %d is not " - "compatible with format %s."), - varname, width, format_str); - else + "compatible with format %s. " + "Use format %s instead."), + varname, width, format_str, better_str); + else return xasprintf (_("String variable with width %d is not compatible " - "with format %s."), - width, format_str); + "with format %s. Use format %s instead."), + width, format_str, better_str); } return NULL; } /* Checks that FORMAT is appropriate for a variable of the given WIDTH and - returns true if so. Otherwise returns false and emits an error message. - VARNAME is optional and only used in the error message. */ + returns true if so, otherwise false. */ bool fmt_check_width_compat (const struct fmt_spec *format, const char *varname, int width) { - return fmt_emit_and_free_error (fmt_check_width_compat__ (format, varname, - width)); + return error_to_bool (fmt_check_width_compat__ (format, varname, width)); } /* Returns the width corresponding to FORMAT. The return value @@ -1019,22 +1033,18 @@ fmt_from_u32 (uint32_t u32, int width, bool loose, struct fmt_spec *f) uint8_t w = u32 >> 8; uint8_t d = u32; - msg_disable (); - f->w = w; - f->d = d; - bool ok = fmt_from_io (raw_type, &f->type); - if (ok) - { - if (loose) - fmt_fix_output (f); - else - ok = fmt_check_output (f); - } - if (ok) - ok = fmt_check_width_compat (f, NULL, width); - msg_enable (); + enum fmt_type type; + if (!fmt_from_io (raw_type, &type)) + return false; + + *f = (struct fmt_spec) { .type = type, .w = w, .d = d }; + + if (loose) + fmt_fix_output (f); + else if (!fmt_check_output (f)) + return false; - return ok; + return fmt_check_width_compat (f, NULL, width); } /* Returns true if TYPE may be used as an input format, diff --git a/src/data/format.h b/src/data/format.h index 5fe6c101f8..5b56e12201 100644 --- a/src/data/format.h +++ b/src/data/format.h @@ -99,6 +99,8 @@ bool fmt_check_width_compat (const struct fmt_spec *, const char *varname, int var_width); char *fmt_check__ (const struct fmt_spec *, enum fmt_use); +char *fmt_check_input__ (const struct fmt_spec *); +char *fmt_check_output__ (const struct fmt_spec *); char *fmt_check_type_compat__ (const struct fmt_spec *, enum val_type); char *fmt_check_width_compat__ (const struct fmt_spec *, const char *varname, int var_width); diff --git a/src/data/por-file-reader.c b/src/data/por-file-reader.c index 9ae6799ac4..9bea44ffa4 100644 --- a/src/data/por-file-reader.c +++ b/src/data/por-file-reader.c @@ -623,29 +623,20 @@ static struct fmt_spec convert_format (struct pfm_reader *r, const int portable_format[3], struct variable *v, bool *report_error) { - struct fmt_spec format; - bool ok; - - if (!fmt_from_io (portable_format[0], &format.type)) + enum fmt_type type; + if (fmt_from_io (portable_format[0], &type)) { - if (*report_error) - warning (r, _("%s: Bad format specifier byte (%d). Variable " - "will be assigned a default format."), - var_get_name (v), portable_format[0]); - goto assign_default; - } + struct fmt_spec format = { + .type = type, + .w = portable_format[1], + .d = portable_format[2], + }; - format.w = portable_format[1]; - format.d = portable_format[2]; + if (fmt_check_output (&format) + && fmt_check_width_compat (&format, var_get_name (v), + var_get_width (v))) + return format; - msg_disable (); - ok = (fmt_check_output (&format) - && fmt_check_width_compat (&format, var_get_name (v), - var_get_width (v))); - msg_enable (); - - if (!ok) - { if (*report_error) { char fmt_string[FMT_STRING_LEN_MAX + 1]; @@ -659,12 +650,15 @@ convert_format (struct pfm_reader *r, const int portable_format[3], "invalid format specifier %s."), var_get_name (v), var_get_width (v), fmt_string); } - goto assign_default; + } + else + { + if (*report_error) + warning (r, _("%s: Bad format specifier byte (%d). Variable " + "will be assigned a default format."), + var_get_name (v), portable_format[0]); } - return format; - -assign_default: *report_error = false; return fmt_default_for_width (var_get_width (v)); } diff --git a/src/language/data-io/data-list.c b/src/language/data-io/data-list.c index 546e33f21e..af24b373cb 100644 --- a/src/language/data-io/data-list.c +++ b/src/language/data-io/data-list.c @@ -464,7 +464,14 @@ parse_free (struct lexer *lexer, struct dictionary *dict, input.d = 0; } - if (!fmt_check_input (&input) || !lex_force_match (lexer, T_RPAREN)) + char *error = fmt_check_input__ (&input); + if (error) + { + lex_next_error (lexer, -1, -1, "%s", error); + free (error); + return NULL; + } + if (!lex_force_match (lexer, T_RPAREN)) return NULL; /* As a special case, N format is treated as F format diff --git a/src/language/data-io/get-data.c b/src/language/data-io/get-data.c index cf96e415ad..4474118166 100644 --- a/src/language/data-io/get-data.c +++ b/src/language/data-io/get-data.c @@ -579,9 +579,13 @@ parse_get_txt (struct lexer *lexer, struct dataset *ds) lex_get (lexer); if (type == DP_DELIMITED) { - if (!parse_format_specifier (lexer, &input) - || !fmt_check_input (&input)) + if (!parse_format_specifier (lexer, &input)) + goto error; + error = fmt_check_input__ (&input); + if (error) { + lex_next_error (lexer, -1, -1, "%s", error); + free (error); goto error; } output = fmt_for_output_from_input (&input, @@ -589,37 +593,47 @@ parse_get_txt (struct lexer *lexer, struct dataset *ds) } else { - char fmt_type_name[FMT_TYPE_LEN_MAX + 1]; - enum fmt_type fmt_type; - uint16_t w; - uint8_t d; - + int start_ofs = lex_ofs (lexer); if (!parse_column_range (lexer, 0, &fc, &lc, NULL)) goto error; /* Accept a format (e.g. F8.2) or just a type name (e.g. DOLLAR). */ + char fmt_type_name[FMT_TYPE_LEN_MAX + 1]; + uint16_t w; + uint8_t d; if (!parse_abstract_format_specifier (lexer, fmt_type_name, &w, &d)) goto error; + + enum fmt_type fmt_type; if (!fmt_from_name (fmt_type_name, &fmt_type)) { lex_next_error (lexer, -1, -1, _("Unknown format type `%s'."), fmt_type_name); goto error; } + int end_ofs = lex_ofs (lexer) - 1; + /* Compose input format. */ - input.type = fmt_type; - input.w = lc - fc + 1; - input.d = 0; - if (!fmt_check_input (&input)) - goto error; + input = (struct fmt_spec) { .type = fmt_type, .w = lc - fc + 1 }; + error = fmt_check_input__ (&input); + if (error) + { + lex_next_error (lexer, start_ofs, end_ofs, "%s", error); + free (error); + goto error; + } + /* Compose output format. */ if (w != 0) { - output.type = fmt_type; - output.w = w; - output.d = d; - if (!fmt_check_output (&output)) - goto error; + output = (struct fmt_spec) { .type = fmt_type, .w = w, .d = d }; + error = fmt_check_output__ (&output); + if (error) + { + lex_next_error (lexer, start_ofs, end_ofs, "%s", error); + free (error); + goto error; + } } else output = fmt_for_output_from_input (&input, diff --git a/src/language/data-io/placement-parser.c b/src/language/data-io/placement-parser.c index c56e089a96..c789a16b52 100644 --- a/src/language/data-io/placement-parser.c +++ b/src/language/data-io/placement-parser.c @@ -115,15 +115,14 @@ fixed_parse_columns (struct lexer *lexer, struct pool *pool, size_t n_vars, enum fmt_use use, struct fmt_spec **formats, size_t *n_formats) { - struct fmt_spec format; - int fc, lc; - size_t i; + int start_ofs = lex_ofs (lexer); + int fc, lc; if (!parse_column_range (lexer, 1, &fc, &lc, NULL)) return false; /* Divide columns evenly. */ - format.w = (lc - fc + 1) / n_vars; + int w = (lc - fc + 1) / n_vars; if ((lc - fc + 1) % n_vars) { msg (SE, _("The %d columns %d-%d " @@ -133,43 +132,53 @@ fixed_parse_columns (struct lexer *lexer, struct pool *pool, size_t n_vars, } /* Format specifier. */ + enum fmt_type type; + int d; if (lex_match (lexer, T_LPAREN)) { /* Get format type. */ if (lex_token (lexer) == T_ID) { - if (!parse_format_specifier_name (lexer, &format.type)) + if (!parse_format_specifier_name (lexer, &type)) return false; lex_match (lexer, T_COMMA); } else - format.type = FMT_F; + type = FMT_F; /* Get decimal places. */ if (lex_is_integer (lexer)) { - format.d = lex_integer (lexer); + d = lex_integer (lexer); lex_get (lexer); } else - format.d = 0; + d = 0; if (!lex_force_match (lexer, T_RPAREN)) return false; } else { - format.type = FMT_F; - format.d = 0; + type = FMT_F; + d = 0; + } + int end_ofs = lex_ofs (lexer) - 1; + + struct fmt_spec format = { .type = type, .w = w, .d = d }; + char *error = fmt_check__ (&format, use); + if (error) + { + lex_ofs_error (lexer, start_ofs, end_ofs, "%s", error); + free (error); + return false; } - if (!fmt_check (&format, use)) - return false; *formats = pool_nalloc (pool, n_vars + 1, sizeof **formats); *n_formats = n_vars + 1; (*formats)[0].type = (enum fmt_type) PRS_TYPE_T; (*formats)[0].w = fc; - for (i = 1; i <= n_vars; i++) + for (size_t i = 1; i <= n_vars; i++) (*formats)[i] = format; return true; } @@ -216,8 +225,8 @@ fixed_parse_fortran (struct lexer *lexer, struct pool *pool, enum fmt_use use, f.type = (enum fmt_type) PRS_TYPE_NEW_REC; else { + int ofs = lex_ofs (lexer); char type[FMT_TYPE_LEN_MAX + 1]; - if (!parse_abstract_format_specifier (lexer, type, &f.w, &f.d)) return false; @@ -233,12 +242,17 @@ fixed_parse_fortran (struct lexer *lexer, struct pool *pool, enum fmt_use use, { if (!fmt_from_name (type, &f.type)) { - lex_next_error (lexer, -1, -1, - _("Unknown format type `%s'."), type); + lex_ofs_error (lexer, ofs, ofs, + _("Unknown format type `%s'."), type); + return false; + } + char *error = fmt_check__ (&f, use); + if (error) + { + lex_ofs_error (lexer, ofs, ofs, "%s", error); + free (error); return false; } - if (!fmt_check (&f, use)) - return false; } } } diff --git a/src/language/data-io/print.c b/src/language/data-io/print.c index 906730dce3..91d2d9c180 100644 --- a/src/language/data-io/print.c +++ b/src/language/data-io/print.c @@ -356,15 +356,15 @@ parse_variable_argument (struct lexer *lexer, const struct dictionary *dict, enum which_formats which_formats) { const struct variable **vars; - size_t n_vars, var_idx; - struct fmt_spec *formats, *f; - size_t n_formats; - bool add_space; - + size_t n_vars; if (!parse_variables_const_pool (lexer, tmp_pool, dict, - &vars, &n_vars, PV_DUPLICATE)) + &vars, &n_vars, PV_DUPLICATE)) return false; + struct fmt_spec *formats, *f; + size_t n_formats; + bool add_space; + int formats_start = lex_ofs (lexer); if (lex_is_number (lexer) || lex_token (lexer) == T_LPAREN) { if (!parse_var_placements (lexer, tmp_pool, n_vars, FMT_FOR_OUTPUT, @@ -374,13 +374,11 @@ parse_variable_argument (struct lexer *lexer, const struct dictionary *dict, } else { - size_t i; - lex_match (lexer, T_ASTERISK); formats = pool_nmalloc (tmp_pool, n_vars, sizeof *formats); n_formats = n_vars; - for (i = 0; i < n_vars; i++) + for (size_t i = 0; i < n_vars; i++) { const struct variable *v = vars[i]; formats[i] = (which_formats == PRINT @@ -389,36 +387,40 @@ parse_variable_argument (struct lexer *lexer, const struct dictionary *dict, } add_space = which_formats == PRINT; } + int formats_end = lex_ofs (lexer) - 1; - var_idx = 0; + size_t var_idx = 0; for (f = formats; f < &formats[n_formats]; f++) if (!execute_placement_format (f, record, column)) { - const struct variable *var; - struct prt_out_spec *spec; - - var = vars[var_idx++]; - if (!fmt_check_width_compat (f, var_get_name (var), - var_get_width (var))) - return false; - - spec = pool_alloc (trns->pool, sizeof *spec); - spec->type = PRT_VAR; - spec->record = *record; - spec->first_column = *column; - spec->var = var; - spec->format = *f; - spec->add_space = add_space; - - /* This is a completely bizarre twist for compatibility: - WRITE outputs the system-missing value as a field - filled with spaces, instead of using the normal format - that usually contains a period. */ - spec->sysmis_as_spaces = (which_formats == WRITE - && var_is_numeric (var) - && (fmt_get_category (spec->format.type) - != FMT_CAT_BINARY)); - + const struct variable *var = vars[var_idx++]; + char *error = fmt_check_width_compat__ (f, var_get_name (var), + var_get_width (var)); + if (error) + { + lex_ofs_error (lexer, formats_start, formats_end, "%s", error); + free (error); + return false; + } + + struct prt_out_spec *spec = pool_alloc (trns->pool, sizeof *spec); + *spec = (struct prt_out_spec) { + .type = PRT_VAR, + .record = *record, + .first_column = *column, + .var = var, + .format = *f, + .add_space = add_space, + + /* This is a completely bizarre twist for compatibility: WRITE + outputs the system-missing value as a field filled with spaces, + instead of using the normal format that usually contains a + period. */ + .sysmis_as_spaces = (which_formats == WRITE + && var_is_numeric (var) + && (fmt_get_category (f->type) + != FMT_CAT_BINARY)), + }; ll_push_tail (&trns->specs, &spec->ll); *column += f->w + add_space; diff --git a/src/language/dictionary/formats.c b/src/language/dictionary/formats.c index b2cc3a5c39..907a47f9ae 100644 --- a/src/language/dictionary/formats.c +++ b/src/language/dictionary/formats.c @@ -34,34 +34,30 @@ #include "gettext.h" #define _(msgid) gettext (msgid) -enum - { - FORMATS_PRINT = 001, - FORMATS_WRITE = 002 - }; - -static int internal_cmd_formats (struct lexer *, struct dataset *ds, int); +static int cmd_formats__ (struct lexer *, struct dataset *ds, + bool print_format, bool write_format); int cmd_print_formats (struct lexer *lexer, struct dataset *ds) { - return internal_cmd_formats (lexer, ds, FORMATS_PRINT); + return cmd_formats__ (lexer, ds, true, false); } int cmd_write_formats (struct lexer *lexer, struct dataset *ds) { - return internal_cmd_formats (lexer, ds, FORMATS_WRITE); + return cmd_formats__ (lexer, ds, false, true); } int cmd_formats (struct lexer *lexer, struct dataset *ds) { - return internal_cmd_formats (lexer, ds, FORMATS_PRINT | FORMATS_WRITE); + return cmd_formats__ (lexer, ds, true, true); } static int -internal_cmd_formats (struct lexer *lexer, struct dataset *ds, int which) +cmd_formats__ (struct lexer *lexer, struct dataset *ds, + bool print_format, bool write_format) { /* Variables. */ struct variable **v; @@ -87,10 +83,17 @@ internal_cmd_formats (struct lexer *lexer, struct dataset *ds, int which) lex_error_expecting (lexer, "`('"); goto fail; } - if (!parse_format_specifier (lexer, &f) - || !fmt_check_output (&f) - || !fmt_check_width_compat (&f, var_get_name (v[0]), width)) - goto fail; + if (!parse_format_specifier (lexer, &f)) + goto fail; + char *error = fmt_check_output__ (&f); + if (!error) + error = fmt_check_width_compat__ (&f, var_get_name (v[0]), width); + if (error) + { + lex_next_error (lexer, -1, -1, "%s", error); + free (error); + goto fail; + } if (!lex_match (lexer, T_RPAREN)) { @@ -100,9 +103,9 @@ internal_cmd_formats (struct lexer *lexer, struct dataset *ds, int which) for (i = 0; i < cv; i++) { - if (which & FORMATS_PRINT) + if (print_format) var_set_print_format (v[i], &f); - if (which & FORMATS_WRITE) + if (write_format) var_set_write_format (v[i], &f); } free (v); diff --git a/src/language/dictionary/numeric.c b/src/language/dictionary/numeric.c index ae9f25325a..07c4193a53 100644 --- a/src/language/dictionary/numeric.c +++ b/src/language/dictionary/numeric.c @@ -58,8 +58,13 @@ cmd_numeric (struct lexer *lexer, struct dataset *ds) if (!parse_format_specifier (lexer, &f)) goto fail; - if (! fmt_check_output (&f)) - goto fail; + char *error = fmt_check_output__ (&f); + if (error) + { + lex_next_error (lexer, -1, -1, "%s", error); + free (error); + goto fail; + } if (fmt_is_string (f.type)) { @@ -130,18 +135,20 @@ cmd_string (struct lexer *lexer, struct dataset *ds) return CMD_FAILURE; if (!lex_force_match (lexer, T_LPAREN) - || !parse_format_specifier (lexer, &f) - || !lex_force_match (lexer, T_RPAREN)) + || !parse_format_specifier (lexer, &f)) goto fail; - if (!fmt_is_string (f.type)) - { - char str[FMT_STRING_LEN_MAX + 1]; - lex_next_error (lexer, -2, -2, - _("Format type %s may not be used with a string " - "variable."), fmt_to_string (&f, str)); - goto fail; - } - if (!fmt_check_output (&f)) + + char *error = fmt_check_type_compat__ (&f, VAL_STRING); + if (!error) + error = fmt_check_output__ (&f); + if (error) + { + lex_next_error (lexer, -2, -2, "%s", error); + free (error); + goto fail; + } + + if (!lex_force_match (lexer, T_RPAREN)) goto fail; width = fmt_var_width (&f); diff --git a/src/language/dictionary/vector.c b/src/language/dictionary/vector.c index fe1ab99e14..35e4738055 100644 --- a/src/language/dictionary/vector.c +++ b/src/language/dictionary/vector.c @@ -133,9 +133,15 @@ cmd_vector (struct lexer *lexer, struct dataset *ds) else if (lex_token (lexer) == T_ID && !seen_format) { seen_format = true; - if (!parse_format_specifier (lexer, &format) - || !fmt_check_output (&format)) + if (!parse_format_specifier (lexer, &format)) goto fail; + char *error = fmt_check_output__ (&format); + if (error) + { + lex_next_error (lexer, -1, -1, "%s", error); + free (error); + goto fail; + } } else { diff --git a/src/language/expressions/evaluate.c b/src/language/expressions/evaluate.c index e07deeaf81..18e794f271 100644 --- a/src/language/expressions/evaluate.c +++ b/src/language/expressions/evaluate.c @@ -227,10 +227,17 @@ cmd_debug_evaluate (struct lexer *lexer, struct dataset *dsother UNUSED) else if (lex_match_id (lexer, "FORMAT")) { lex_match (lexer, T_EQUALS); - if (!parse_format_specifier (lexer, &format) - || !fmt_check_output (&format) - || !fmt_check_type_compat (&format, VAL_NUMERIC)) + if (!parse_format_specifier (lexer, &format)) goto done; + char *error = fmt_check_output__ (&format); + if (!error) + error = fmt_check_type_compat__ (&format, VAL_NUMERIC); + if (error) + { + lex_next_error (lexer, -1, -1, "%s", error); + free (error); + goto done; + } has_format = true; } else diff --git a/src/language/expressions/parse.c b/src/language/expressions/parse.c index d1aaee3b17..7accce15f1 100644 --- a/src/language/expressions/parse.c +++ b/src/language/expressions/parse.c @@ -490,31 +490,25 @@ type_coercion__ (struct expression *e, struct expr_node *node, size_t arg_idx, NOT_REACHED (); case OP_ni_format: - msg_disable (); if (arg->type == OP_format && fmt_check_input (&arg->format) && fmt_check_type_compat (&arg->format, VAL_NUMERIC)) { - msg_enable (); if (do_coercion) arg->type = OP_ni_format; return true; } - msg_enable (); break; case OP_no_format: - msg_disable (); if (arg->type == OP_format && fmt_check_output (&arg->format) && fmt_check_type_compat (&arg->format, VAL_NUMERIC)) { - msg_enable (); if (do_coercion) arg->type = OP_no_format; return true; } - msg_enable (); break; case OP_num_var: diff --git a/src/language/stats/ctables.c b/src/language/stats/ctables.c index 514765962a..325eb242de 100644 --- a/src/language/stats/ctables.c +++ b/src/language/stats/ctables.c @@ -1288,9 +1288,20 @@ parse_ctables_format_specifier (struct lexer *lexer, struct fmt_spec *format, else { *is_ctables_format = false; - return (parse_format_specifier (lexer, format) - && fmt_check_output (format) - && fmt_check_type_compat (format, VAL_NUMERIC)); + if (!parse_format_specifier (lexer, format)) + return false; + + char *error = fmt_check_output__ (format); + if (!error) + error = fmt_check_type_compat__ (format, VAL_NUMERIC); + if (error) + { + lex_next_error (lexer, -1, -1, "%s", error); + free (error); + return false; + } + + return true; } lex_get (lexer); diff --git a/src/language/stats/matrix.c b/src/language/stats/matrix.c index 01d4480787..7e9bd54658 100644 --- a/src/language/stats/matrix.c +++ b/src/language/stats/matrix.c @@ -7249,9 +7249,38 @@ matrix_write_parse (struct matrix_state *s) write->format = xmalloc (sizeof *write->format); *write->format = (struct fmt_spec) { .type = format, .w = w }; - if (!fmt_check_output (write->format)) - goto error; - }; + char *error = fmt_check_output__ (write->format); + if (error) + { + msg (SE, "%s", error); + free (error); + + if (has_format) + lex_ofs_msg (s->lexer, SN, format_ofs, format_ofs, + _("This syntax specifies format %s."), + fmt_name (format)); + + if (repetitions) + { + lex_ofs_msg (s->lexer, SN, format_ofs, format_ofs, + ngettext ("This syntax specifies %d repetition.", + "This syntax specifies %d repetitions.", + repetitions), + repetitions); + lex_ofs_msg (s->lexer, SN, record_width_start, record_width_end, + _("This syntax designates record width %d, " + "which divided by %d repetitions implies " + "field width %d."), + record_width, repetitions, w); + } + + if (by) + lex_ofs_msg (s->lexer, SN, by_ofs, by_ofs, + _("This syntax specifies field width %d."), by); + + goto error; + } + } if (write->format && fmt_var_width (write->format) > sizeof (double)) { diff --git a/src/language/tests/format-guesser-test.c b/src/language/tests/format-guesser-test.c index 0900ebd817..de68cb20d7 100644 --- a/src/language/tests/format-guesser-test.c +++ b/src/language/tests/format-guesser-test.c @@ -43,14 +43,12 @@ cmd_debug_format_guesser (struct lexer *lexer, struct dataset *ds UNUSED) fmt_guesser_guess (g, &format); fmt_to_string (&format, format_string); fprintf (stderr, "=> %s", format_string); - msg_disable (); if (!fmt_check_input (&format)) { fmt_fix_input (&format); fmt_to_string (&format, format_string); fprintf (stderr, " (%s)", format_string); } - msg_enable (); putc ('\n', stderr); fmt_guesser_destroy (g); diff --git a/src/language/utilities/set.c b/src/language/utilities/set.c index 43adaf6da3..2c867145ef 100644 --- a/src/language/utilities/set.c +++ b/src/language/utilities/set.c @@ -523,8 +523,13 @@ parse_FORMAT (struct lexer *lexer) if (!parse_format_specifier (lexer, &fmt)) return false; - if (!fmt_check_output (&fmt)) - return false; + char *error = fmt_check_output__ (&fmt); + if (error) + { + lex_next_error (lexer, -1, -1, "%s", error); + free (error); + return false; + } int end = lex_ofs (lexer) - 1; if (fmt_is_string (fmt.type)) diff --git a/src/output/spv/spv.c b/src/output/spv/spv.c index 04aa610645..4e59fa3a23 100644 --- a/src/output/spv/spv.c +++ b/src/output/spv/spv.c @@ -882,7 +882,6 @@ spv_decode_fmt_spec (uint32_t u32, struct fmt_spec *out) uint8_t w = u32 >> 8; uint8_t d = u32; - msg_disable (); *out = (struct fmt_spec) { .type = FMT_F, .w = w, .d = d }; bool ok = raw_type >= 40 || fmt_from_io (raw_type, &out->type); if (ok) @@ -890,7 +889,6 @@ spv_decode_fmt_spec (uint32_t u32, struct fmt_spec *out) fmt_fix_output (out); ok = fmt_check_width_compat (out, NULL, 0); } - msg_enable (); if (!ok) { diff --git a/src/ui/gui/var-type-dialog.c b/src/ui/gui/var-type-dialog.c index a6d89cbf8a..e8646e9d0b 100644 --- a/src/ui/gui/var-type-dialog.c +++ b/src/ui/gui/var-type-dialog.c @@ -432,7 +432,6 @@ preview_custom (GtkWidget *w, gpointer data) text = gtk_entry_get_text (GTK_ENTRY (dialog->entry_width)); dialog->fmt_l.w = atoi (text); - msg_disable (); if (! fmt_check_output (&dialog->fmt_l)) { gtk_label_set_text (GTK_LABEL (dialog->label_psample), "---"); @@ -455,7 +454,6 @@ preview_custom (GtkWidget *w, gpointer data) gtk_label_set_text (GTK_LABEL (dialog->label_nsample), sample_text); g_free (sample_text); } - msg_enable (); } static gint diff --git a/tests/language/dictionary/formats.at b/tests/language/dictionary/formats.at index 93e135b106..9e9d171ae4 100644 --- a/tests/language/dictionary/formats.at +++ b/tests/language/dictionary/formats.at @@ -84,20 +84,28 @@ x,A1 y,A2 z,A3 -"formats.sps:2: error: FORMATS: Output format E6.1 specifies 1 decimal place, but the given width does not allow for any decimals." +"formats.sps:2.12-2.15: error: FORMATS: Output format E6.1 specifies 1 decimal place, but the given width does not allow for any decimals. + 2 | FORMATS a (E6.1). + | ^~~~" "formats.sps:3.11: error: FORMATS: a and y are not the same type. All variables in this variable list must be of the same type. y will be omitted from the list. 3 | FORMATS a y (F4). | ^" -formats.sps:4: error: FORMATS: String variable x with width 1 is not compatible with format A2. +"formats.sps:4.12-4.13: error: FORMATS: String variable x with width 1 is not compatible with format A2. Use format A1 instead. + 4 | FORMATS x (A2). + | ^~" -formats.sps:5: error: FORMATS: String variable y with width 2 is not compatible with format AHEX2. +"formats.sps:5.12-5.16: error: FORMATS: String variable y with width 2 is not compatible with format AHEX2. Use format AHEX4 instead. + 5 | FORMATS y (AHEX2). + | ^~~~~" "formats.sps:6.11: error: FORMATS: x and y are string variables with different widths. All variables in this variable list must have the same width. y will be omitted from the list. 6 | FORMATS x y (A2). | ^" -formats.sps:6: error: FORMATS: String variable x with width 1 is not compatible with format A2. +"formats.sps:6.14-6.15: error: FORMATS: String variable x with width 1 is not compatible with format A2. Use format A1 instead. + 6 | FORMATS x y (A2). + | ^~" ]) AT_CLEANUP diff --git a/tests/language/stats/ctables.at b/tests/language/stats/ctables.at index 5bc111787c..63a8cdfd28 100644 --- a/tests/language/stats/ctables.at +++ b/tests/language/stats/ctables.at @@ -262,8 +262,10 @@ and 100 for PTILE. 11 | CTABLES /TABLE qn1 [PTILE 101]. | ^~~ -ctables.sps:12: error: CTABLES: Output format F0.1 specifies width 0, but F -requires a width between 1 and 40. +ctables.sps:12.26-12.29: error: CTABLES: Output format F0.1 specifies width 0, +but F requires a width between 1 and 40. + 12 | CTABLES /TABLE qn1 [MEAN F0.1]. + | ^~~~ ctables.sps:13.26-13.36: error: CTABLES: Output format NEGPAREN requires width 2 or greater. diff --git a/tests/language/stats/matrix.at b/tests/language/stats/matrix.at index c3cb334650..5a6cd48bb2 100644 --- a/tests/language/stats/matrix.at +++ b/tests/language/stats/matrix.at @@ -3643,6 +3643,14 @@ matrix.sps:18.26: note: WRITE: This syntax specifies field width 5. matrix.sps:19: error: WRITE: Output format E5.0 specifies width 5, but E requires a width between 6 and 40. +matrix.sps:19.56: note: WRITE: This syntax specifies format E. + 19 | WRITE 1/FIELD=1 TO 10 BY 5/OUTFILE='matrix.txt'/FORMAT=E. + | ^ + +matrix.sps:19.26: note: WRITE: This syntax specifies field width 5. + 19 | WRITE 1/FIELD=1 TO 10 BY 5/OUTFILE='matrix.txt'/FORMAT=E. + | ^ + matrix.sps:20.51-20.52: error: WRITE: Format A9 is too wide for 8-byte matrix elements. 20 | WRITE 1/FIELD=1 TO 10/OUTFILE='matrix.txt'/FORMAT=A9. diff --git a/tests/language/utilities/set.at b/tests/language/utilities/set.at index f93ac13012..2103f2e96d 100644 --- a/tests/language/utilities/set.at +++ b/tests/language/utilities/set.at @@ -29,7 +29,9 @@ SET FORMAT F41. DESCRIPTIVES /x. ]) AT_CHECK([pspp -O format=csv set.pspp], [1], [dnl -"set.pspp:7: error: SET: Output format F41.0 specifies width 41, but F requires a width between 1 and 40." +"set.pspp:7.12-7.14: error: SET: Output format F41.0 specifies width 41, but F requires a width between 1 and 40. + 7 | SET FORMAT F41. + | ^~~" Table: Descriptive Statistics ,N,Mean,Std Dev,Minimum,Maximum -- 2.30.2