From: David Malcolm <dmalcolm@redhat.com>
To: Lewis Hyatt <lhyatt@gmail.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH 2/6] diagnostics: Use an inline function rather than hardcoding <built-in> string
Date: Fri, 04 Nov 2022 11:55:12 -0400 [thread overview]
Message-ID: <0778ff93d5da38dfde39d2a755710fe8225e4110.camel@redhat.com> (raw)
In-Reply-To: <65bfbf319942664358737a1d9d9103f7304199d0.1667514153.git.lhyatt@gmail.com>
On Fri, 2022-11-04 at 09:44 -0400, Lewis Hyatt via Gcc-patches wrote:
> The string "<built-in>" is hard-coded in several places throughout
> the
> diagnostics code, and in some of those places, it is used incorrectly
> with
> respect to internationalization. (Comparing a translated string to an
> untranslated string.) The error is not currently observable in any
> output GCC
> actually produces, hence no testcase added here, but it's worth
> fixing, and
> also, I am shortly going to add a new such string and want to avoid
> hardcoding
> that one in similar places.
Thanks; looks good to me.
Dave
>
> gcc/c-family/ChangeLog:
>
> * c-opts.cc (c_finish_options): Use special_fname_builtin ()
> rather
> than a hard-coded string.
>
> gcc/ChangeLog:
>
> * diagnostic.cc (diagnostic_get_location_text): Use
> special_fname_builtin () rather than a hardcoded string
> (which was
> also incorrectly left untranslated previously.)
> * input.cc (special_fname_builtin): New function.
> (expand_location_1): Use special_fname_builtin () rather than
> a
> hard-coded string.
> (test_builtins): Likewise.
> * input.h (special_fname_builtin): Declare.
>
> gcc/fortran/ChangeLog:
>
> * cpp.cc (gfc_cpp_init): Use special_fname_builtin () rather
> than a
> hardcoded string (which was also incorrectly left
> untranslated
> previously.)
> * error.cc (gfc_diagnostic_build_locus_prefix): Likewise.
> * f95-lang.cc (gfc_init): Likewise.
> ---
> gcc/c-family/c-opts.cc | 2 +-
> gcc/diagnostic.cc | 2 +-
> gcc/fortran/cpp.cc | 2 +-
> gcc/fortran/error.cc | 4 ++--
> gcc/fortran/f95-lang.cc | 2 +-
> gcc/input.cc | 10 ++++++++--
> gcc/input.h | 3 +++
> 7 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
> index 32b929e3ece..521797fb7eb 100644
> --- a/gcc/c-family/c-opts.cc
> +++ b/gcc/c-family/c-opts.cc
> @@ -1476,7 +1476,7 @@ c_finish_options (void)
> {
> const line_map_ordinary *bltin_map
> = linemap_check_ordinary (linemap_add (line_table, LC_RENAME,
> 0,
> - _("<built-in>"), 0));
> + special_fname_builtin
> (), 0));
> cb_file_change (parse_in, bltin_map);
> linemap_line_start (line_table, 0, 1);
>
> diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
> index 22f7b0b6d6e..7c7ee6da746 100644
> --- a/gcc/diagnostic.cc
> +++ b/gcc/diagnostic.cc
> @@ -470,7 +470,7 @@ diagnostic_get_location_text (diagnostic_context
> *context,
> const char *file = s.file ? s.file : progname;
> int line = 0;
> int col = -1;
> - if (strcmp (file, N_("<built-in>")))
> + if (strcmp (file, special_fname_builtin ()))
> {
> line = s.line;
> if (context->show_column)
> diff --git a/gcc/fortran/cpp.cc b/gcc/fortran/cpp.cc
> index 364bd0d2a85..0b5755edbb4 100644
> --- a/gcc/fortran/cpp.cc
> +++ b/gcc/fortran/cpp.cc
> @@ -605,7 +605,7 @@ gfc_cpp_init (void)
> if (gfc_option.flag_preprocessed)
> return;
>
> - cpp_change_file (cpp_in, LC_RENAME, _("<built-in>"));
> + cpp_change_file (cpp_in, LC_RENAME, special_fname_builtin ());
> if (!gfc_cpp_option.no_predefined)
> {
> /* Make sure all of the builtins about to be declared have
> diff --git a/gcc/fortran/error.cc b/gcc/fortran/error.cc
> index c9d6edbb923..214fb78ba7b 100644
> --- a/gcc/fortran/error.cc
> +++ b/gcc/fortran/error.cc
> @@ -1147,7 +1147,7 @@ gfc_diagnostic_build_locus_prefix
> (diagnostic_context *context,
> const char *locus_ce = colorize_stop (pp_show_color (pp));
> return (s.file == NULL
> ? build_message_string ("%s%s:%s", locus_cs, progname,
> locus_ce )
> - : !strcmp (s.file, N_("<built-in>"))
> + : !strcmp (s.file, special_fname_builtin ())
> ? build_message_string ("%s%s:%s", locus_cs, s.file,
> locus_ce)
> : context->show_column
> ? build_message_string ("%s%s:%d:%d:%s", locus_cs, s.file,
> s.line,
> @@ -1167,7 +1167,7 @@ gfc_diagnostic_build_locus_prefix
> (diagnostic_context *context,
>
> return (s.file == NULL
> ? build_message_string ("%s%s:%s", locus_cs, progname,
> locus_ce )
> - : !strcmp (s.file, N_("<built-in>"))
> + : !strcmp (s.file, special_fname_builtin ())
> ? build_message_string ("%s%s:%s", locus_cs, s.file,
> locus_ce)
> : context->show_column
> ? build_message_string ("%s%s:%d:%d-%d:%s", locus_cs,
> s.file, s.line,
> diff --git a/gcc/fortran/f95-lang.cc b/gcc/fortran/f95-lang.cc
> index a6750bea787..0d83f3f8b69 100644
> --- a/gcc/fortran/f95-lang.cc
> +++ b/gcc/fortran/f95-lang.cc
> @@ -259,7 +259,7 @@ gfc_init (void)
> if (!gfc_cpp_enabled ())
> {
> linemap_add (line_table, LC_ENTER, false, gfc_source_file, 1);
> - linemap_add (line_table, LC_RENAME, false, "<built-in>", 0);
> + linemap_add (line_table, LC_RENAME, false,
> special_fname_builtin (), 0);
> }
> else
> gfc_cpp_init_0 ();
> diff --git a/gcc/input.cc b/gcc/input.cc
> index a28abfac5ac..483cb6e940d 100644
> --- a/gcc/input.cc
> +++ b/gcc/input.cc
> @@ -29,6 +29,12 @@ along with GCC; see the file COPYING3. If not see
> #define HAVE_ICONV 0
> #endif
>
> +const char *
> +special_fname_builtin ()
> +{
> + return _("<built-in>");
> +}
> +
> /* Input charset configuration. */
> static const char *default_charset_callback (const char *)
> {
> @@ -275,7 +281,7 @@ expand_location_1 (location_t loc,
>
> xloc.data = block;
> if (loc <= BUILTINS_LOCATION)
> - xloc.file = loc == UNKNOWN_LOCATION ? NULL : _("<built-in>");
> + xloc.file = loc == UNKNOWN_LOCATION ? NULL :
> special_fname_builtin ();
>
> return xloc;
> }
> @@ -2102,7 +2108,7 @@ test_unknown_location ()
> static void
> test_builtins ()
> {
> - assert_loceq (_("<built-in>"), 0, 0, BUILTINS_LOCATION);
> + assert_loceq (special_fname_builtin (), 0, 0, BUILTINS_LOCATION);
> ASSERT_PRED1 (is_location_from_builtin_token, BUILTINS_LOCATION);
> }
>
> diff --git a/gcc/input.h b/gcc/input.h
> index 11c571d076f..0b23e66e53b 100644
> --- a/gcc/input.h
> +++ b/gcc/input.h
> @@ -32,6 +32,9 @@ extern GTY(()) class line_maps *saved_line_table;
> /* The location for declarations in "<built-in>" */
> #define BUILTINS_LOCATION ((location_t) 1)
>
> +/* Returns the translated string referring to the special location.
> */
> +const char *special_fname_builtin ();
> +
> /* line-map.cc reserves RESERVED_LOCATION_COUNT to the user. Ensure
> both UNKNOWN_LOCATION and BUILTINS_LOCATION fit into that. */
> STATIC_ASSERT (BUILTINS_LOCATION < RESERVED_LOCATION_COUNT);
>
next prev parent reply other threads:[~2022-11-04 15:55 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-04 13:44 [PATCH 0/6] diagnostics: libcpp: Overhaul locations for _Pragma tokens Lewis Hyatt
2022-11-04 13:44 ` [PATCH 1/6] diagnostics: Fix macro tracking for ad-hoc locations Lewis Hyatt
2022-11-04 15:53 ` David Malcolm
2022-11-04 13:44 ` [PATCH 2/6] diagnostics: Use an inline function rather than hardcoding <built-in> string Lewis Hyatt
2022-11-04 15:55 ` David Malcolm [this message]
2022-11-04 13:44 ` [PATCH 3/6] libcpp: Fix paste error with unknown pragma after macro expansion Lewis Hyatt
2022-11-21 17:50 ` Jeff Law
2022-11-04 13:44 ` [PATCH 4/6] diagnostics: libcpp: Add LC_GEN linemaps to support in-memory buffers Lewis Hyatt
2022-11-05 16:23 ` David Malcolm
2022-11-05 17:28 ` Lewis Hyatt
2022-11-17 21:21 ` Lewis Hyatt
2023-01-05 22:34 ` Lewis Hyatt
2022-11-04 13:44 ` [PATCH 5/6] diagnostics: Support generated data in additional contexts Lewis Hyatt
2022-11-04 16:42 ` David Malcolm
2022-11-04 21:05 ` Lewis Hyatt
2022-11-05 1:54 ` [PATCH 5b/6] diagnostics: Remove null-termination requirement for json::string David Malcolm
2022-11-05 1:55 ` [PATCH 5a/6] diagnostics: Handle generated data locations in edit_context David Malcolm
2022-11-04 13:44 ` [PATCH 6/6] diagnostics: libcpp: Assign real locations to the tokens inside _Pragma strings Lewis Hyatt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0778ff93d5da38dfde39d2a755710fe8225e4110.camel@redhat.com \
--to=dmalcolm@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=lhyatt@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).