[2/6] diagnostics: Use an inline function rather than hardcoding <built-in> string

Message ID 65bfbf319942664358737a1d9d9103f7304199d0.1667514153.git.lhyatt@gmail.com
State Accepted
Headers
Series diagnostics: libcpp: Overhaul locations for _Pragma tokens |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Lewis Hyatt Nov. 4, 2022, 1:44 p.m. UTC
  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.

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(-)
  

Comments

David Malcolm Nov. 4, 2022, 3:55 p.m. UTC | #1
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);
>
  

Patch

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);