From: Ben Pfaff Date: Sat, 3 Jul 2021 18:28:05 +0000 (-0700) Subject: message: Break message location out into a separate struct. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bd8fac754b2cff134bab7e4ad425a0a988123dba;p=pspp message: Break message location out into a separate struct. This will make it cleaner to have a stack of locations for use in reporting macro expansion errors. --- diff --git a/src/language/data-io/data-parser.c b/src/language/data-io/data-parser.c index 818c15a30f..6de88b170d 100644 --- a/src/language/data-io/data-parser.c +++ b/src/language/data-io/data-parser.c @@ -495,14 +495,18 @@ static void parse_error (const struct dfm_reader *reader, const struct field *field, int first_column, int last_column, char *error) { - struct msg m = { - .category = MSG_C_DATA, - .severity = MSG_S_WARNING, + int line_number = dfm_get_line_number (reader); + const struct msg_location location = { .file_name = CONST_CAST (char *, dfm_get_file_name (reader)), - .first_line = dfm_get_line_number (reader), - .last_line = m.first_line + 1, + .first_line = line_number, + .last_line = line_number + 1, .first_column = first_column, .last_column = last_column, + }; + struct msg m = { + .category = MSG_C_DATA, + .severity = MSG_S_WARNING, + .location = CONST_CAST (struct msg_location *, &location), .text = xasprintf (_("Data for variable %s is not valid as format %s: %s"), field->name, fmt_name (field->format.type), error), }; diff --git a/src/language/data-io/data-reader.c b/src/language/data-io/data-reader.c index ab3e065789..9a9cafa7f9 100644 --- a/src/language/data-io/data-reader.c +++ b/src/language/data-io/data-reader.c @@ -708,15 +708,19 @@ dfm_get_column (const struct dfm_reader *r, const char *p) const char * dfm_get_file_name (const struct dfm_reader *r) { - return (fh_get_referent (r->fh) == FH_REF_FILE - ? fh_get_file_name (r->fh) + enum fh_referent referent = fh_get_referent (r->fh); + return (referent == FH_REF_FILE ? fh_get_file_name (r->fh) + : referent == FH_REF_INLINE ? lex_get_file_name (r->lexer) : NULL); } int dfm_get_line_number (const struct dfm_reader *r) { - return fh_get_referent (r->fh) == FH_REF_FILE ? r->line_number : -1; + enum fh_referent referent = fh_get_referent (r->fh); + return (referent == FH_REF_FILE ? r->line_number + : referent == FH_REF_INLINE ? lex_get_first_line_number (r->lexer, 0) + : -1); } /* BEGIN DATA...END DATA procedure. */ diff --git a/src/language/lexer/lexer.c b/src/language/lexer/lexer.c index c14bc6acb8..159949e0b0 100644 --- a/src/language/lexer/lexer.c +++ b/src/language/lexer/lexer.c @@ -1157,6 +1157,34 @@ lex_get_file_name (const struct lexer *lexer) return src == NULL ? NULL : src->reader->file_name; } +/* Returns a newly allocated msg_location for the syntax that represents tokens + with 0-based offsets N0...N1, inclusive, from the current token. The caller + must eventually free the location (with msg_location_destroy()). */ +struct msg_location * +lex_get_location (const struct lexer *lexer, int n0, int n1) +{ + struct msg_location *loc = lex_get_lines (lexer, n0, n1); + loc->first_column = lex_get_first_column (lexer, n0); + loc->last_column = lex_get_last_column (lexer, n1); + return loc; +} + +/* Returns a newly allocated msg_location for the syntax that represents tokens + with 0-based offsets N0...N1, inclusive, from the current token. The + location only covers the tokens' lines, not the columns. The caller must + eventually free the location (with msg_location_destroy()). */ +struct msg_location * +lex_get_lines (const struct lexer *lexer, int n0, int n1) +{ + struct msg_location *loc = xmalloc (sizeof *loc); + *loc = (struct msg_location) { + .file_name = xstrdup_if_nonnull (lex_get_file_name (lexer)), + .first_line = lex_get_first_line_number (lexer, n0), + .last_line = lex_get_last_line_number (lexer, n1), + }; + return loc; +} + const char * lex_get_encoding (const struct lexer *lexer) { @@ -1411,14 +1439,17 @@ lex_source_error_valist (struct lex_source *src, int n0, int n1, if (ds_last (&s) != '.') ds_put_byte (&s, '.'); - struct msg m = { - .category = MSG_C_SYNTAX, - .severity = MSG_S_ERROR, + struct msg_location location = { .file_name = src->reader->file_name, .first_line = lex_source_get_first_line_number (src, n0), .last_line = lex_source_get_last_line_number (src, n1), .first_column = lex_source_get_first_column (src, n0), .last_column = lex_source_get_last_column (src, n1), + }; + struct msg m = { + .category = MSG_C_SYNTAX, + .severity = MSG_S_ERROR, + .location = &location, .text = ds_steal_cstr (&s), }; msg_emit (&m); diff --git a/src/language/lexer/lexer.h b/src/language/lexer/lexer.h index f57ee822ed..9434fc684a 100644 --- a/src/language/lexer/lexer.h +++ b/src/language/lexer/lexer.h @@ -152,6 +152,8 @@ int lex_get_last_line_number (const struct lexer *, int n); int lex_get_first_column (const struct lexer *, int n); int lex_get_last_column (const struct lexer *, int n); const char *lex_get_file_name (const struct lexer *); +struct msg_location *lex_get_location (const struct lexer *, int n0, int n1); +struct msg_location *lex_get_lines (const struct lexer *, int n0, int n1); const char *lex_get_encoding (const struct lexer *); /* Issuing errors. */ diff --git a/src/libpspp/message.c b/src/libpspp/message.c index e40b34c93c..7d4f9d8430 100644 --- a/src/libpspp/message.c +++ b/src/libpspp/message.c @@ -86,7 +86,6 @@ msg_error (int errnum, const char *format, ...) struct msg m = { .category = MSG_C_GENERAL, .severity = MSG_S_ERROR, - .file_name = NULL, .text = xasprintf (_("%s: %s"), e, strerror (errnum)), }; msg_emit (&m); @@ -103,6 +102,100 @@ msg_set_handler (void (*handler) (const struct msg *, void *aux), void *aux) msg_aux = aux; } +/* msg_location. */ + +void +msg_location_destroy (struct msg_location *loc) +{ + if (loc) + { + free (loc->file_name); + free (loc); + } +} + +struct msg_location * +msg_location_dup (const struct msg_location *src) +{ + if (!src) + return NULL; + + struct msg_location *dst = xmalloc (sizeof *dst); + *dst = (struct msg_location) { + .file_name = xstrdup_if_nonnull (src->file_name), + .first_line = src->first_line, + .last_line = src->last_line, + .first_column = src->first_column, + .last_column = src->last_column, + }; + return dst; +} + +bool +msg_location_is_empty (const struct msg_location *loc) +{ + return !loc || (!loc->file_name + && loc->first_line <= 0 + && loc->first_column <= 0); +} + +void +msg_location_format (const struct msg_location *loc, struct string *s) +{ + if (!loc) + return; + + if (loc->file_name) + ds_put_cstr (s, loc->file_name); + + int l1 = loc->first_line; + int l2 = MAX (loc->first_line, loc->last_line - 1); + int c1 = loc->first_column; + int c2 = MAX (loc->first_column, loc->last_column - 1); + + if (l1 > 0) + { + if (loc->file_name) + ds_put_byte (s, ':'); + + if (l2 > l1) + { + if (c1 > 0) + ds_put_format (s, "%d.%d-%d.%d", l1, c1, l2, c2); + else + ds_put_format (s, "%d-%d", l1, l2); + } + else + { + if (c1 > 0) + { + if (c2 > c1) + { + /* The GNU coding standards say to use + LINENO-1.COLUMN-1-COLUMN-2 for this case, but GNU + Emacs interprets COLUMN-2 as LINENO-2 if I do that. + I've submitted an Emacs bug report: + http://debbugs.gnu.org/cgi/bugreport.cgi?bug=7725. + + For now, let's be compatible. */ + ds_put_format (s, "%d.%d-%d.%d", l1, c1, l1, c2); + } + else + ds_put_format (s, "%d.%d", l1, c1); + } + else + ds_put_format (s, "%d", l1); + } + } + else if (c1 > 0) + { + if (c2 > c1) + ds_put_format (s, ".%d-%d", c1, c2); + else + ds_put_format (s, ".%d", c1); + } +} + /* Working with messages. */ const char * @@ -122,18 +215,17 @@ msg_severity_to_string (enum msg_severity severity) /* Duplicate a message */ struct msg * -msg_dup (const struct msg *m) +msg_dup (const struct msg *src) { - struct msg *new_msg; - - new_msg = xmemdup (m, sizeof *m); - if (m->file_name != NULL) - new_msg->file_name = xstrdup (m->file_name); - if (m->command_name != NULL) - new_msg->command_name = xstrdup (m->command_name); - new_msg->text = xstrdup (m->text); - - return new_msg; + struct msg *dst = xmalloc (sizeof *dst); + *dst = (struct msg) { + .category = src->category, + .severity = src->severity, + .location = msg_location_dup (src->location), + .command_name = xstrdup_if_nonnull (src->command_name), + .text = xstrdup (src->text), + }; + return dst; } /* Frees a message created by msg_dup(). @@ -144,10 +236,13 @@ msg_dup (const struct msg *m) void msg_destroy (struct msg *m) { - free (m->file_name); - free (m->text); - free (m->command_name); - free (m); + if (m) + { + msg_location_destroy (m->location); + free (m->text); + free (m->command_name); + free (m); + } } char * @@ -157,58 +252,9 @@ msg_to_string (const struct msg *m) ds_init_empty (&s); - if (m->category != MSG_C_GENERAL - && (m->file_name || m->first_line > 0 || m->first_column > 0)) + if (m->category != MSG_C_GENERAL && !msg_location_is_empty (m->location)) { - int l1 = m->first_line; - int l2 = MAX (m->first_line, m->last_line - 1); - int c1 = m->first_column; - int c2 = MAX (m->first_column, m->last_column - 1); - - if (m->file_name) - ds_put_format (&s, "%s", m->file_name); - - if (l1 > 0) - { - if (!ds_is_empty (&s)) - ds_put_byte (&s, ':'); - - if (l2 > l1) - { - if (c1 > 0) - ds_put_format (&s, "%d.%d-%d.%d", l1, c1, l2, c2); - else - ds_put_format (&s, "%d-%d", l1, l2); - } - else - { - if (c1 > 0) - { - if (c2 > c1) - { - /* The GNU coding standards say to use - LINENO-1.COLUMN-1-COLUMN-2 for this case, but GNU - Emacs interprets COLUMN-2 as LINENO-2 if I do that. - I've submitted an Emacs bug report: - http://debbugs.gnu.org/cgi/bugreport.cgi?bug=7725. - - For now, let's be compatible. */ - ds_put_format (&s, "%d.%d-%d.%d", l1, c1, l1, c2); - } - else - ds_put_format (&s, "%d.%d", l1, c1); - } - else - ds_put_format (&s, "%d", l1); - } - } - else if (c1 > 0) - { - if (c2 > c1) - ds_put_format (&s, ".%d-%d", c1, c2); - else - ds_put_format (&s, ".%d", c1); - } + msg_location_format (m->location, &s); ds_put_cstr (&s, ": "); } diff --git a/src/libpspp/message.h b/src/libpspp/message.h index af7bc699d7..36aae9c5d1 100644 --- a/src/libpspp/message.h +++ b/src/libpspp/message.h @@ -21,6 +21,8 @@ #include #include "libpspp/compiler.h" +struct string; + /* What kind of message is this? */ enum msg_category { @@ -69,17 +71,28 @@ msg_class_from_category_and_severity (enum msg_category category, return category * 3 + severity; } +struct msg_location + { + char *file_name; /* Name of file containing error, or NULL. */ + int first_line; /* 1-based line number, or 0 if none. */ + int last_line; /* 1-based exclusive last line (0=none). */ + int first_column; /* 1-based first column, or 0 if none. */ + int last_column; /* 1-based exclusive last column (0=none). */ + }; + +void msg_location_destroy (struct msg_location *); +struct msg_location *msg_location_dup (const struct msg_location *); + +bool msg_location_is_empty (const struct msg_location *); +void msg_location_format (const struct msg_location *, struct string *); + /* A message. */ struct msg { enum msg_category category; /* Message category. */ enum msg_severity severity; /* Message severity. */ - char *file_name; /* Name of file containing error, or NULL. */ + struct msg_location *location; /* Code location. */ char *command_name; /* Name of erroneous command, or NULL. */ - int first_line; /* 1-based line number, or 0 if none. */ - int last_line; /* 1-based exclusive last line (0=none). */ - int first_column; /* 1-based first column, or 0 if none. */ - int last_column; /* 1-based exclusive last column (0=none). */ char *text; /* Error text. */ }; diff --git a/src/ui/gui/psppire.c b/src/ui/gui/psppire.c index 12a5e2612f..7344b98161 100644 --- a/src/ui/gui/psppire.c +++ b/src/ui/gui/psppire.c @@ -145,21 +145,21 @@ static void handle_msg (const struct msg *m_, void *lexer_) { struct lexer *lexer = lexer_; - struct msg m = *m_; - - if (lexer != NULL && m.file_name == NULL) - { - m.file_name = CONST_CAST (char *, lex_get_file_name (lexer)); - m.first_line = lex_get_first_line_number (lexer, 0); - m.last_line = lex_get_last_line_number (lexer, 0); - m.first_column = lex_get_first_column (lexer, 0); - m.last_column = lex_get_last_column (lexer, 0); - } - m.command_name = output_get_uppercase_command_name (); + struct msg m = { + .category = m_->category, + .severity = m_->severity, + .location = (m_->location ? m_->location + : lexer ? lex_get_location (lexer, 0, 0) + : NULL), + .command_name = output_get_uppercase_command_name (), + .text = m_->text, + }; output_item_submit (message_item_create (&m)); free (m.command_name); + if (m.location != m_->location) + msg_location_destroy (m.location); } void diff --git a/src/ui/terminal/main.c b/src/ui/terminal/main.c index 630089b2d2..1def93fdab 100644 --- a/src/ui/terminal/main.c +++ b/src/ui/terminal/main.c @@ -218,20 +218,21 @@ static void output_msg (const struct msg *m_, void *lexer_) { struct lexer *lexer = lexer_; - struct msg m = *m_; - - if (m.file_name == NULL) - { - m.file_name = CONST_CAST (char *, lex_get_file_name (lexer)); - m.first_line = lex_get_first_line_number (lexer, 0); - m.last_line = lex_get_last_line_number (lexer, 0); - } - - m.command_name = output_get_uppercase_command_name (); + struct msg m = { + .category = m_->category, + .severity = m_->severity, + .location = (m_->location ? m_->location + : lexer ? lex_get_lines (lexer, 0, 0) + : NULL), + .command_name = output_get_uppercase_command_name (), + .text = m_->text, + }; output_item_submit (message_item_create (&m)); free (m.command_name); + if (m.location != m_->location) + msg_location_destroy (m.location); } static void diff --git a/tests/output/pivot-table-test.c b/tests/output/pivot-table-test.c index cf476d5392..f1944c3237 100644 --- a/tests/output/pivot-table-test.c +++ b/tests/output/pivot-table-test.c @@ -1233,18 +1233,19 @@ static void output_msg (const struct msg *m_, void *lexer_) { struct lexer *lexer = lexer_; - struct msg m = *m_; - - if (m.file_name == NULL) - { - m.file_name = CONST_CAST (char *, lex_get_file_name (lexer)); - m.first_line = lex_get_first_line_number (lexer, 0); - m.last_line = lex_get_last_line_number (lexer, 0); - } - - m.command_name = output_get_uppercase_command_name (); + struct msg m = { + .category = m_->category, + .severity = m_->severity, + .location = (m_->location ? m_->location + : lexer ? lex_get_location (lexer, 0, 0) + : NULL), + .command_name = output_get_uppercase_command_name (), + .text = m_->text, + }; output_item_submit (message_item_create (&m)); free (m.command_name); + if (m.location != m_->location) + msg_location_destroy (m.location); }