[PATCH 0/7] Various i18n fixes (and questions)

[PATCH 0/7] Various i18n fixes (and questions)

[David Malcolm]

Roland Illig [CCed] recently filed numerous issues within our Bugzilla
instance relating to i18n.

In general, the issues look valid to me, so I've attempted to fix some
of them in the following patch kit.

However, we're deep in stage 4 of the development cycle.  Looking at
our development plan
  https://gcc.gnu.org/develop.html
it's not clear to me how such changes fit into our schedule: the plan
seems to make no mention of how i18n and translation fit in to the
stages (it talks about bugs and documentation, but not about translatable
strings).

Do we have a "string freeze" in our schedule?  i.e. is there a point
at which we avoid changing strings to avoid disrupting things for
translators?

For example, patch 6 in the kit fixes a bug in a rarely-seen message
and would add new entries to the gcc.pot.

Other patches in the kit fix grammar/style nits in diagnostic
messages, and these would mean changes to entries in the gcc.pot.
Some of these nits are clearly worth fixing (e.g. patch 1); others
(e.g. patch 7) may be more of a judgement call.

Given that we're deep in stage 4 of the cycle, how should
deferred issues be marked in BZ?  (do we have a
fix-queued-for-next-release tag?)

FWIW the combined patch kit was successfully bootstrapped&regrtested
on x86_64-pc-linux-gnu.

OK for trunk for next stage 1? (or for stage 4, if any of these
look high-priority or sufficiently low-risk?).

Also, from a developer POV, when should we regenerate and check-in
the .pot files?  The rules for submitting patches:
  https://gcc.gnu.org/contribute.html
and for committing them:
  https://gcc.gnu.org/svnwrite.html
seem to make no mention of gettext and .pot files.

Looking in libcpp/po/ChangeLog and gcc/po/ChangeLog I see that
Joseph [CCed] seems to regularly commit regenerated copies of the
.pot files; do we have a policy about this?

Thanks; sorry if I'm missing something obvious here.
Dave

David Malcolm (7):
  Add missing punctuation to message (PR driver/79875)
  aarch64.c: tweaks to quoting in error messages (PR target/79925)
  Remove trailing period from various diagnostic messages (PR
    translation/79923)
  c-indentation.c: workaround xgettext limitation (PR c/79921)
  fortran: remove trailing exclamation mark from various diagnostics (PR
    fortran/79852)
  i386.c: make "sorry" message more amenable to translation (PR
    target/79926)
  Simplify uses of "%<%s%>" to "%qs" (PR translation/79848)

 gcc/auto-profile.c            | 14 ++++++-------
 gcc/c-family/c-format.c       |  6 +++---
 gcc/c-family/c-indentation.c  |  4 ++--
 gcc/c/c-decl.c                | 16 +++++++--------
 gcc/c/c-parser.c              |  2 +-
 gcc/config/aarch64/aarch64.c  | 10 ++++-----
 gcc/config/arm/arm-builtins.c | 48 +++++++++++++++++++++----------------------
 gcc/config/arm/arm.c          |  2 +-
 gcc/config/i386/i386.c        | 12 ++++++++---
 gcc/config/msp430/msp430.c    |  4 ++--
 gcc/cp/decl.c                 |  6 +++---
 gcc/fortran/bbt.c             |  2 +-
 gcc/fortran/decl.c            |  2 +-
 gcc/fortran/dump-parse-tree.c |  2 +-
 gcc/fortran/module.c          |  2 +-
 gcc/fortran/primary.c         | 12 +++++------
 gcc/ipa-devirt.c              |  2 +-
 gcc/ipa-pure-const.c          |  4 ++--
 gcc/opts.c                    |  2 +-
 19 files changed, 79 insertions(+), 73 deletions(-)

-- 
1.8.5.3

[PATCH 7/7] Simplify uses of "%<%s%>" to "%qs" (PR translation/79848)

[David Malcolm]

PR translation/79848 requests that uses of "%<%s%>" in diagnostic messages
be simplified to "%qs", presumably to make it easier for translators
to deal with them.

This patch does this for all such strings found in the source tree.

gcc/c-family/ChangeLog:
	PR translation/79848
	* c-format.c (check_format_string): Simplify uses of "%<%s%>" to
	"%qs".

gcc/c/ChangeLog:
	PR translation/79848
	* c-decl.c (declspecs_add_type): Simplify uses of "%<%s%>" to
	"%qs".
	* c-parser.c (c_parser_oacc_shape_clause): Likewise.

gcc/cp/ChangeLog:
	PR translation/79848
	* decl.c (grokfndecl): Simplify uses of "%<%s%>" to "%qs".

gcc/ChangeLog:
	PR translation/79848
	* ipa-devirt.c (warn_types_mismatch): Simplify uses of "%<%s%>" to
	"%qs".
	* ipa-pure-const.c (suggest_attribute): Likewise.
---
 gcc/c-family/c-format.c |  6 +++---
 gcc/c/c-decl.c          | 16 ++++++++--------
 gcc/c/c-parser.c        |  2 +-
 gcc/cp/decl.c           |  6 +++---
 gcc/ipa-devirt.c        |  2 +-
 gcc/ipa-pure-const.c    |  4 ++--
 6 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index 81be935..400eb66 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -207,7 +207,7 @@ check_format_string (tree fntype, unsigned HOST_WIDE_INT format_num,
 	{
 	  /* We expected a char but found an extended string type.  */
 	  if (is_objc_sref)
-	    error ("found a %<%s%> reference but the format argument should"
+	    error ("found a %qs reference but the format argument should"
 		   " be a string", format_name (gcc_objc_string_format_type));
 	  else
 	    error ("found a %qT but the format argument should be a string",
@@ -220,7 +220,7 @@ check_format_string (tree fntype, unsigned HOST_WIDE_INT format_num,
   /* We expect a string object type as the format arg.  */
   if (is_char_ref)
     {
-      error ("format argument should be a %<%s%> reference but"
+      error ("format argument should be a %qs reference but"
 	     " a string was found", format_name (expected_format_type));
       *no_add_attrs = true;
       return false;
@@ -242,7 +242,7 @@ check_format_string (tree fntype, unsigned HOST_WIDE_INT format_num,
     return true;
   else
     {
-      error ("format argument should be a %<%s%> reference", 
+      error ("format argument should be a %qs reference",
 	      format_name (expected_format_type));
       *no_add_attrs = true;
       return false;
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 645304a..dd66cb6 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -10513,37 +10513,37 @@ declspecs_add_type (location_t loc, struct c_declspecs *specs,
 		  str = "_Decimal128";
 		if (specs->long_long_p)
 		  error_at (loc,
-			    ("both %<long long%> and %<%s%> in "
+			    ("both %<long long%> and %qs in "
 			     "declaration specifiers"),
 			    str);
 		if (specs->long_p)
 		  error_at (loc,
-			    ("both %<long%> and %<%s%> in "
+			    ("both %<long%> and %qs in "
 			     "declaration specifiers"),
 			    str);
 		else if (specs->short_p)
 		  error_at (loc,
-			    ("both %<short%> and %<%s%> in "
+			    ("both %<short%> and %qs in "
 			     "declaration specifiers"),
 			    str);
 		else if (specs->signed_p)
 		  error_at (loc,
-			    ("both %<signed%> and %<%s%> in "
+			    ("both %<signed%> and %qs in "
 			     "declaration specifiers"),
 			    str);
 		else if (specs->unsigned_p)
 		  error_at (loc,
-			    ("both %<unsigned%> and %<%s%> in "
+			    ("both %<unsigned%> and %qs in "
 			     "declaration specifiers"),
 			    str);
                 else if (specs->complex_p)
                   error_at (loc,
-			    ("both %<complex%> and %<%s%> in "
+			    ("both %<complex%> and %qs in "
 			     "declaration specifiers"),
 			    str);
                 else if (specs->saturating_p)
                   error_at (loc,
-			    ("both %<_Sat%> and %<%s%> in "
+			    ("both %<_Sat%> and %qs in "
 			     "declaration specifiers"),
 			    str);
 		else if (i == RID_DFLOAT32)
@@ -10571,7 +10571,7 @@ declspecs_add_type (location_t loc, struct c_declspecs *specs,
 		  str = "_Accum";
                 if (specs->complex_p)
                   error_at (loc,
-			    ("both %<complex%> and %<%s%> in "
+			    ("both %<complex%> and %qs in "
 			     "declaration specifiers"),
 			    str);
 		else if (i == RID_FRACT)
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index fa4e950..2a37ea8 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -11816,7 +11816,7 @@ c_parser_oacc_shape_clause (c_parser *parser, omp_clause_code kind,
 	  if (c == boolean_true_node)
 	    {
 	      warning_at (loc, 0,
-			  "%<%s%> value must be positive", str);
+			  "%qs value must be positive", str);
 	      expr = integer_one_node;
 	    }
 
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 3e7316f..a8d45bd 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -8777,13 +8777,13 @@ grokfndecl (tree ctype,
 	  if (long_long_unsigned_p)
 	    {
 	      if (cpp_interpret_int_suffix (parse_in, suffix, strlen (suffix)))
-		warning (0, "integer suffix %<%s%>"
+		warning (0, "integer suffix %qs"
 			    " shadowed by implementation", suffix);
 	    }
 	  else if (long_double_p)
 	    {
 	      if (cpp_interpret_float_suffix (parse_in, suffix, strlen (suffix)))
-		warning (0, "floating point suffix %<%s%>"
+		warning (0, "floating point suffix %qs"
 			    " shadowed by implementation", suffix);
 	    }
 	}
@@ -11838,7 +11838,7 @@ grokdeclarator (const cp_declarator *declarator,
 		     headers because glibc uses them.  */;
 		else if (name)
 		  pedwarn (input_location, OPT_Wpedantic,
-			   "ISO C++ forbids flexible array member %<%s%>", name);
+			   "ISO C++ forbids flexible array member %qs", name);
 		else
 		  pedwarn (input_location, OPT_Wpedantic,
 			   "ISO C++ forbids flexible array members");
diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index 0e5eb85..0c74c87 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -1139,7 +1139,7 @@ warn_types_mismatch (tree t1, tree t2, location_t loc1, location_t loc2)
       if (name1 && name2 && strcmp (name1, name2))
 	{
 	  inform (loc_t1,
-		  "type name %<%s%> should match type name %<%s%>",
+		  "type name %qs should match type name %qs",
 		  name1, name2);
 	  if (loc_t2_useful)
 	    inform (loc_t2,
diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
index 5cc2002..2ceb86f 100644
--- a/gcc/ipa-pure-const.c
+++ b/gcc/ipa-pure-const.c
@@ -184,8 +184,8 @@ suggest_attribute (int option, tree decl, bool known_finite,
   warning_at (DECL_SOURCE_LOCATION (decl),
 	      option,
 	      known_finite
-	      ? _("function might be candidate for attribute %<%s%>")
-	      : _("function might be candidate for attribute %<%s%>"
+	      ? _("function might be candidate for attribute %qs")
+	      : _("function might be candidate for attribute %qs"
 		  " if it is known to return normally"), attrib_name);
   return warned_about;
 }
-- 
1.8.5.3

[PATCH 5/7] fortran: remove trailing exclamation mark from various diagnostics (PR fortran/79852)

[David Malcolm]

PR fortran/79852 reports various Fortran diagnostics that have a trailing
exclamation mark in their messages.

Is there a Fortran-specific convention here?

Otherwise, this patch removes the trailing exclamation marks.

gcc/fortran/ChangeLog:
	PR fortran/79852
	* bbt.c (insert): Remove trailing exclamation mark from message.
	* decl.c (gfc_match_final_decl): Likewise.
	* dump-parse-tree.c (show_expr): Likewise.
	* module.c (gfc_use_module): Likewise.
	* primary.c (build_actual_constructor): Likewise.
	(gfc_convert_to_structure_constructor): Likewise.
---
 gcc/fortran/bbt.c             |  2 +-
 gcc/fortran/decl.c            |  2 +-
 gcc/fortran/dump-parse-tree.c |  2 +-
 gcc/fortran/module.c          |  2 +-
 gcc/fortran/primary.c         | 12 ++++++------
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/gcc/fortran/bbt.c b/gcc/fortran/bbt.c
index 37b5dc3..5cf5ef5 100644
--- a/gcc/fortran/bbt.c
+++ b/gcc/fortran/bbt.c
@@ -116,7 +116,7 @@ insert (gfc_bbt *new_bbt, gfc_bbt *t, compare_fn compare)
 	t = rotate_left (t);
     }
   else /* if (c == 0)  */
-    gfc_internal_error("insert_bbt(): Duplicate key found!");
+    gfc_internal_error("insert_bbt(): Duplicate key found");
 
   return t;
 }
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 52de1c1..93fadca 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -9950,7 +9950,7 @@ gfc_match_final_decl (void)
       for (f = block->f2k_derived->finalizers; f; f = f->next)
 	if (f->proc_sym == sym)
 	  {
-	    gfc_error ("%qs at %C is already defined as FINAL procedure!",
+	    gfc_error ("%qs at %C is already defined as FINAL procedure",
 		       name);
 	    return MATCH_ERROR;
 	  }
diff --git a/gcc/fortran/dump-parse-tree.c b/gcc/fortran/dump-parse-tree.c
index 87a5304..ede599d 100644
--- a/gcc/fortran/dump-parse-tree.c
+++ b/gcc/fortran/dump-parse-tree.c
@@ -573,7 +573,7 @@ show_expr (gfc_expr *p)
 
 	default:
 	  gfc_internal_error
-	    ("show_expr(): Bad intrinsic in expression!");
+	    ("show_expr(): Bad intrinsic in expression");
 	}
 
       show_expr (p->value.op.op1);
diff --git a/gcc/fortran/module.c b/gcc/fortran/module.c
index 5515fed..cc4e12f 100644
--- a/gcc/fortran/module.c
+++ b/gcc/fortran/module.c
@@ -6996,7 +6996,7 @@ gfc_use_module (gfc_use_list *module)
   for (p = gfc_state_stack; p; p = p->previous)
     if ((p->state == COMP_MODULE || p->state == COMP_SUBMODULE)
 	 && strcmp (p->sym->name, module_name) == 0)
-      gfc_fatal_error ("Can't USE the same %smodule we're building!",
+      gfc_fatal_error ("Can't USE the same %smodule we're building",
 		       p->state == COMP_SUBMODULE ? "sub" : "");
 
   init_pi_tree ();
diff --git a/gcc/fortran/primary.c b/gcc/fortran/primary.c
index d7fc6c4..7b5ac5f 100644
--- a/gcc/fortran/primary.c
+++ b/gcc/fortran/primary.c
@@ -2677,7 +2677,7 @@ build_actual_constructor (gfc_structure_ctor_component **comp_head,
 	  else if (!comp->attr.artificial)
 	    {
 	      gfc_error ("No initializer for component %qs given in the"
-			 " structure constructor at %C!", comp->name);
+			 " structure constructor at %C", comp->name);
 	      return false;
 	    }
 	}
@@ -2760,13 +2760,13 @@ gfc_convert_to_structure_constructor (gfc_expr *e, gfc_symbol *sym, gfc_expr **c
 	    {
 	      if (last_name)
 		gfc_error ("Component initializer without name after component"
-			   " named %s at %L!", last_name,
+			   " named %s at %L", last_name,
 			   actual->expr ? &actual->expr->where
 					: &gfc_current_locus);
 	      else
 		gfc_error ("Too many components in structure constructor at "
-			   "%L!", actual->expr ? &actual->expr->where
-					       : &gfc_current_locus);
+			   "%L", actual->expr ? &actual->expr->where
+					      : &gfc_current_locus);
 	      goto cleanup;
 	    }
 
@@ -2802,7 +2802,7 @@ gfc_convert_to_structure_constructor (gfc_expr *e, gfc_symbol *sym, gfc_expr **c
 	  if (!strcmp (comp_iter->name, comp_tail->name))
 	    {
 	      gfc_error ("Component %qs is initialized twice in the structure"
-			 " constructor at %L!", comp_tail->name,
+			 " constructor at %L", comp_tail->name,
 			 comp_tail->val ? &comp_tail->where
 					: &gfc_current_locus);
 	      goto cleanup;
@@ -2814,7 +2814,7 @@ gfc_convert_to_structure_constructor (gfc_expr *e, gfc_symbol *sym, gfc_expr **c
 	  && gfc_is_coindexed (comp_tail->val))
      	{
 	  gfc_error ("Coindexed expression to pointer component %qs in "
-		     "structure constructor at %L!", comp_tail->name,
+		     "structure constructor at %L", comp_tail->name,
 		     &comp_tail->where);
 	  goto cleanup;
 	}
-- 
1.8.5.3

[PATCH 1/7] Add missing punctuation to message (PR driver/79875)

[David Malcolm]

gcc/ChangeLog:
	PR driver/79875
	* opts.c (parse_sanitizer_options): Add missing question mark to
	"did you mean" message.
---
 gcc/opts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/opts.c b/gcc/opts.c
index 8274fab..6ea57af 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1640,7 +1640,7 @@ parse_sanitizer_options (const char *p, location_t loc, int scode,
 	  if (hint)
 	    error_at (loc,
 		      "unrecognized argument to -f%ssanitize%s= option: %q.*s;"
-		      " did you mean %qs",
+		      " did you mean %qs?",
 		      value ? "" : "no-",
 		      code == OPT_fsanitize_ ? "" : "-recover",
 		      (int) len, p, hint);
-- 
1.8.5.3

[PATCH 6/7] i386.c: make "sorry" message more amenable to translation (PR target/79926)

[David Malcolm]

PR target/79926 notes that in:

    sorry ("%s instructions aren't allowed in %s service routine",
           isa, (cfun->machine->func_type == TYPE_EXCEPTION
                 ? "exception" : "interrupt"));

the text from the second %s won't be translated, but should be.

This patch reworks the diagnostic by breaking it out into two messages
and marking them with G_() so they're seen by xgettext; a test run of
xgettext confirms that both messages make it into po/gcc.pot (in an
earlier version of the patch I attempted to do it without introducing
a local, but xgettext only picked up on one of the strings).

gcc/ChangeLog:
	PR target/79926
	* config/i386/i386.c (ix86_set_current_function): Make "sorry"
	message more amenable to translation.
---
 gcc/config/i386/i386.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index e705a3e..b8688e3 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -7271,9 +7271,15 @@ ix86_set_current_function (tree fndecl)
       if (isa != NULL)
 	{
 	  if (cfun->machine->func_type != TYPE_NORMAL)
-	    sorry ("%s instructions aren't allowed in %s service routine",
-		   isa, (cfun->machine->func_type == TYPE_EXCEPTION
-			 ? "exception" : "interrupt"));
+	    {
+	      const char *msgid
+		= (cfun->machine->func_type == TYPE_EXCEPTION
+		   ? G_("%s instructions aren't allowed in"
+			" exception service routine")
+		   : G_("%s instructions aren't allowed in"
+			" interrupt service routine"));
+	      sorry (msgid, isa);
+	    }
 	  else
 	    sorry ("%s instructions aren't allowed in function with "
 		   "no_caller_saved_registers attribute", isa);
-- 
1.8.5.3

[PATCH 3/7] Remove trailing period from various diagnostic messages (PR translation/79923)

[David Malcolm]

gcc/ChangeLog:
	PR translation/79923
	* auto-profile.c (get_combined_location): Convert leading
	character of diagnostics to lower case and remove trailing period.
	(read_profile): Likewise for various diagnostics.
	* config/arm/arm-builtins.c (arm_expand_builtin): Remove trailing
	period from various diagnostics.
	* config/arm/arm.c (arm_option_override): Likewise.
	* config/msp430/msp430.c (msp430_expand_delay_cycles): Likewise.
	(msp430_expand_delay_cycles): Likewise.
---
 gcc/auto-profile.c            | 14 ++++++-------
 gcc/config/arm/arm-builtins.c | 48 +++++++++++++++++++++----------------------
 gcc/config/arm/arm.c          |  2 +-
 gcc/config/msp430/msp430.c    |  4 ++--
 4 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/gcc/auto-profile.c b/gcc/auto-profile.c
index 6255f94..4b21340 100644
--- a/gcc/auto-profile.c
+++ b/gcc/auto-profile.c
@@ -344,7 +344,7 @@ get_combined_location (location_t loc, tree decl)
 {
   /* TODO: allow more bits for line and less bits for discriminator.  */
   if (LOCATION_LINE (loc) - DECL_SOURCE_LINE (decl) >= (1<<16))
-    warning_at (loc, OPT_Woverflow, "Offset exceeds 16 bytes.");
+    warning_at (loc, OPT_Woverflow, "offset exceeds 16 bytes");
   return ((LOCATION_LINE (loc) - DECL_SOURCE_LINE (decl)) << 16);
 }
 
@@ -917,13 +917,13 @@ read_profile (void)
 {
   if (gcov_open (auto_profile_file, 1) == 0)
     {
-      error ("Cannot open profile file %s.", auto_profile_file);
+      error ("cannot open profile file %s", auto_profile_file);
       return;
     }
 
   if (gcov_read_unsigned () != GCOV_DATA_MAGIC)
     {
-      error ("AutoFDO profile magic number does not match.");
+      error ("AutoFDO profile magic number does not match");
       return;
     }
 
@@ -931,7 +931,7 @@ read_profile (void)
   unsigned version = gcov_read_unsigned ();
   if (version != AUTO_PROFILE_VERSION)
     {
-      error ("AutoFDO profile version %u does match %u.",
+      error ("AutoFDO profile version %u does match %u",
 	     version, AUTO_PROFILE_VERSION);
       return;
     }
@@ -943,7 +943,7 @@ read_profile (void)
   afdo_string_table = new string_table ();
   if (!afdo_string_table->read())
     {
-      error ("Cannot read string table from %s.", auto_profile_file);
+      error ("cannot read string table from %s", auto_profile_file);
       return;
     }
 
@@ -951,7 +951,7 @@ read_profile (void)
   afdo_source_profile = autofdo_source_profile::create ();
   if (afdo_source_profile == NULL)
     {
-      error ("Cannot read function profile from %s.", auto_profile_file);
+      error ("cannot read function profile from %s", auto_profile_file);
       return;
     }
 
@@ -961,7 +961,7 @@ read_profile (void)
   /* Read in the working set.  */
   if (gcov_read_unsigned () != GCOV_TAG_AFDO_WORKING_SET)
     {
-      error ("Cannot read working set from %s.", auto_profile_file);
+      error ("cannot read working set from %s", auto_profile_file);
       return;
     }
 
diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c
index 689219c..661c028 100644
--- a/gcc/config/arm/arm-builtins.c
+++ b/gcc/config/arm/arm-builtins.c
@@ -2990,60 +2990,60 @@ arm_expand_builtin (tree exp,
 	      && (imm < 0 || imm > 32))
 	    {
 	      if (fcode == ARM_BUILTIN_WRORHI)
-		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_rori_pi16 in code.");
+		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_rori_pi16 in code");
 	      else if (fcode == ARM_BUILTIN_WRORWI)
-		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_rori_pi32 in code.");
+		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_rori_pi32 in code");
 	      else if (fcode == ARM_BUILTIN_WRORH)
-		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_ror_pi16 in code.");
+		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_ror_pi16 in code");
 	      else
-		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_ror_pi32 in code.");
+		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_ror_pi32 in code");
 	    }
 	  else if ((fcode == ARM_BUILTIN_WRORDI || fcode == ARM_BUILTIN_WRORD)
 		   && (imm < 0 || imm > 64))
 	    {
 	      if (fcode == ARM_BUILTIN_WRORDI)
-		error ("the range of count should be in 0 to 64.  please check the intrinsic _mm_rori_si64 in code.");
+		error ("the range of count should be in 0 to 64.  please check the intrinsic _mm_rori_si64 in code");
 	      else
-		error ("the range of count should be in 0 to 64.  please check the intrinsic _mm_ror_si64 in code.");
+		error ("the range of count should be in 0 to 64.  please check the intrinsic _mm_ror_si64 in code");
 	    }
 	  else if (imm < 0)
 	    {
 	      if (fcode == ARM_BUILTIN_WSRLHI)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_srli_pi16 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_srli_pi16 in code");
 	      else if (fcode == ARM_BUILTIN_WSRLWI)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_srli_pi32 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_srli_pi32 in code");
 	      else if (fcode == ARM_BUILTIN_WSRLDI)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_srli_si64 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_srli_si64 in code");
 	      else if (fcode == ARM_BUILTIN_WSLLHI)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_slli_pi16 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_slli_pi16 in code");
 	      else if (fcode == ARM_BUILTIN_WSLLWI)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_slli_pi32 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_slli_pi32 in code");
 	      else if (fcode == ARM_BUILTIN_WSLLDI)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_slli_si64 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_slli_si64 in code");
 	      else if (fcode == ARM_BUILTIN_WSRAHI)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_srai_pi16 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_srai_pi16 in code");
 	      else if (fcode == ARM_BUILTIN_WSRAWI)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_srai_pi32 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_srai_pi32 in code");
 	      else if (fcode == ARM_BUILTIN_WSRADI)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_srai_si64 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_srai_si64 in code");
 	      else if (fcode == ARM_BUILTIN_WSRLH)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_srl_pi16 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_srl_pi16 in code");
 	      else if (fcode == ARM_BUILTIN_WSRLW)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_srl_pi32 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_srl_pi32 in code");
 	      else if (fcode == ARM_BUILTIN_WSRLD)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_srl_si64 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_srl_si64 in code");
 	      else if (fcode == ARM_BUILTIN_WSLLH)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_sll_pi16 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_sll_pi16 in code");
 	      else if (fcode == ARM_BUILTIN_WSLLW)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_sll_pi32 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_sll_pi32 in code");
 	      else if (fcode == ARM_BUILTIN_WSLLD)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_sll_si64 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_sll_si64 in code");
 	      else if (fcode == ARM_BUILTIN_WSRAH)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_sra_pi16 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_sra_pi16 in code");
 	      else if (fcode == ARM_BUILTIN_WSRAW)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_sra_pi32 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_sra_pi32 in code");
 	      else
-		error ("the count should be no less than 0.  please check the intrinsic _mm_sra_si64 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_sra_si64 in code");
 	    }
 	}
       return arm_expand_binop_builtin (icode, exp, target);
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index b397a73..511e163 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -3370,7 +3370,7 @@ arm_option_override (void)
   if (arm_fp16_inst)
     {
       if (arm_fp16_format == ARM_FP16_FORMAT_ALTERNATIVE)
-	error ("selected fp16 options are incompatible.");
+	error ("selected fp16 options are incompatible");
       arm_fp16_format = ARM_FP16_FORMAT_IEEE;
     }
 
diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index 15d9678..710a97b 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -2501,7 +2501,7 @@ msp430_expand_delay_cycles (rtx arg)
     {
       if (c < 0)
 	{
-	  error ("__delay_cycles only takes non-negative cycle counts.");
+	  error ("__delay_cycles only takes non-negative cycle counts");
 	  return NULL_RTX;
 	}
     }
@@ -2521,7 +2521,7 @@ msp430_expand_delay_cycles (rtx arg)
 	  c %= 4;
 	  if ((unsigned long long) i > 0xffffffffULL)
 	    {
-	      error ("__delay_cycles is limited to 32-bit loop counts.");
+	      error ("__delay_cycles is limited to 32-bit loop counts");
 	      return NULL_RTX;
 	    }
 	}
-- 
1.8.5.3

[PATCH 4/7] c-indentation.c: workaround xgettext limitation (PR c/79921)

[David Malcolm]

xgettext wasn't locating the message:

	("...this statement, but the latter is misleadingly indented"
	 " as if it were guarded by the %qs"),

when building gcc.pot, and so this message was not being translated.

It appears the root cause is the parens around the concatenated
literals.  A fix would be to convert it to usage of the G_ macro,
but this patch simply removes the parens.

gcc/c-family/ChangeLog:
	PR c/79921
	* c-indentation.c (warn_for_misleading_indentation): Remove parens
	from inform's message, so that xgettext can locate it.
---
 gcc/c-family/c-indentation.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c
index 329f470..8300788 100644
--- a/gcc/c-family/c-indentation.c
+++ b/gcc/c-family/c-indentation.c
@@ -607,8 +607,8 @@ warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
 		      "this %qs clause does not guard...",
 		      guard_tinfo_to_string (guard_tinfo)))
 	inform (next_tinfo.location,
-		("...this statement, but the latter is misleadingly indented"
-		 " as if it were guarded by the %qs"),
+		"...this statement, but the latter is misleadingly indented"
+		" as if it were guarded by the %qs",
 		guard_tinfo_to_string (guard_tinfo));
     }
 }
-- 
1.8.5.3

[PATCH 2/7] aarch64.c: tweaks to quoting in error messages (PR target/79925)

[David Malcolm]

gcc/ChangeLog:
	PR target/79925
	* config/aarch64/aarch64.c (aarch64_validate_mcpu): Quote the
	full command-line argument, rather than just "str".
	(aarch64_validate_march): Likewise.
	(aarch64_validate_mtune): Likewise.
---
 gcc/config/aarch64/aarch64.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 714bb79..a069427 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -8748,14 +8748,14 @@ aarch64_validate_mcpu (const char *str, const struct processor **res,
   switch (parse_res)
     {
       case AARCH64_PARSE_MISSING_ARG:
-	error ("missing cpu name in -mcpu=%qs", str);
+	error ("missing cpu name in %<-mcpu=%s%>", str);
 	break;
       case AARCH64_PARSE_INVALID_ARG:
 	error ("unknown value %qs for -mcpu", str);
 	aarch64_print_hint_for_core (str);
 	break;
       case AARCH64_PARSE_INVALID_FEATURE:
-	error ("invalid feature modifier in -mcpu=%qs", str);
+	error ("invalid feature modifier in %<-mcpu=%s%>", str);
 	break;
       default:
 	gcc_unreachable ();
@@ -8782,14 +8782,14 @@ aarch64_validate_march (const char *str, const struct processor **res,
   switch (parse_res)
     {
       case AARCH64_PARSE_MISSING_ARG:
-	error ("missing arch name in -march=%qs", str);
+	error ("missing arch name in %<-march=%s%>", str);
 	break;
       case AARCH64_PARSE_INVALID_ARG:
 	error ("unknown value %qs for -march", str);
 	aarch64_print_hint_for_arch (str);
 	break;
       case AARCH64_PARSE_INVALID_FEATURE:
-	error ("invalid feature modifier in -march=%qs", str);
+	error ("invalid feature modifier in %<-march=%s%>", str);
 	break;
       default:
 	gcc_unreachable ();
@@ -8815,7 +8815,7 @@ aarch64_validate_mtune (const char *str, const struct processor **res)
   switch (parse_res)
     {
       case AARCH64_PARSE_MISSING_ARG:
-	error ("missing cpu name in -mtune=%qs", str);
+	error ("missing cpu name in %<-mtune=%s%>", str);
 	break;
       case AARCH64_PARSE_INVALID_ARG:
 	error ("unknown value %qs for -mtune", str);
-- 
1.8.5.3

Re: [PATCH 0/7] Various i18n fixes (and questions)

[Gerald Pfeifer]

On Thu, 9 Mar 2017, David Malcolm wrote:
> However, we're deep in stage 4 of the development cycle.  Looking at
> our development plan
>   https://gcc.gnu.org/develop.html
> it's not clear to me how such changes fit into our schedule: the plan
> seems to make no mention of how i18n and translation fit in to the
> stages (it talks about bugs and documentation, but not about translatable
> strings).

A key reason documentation changes of all sorts are fine is that there 
is very little risk for them to break the build on some platform, but
not others.  (Of course different versions of makeinfo may run into
different things, but that, too, will be noticed and can be fixed
quickly.  And does not happen all that often.)

So the question I'd ask:  What is the risk of such changes breaking
things?  If it's not big, probably makes sense to just proceed with
your changes.

> Do we have a "string freeze" in our schedule?  i.e. is there a point
> at which we avoid changing strings to avoid disrupting things for
> translators?

I am not aware of one, though I'd say latest after the branch point 
we probably should be careful in not changing things too much (unless
something is really broken)?

(GCC is not exactly for end users, and most people using it, in
particular a brand new release, will understand English when push 
comes to shove, which is what's shown if a message is not translated,
correct?)

> Also, from a developer POV, when should we regenerate and check-in
> the .pot files?  The rules for submitting patches:
>   https://gcc.gnu.org/contribute.html
> and for committing them:
>   https://gcc.gnu.org/svnwrite.html
> seem to make no mention of gettext and .pot files.

That's a good point.  Joseph, can you help fill that in?  (If you
give me a bit of text, I can take care of HTMLifying and adding it.)

Gerald

Re: [PATCH 1/7] Add missing punctuation to message (PR driver/79875)

[Martin Sebor]

On 03/09/2017 10:45 AM, David Malcolm wrote:
> gcc/ChangeLog:
> 	PR driver/79875
> 	* opts.c (parse_sanitizer_options): Add missing question mark to
> 	"did you mean" message.
> ---
>  gcc/opts.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/opts.c b/gcc/opts.c
> index 8274fab..6ea57af 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -1640,7 +1640,7 @@ parse_sanitizer_options (const char *p, location_t loc, int scode,
>  	  if (hint)
>  	    error_at (loc,
>  		      "unrecognized argument to -f%ssanitize%s= option: %q.*s;"
> -		      " did you mean %qs",
> +		      " did you mean %qs?",

I have just an observation/question here for future consideration.
If this sort of diagnostic is common (I count 23 instances of it)
or if it is expected to become common, would it make sense to add
a directive for it to the pretty printer to ensure consistency?
I.e., to automatically prepend "; did you mean" and append the
"?" to the new directive?

My search shows the following forms:

1) Space before "?"
gcc/c-family/c-common.c: "%<<<%> in boolean context, did you mean %<<%> ?");

2) No question mark at the end:
gcc/git/gcc/opts.c: "unrecognized argument to -f%ssanitize%s= option: 
%q.*s;"
		      " did you mean %qs",
		      value ? "" : "no-",

3) Missing space after semicolon:

gcc/c/c-decl.c: G_("implicit declaration of function %qE;did you mean 
%qs?"),

4) Explicit quotes:
gcc/gensupport.c: message_at (loc, "(did you mean \"%s\"?)",

(though this won't be fixed by the suggested directive).

Martin

Re: [PATCH 6/7] i386.c: make "sorry" message more amenable to translation (PR target/79926)

[Martin Sebor]

On 03/09/2017 10:45 AM, David Malcolm wrote:
> PR target/79926 notes that in:
>
>     sorry ("%s instructions aren't allowed in %s service routine",
>            isa, (cfun->machine->func_type == TYPE_EXCEPTION
>                  ? "exception" : "interrupt"));
>
> the text from the second %s won't be translated, but should be.
>
> This patch reworks the diagnostic by breaking it out into two messages
> and marking them with G_() so they're seen by xgettext; a test run of
> xgettext confirms that both messages make it into po/gcc.pot (in an
> earlier version of the patch I attempted to do it without introducing
> a local, but xgettext only picked up on one of the strings).
>
> gcc/ChangeLog:
> 	PR target/79926
> 	* config/i386/i386.c (ix86_set_current_function): Make "sorry"
> 	message more amenable to translation.
> ---
>  gcc/config/i386/i386.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index e705a3e..b8688e3 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -7271,9 +7271,15 @@ ix86_set_current_function (tree fndecl)
>        if (isa != NULL)
>  	{
>  	  if (cfun->machine->func_type != TYPE_NORMAL)
> -	    sorry ("%s instructions aren't allowed in %s service routine",
> -		   isa, (cfun->machine->func_type == TYPE_EXCEPTION
> -			 ? "exception" : "interrupt"));
> +	    {
> +	      const char *msgid
> +		= (cfun->machine->func_type == TYPE_EXCEPTION
> +		   ? G_("%s instructions aren't allowed in"
> +			" exception service routine")
> +		   : G_("%s instructions aren't allowed in"
> +			" interrupt service routine"));
> +	      sorry (msgid, isa);
> +	    }
>  	  else
>  	    sorry ("%s instructions aren't allowed in function with "
>  		   "no_caller_saved_registers attribute", isa);
>

Just a very minor point but I think one in line with the rest
of the problems addressed by these patches:  in the last sorry,
no_caller_saved_registers should be quoted in %< and %>.

(Also, although it's often left out, the other message above
seem like they could do with an article:

   instructions aren't allowed in <ins>an </ins>interrupt service
   routine)

Martin

Re: [PATCH 1/7] Add missing punctuation to message (PR driver/79875)

[Jakub Jelinek]

On Thu, Mar 09, 2017 at 12:45:23PM -0500, David Malcolm wrote:
> gcc/ChangeLog:
> 	PR driver/79875
> 	* opts.c (parse_sanitizer_options): Add missing question mark to
> 	"did you mean" message.
> ---
>  gcc/opts.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/opts.c b/gcc/opts.c
> index 8274fab..6ea57af 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -1640,7 +1640,7 @@ parse_sanitizer_options (const char *p, location_t loc, int scode,
>  	  if (hint)
>  	    error_at (loc,
>  		      "unrecognized argument to -f%ssanitize%s= option: %q.*s;"
> -		      " did you mean %qs",
> +		      " did you mean %qs?",
>  		      value ? "" : "no-",
>  		      code == OPT_fsanitize_ ? "" : "-recover",
>  		      (int) len, p, hint);

Ok, thanks.

	Jakub

Re: [PATCH 3/7] Remove trailing period from various diagnostic messages (PR translation/79923)

[Jakub Jelinek]

On Thu, Mar 09, 2017 at 12:45:25PM -0500, David Malcolm wrote:
> gcc/ChangeLog:
> 	PR translation/79923
> 	* auto-profile.c (get_combined_location): Convert leading
> 	character of diagnostics to lower case and remove trailing period.
> 	(read_profile): Likewise for various diagnostics.
> 	* config/arm/arm-builtins.c (arm_expand_builtin): Remove trailing
> 	period from various diagnostics.
> 	* config/arm/arm.c (arm_option_override): Likewise.
> 	* config/msp430/msp430.c (msp430_expand_delay_cycles): Likewise.
> 	(msp430_expand_delay_cycles): Likewise.

Mostly ok, but for

> --- a/gcc/config/arm/arm-builtins.c
> +++ b/gcc/config/arm/arm-builtins.c
> @@ -2990,60 +2990,60 @@ arm_expand_builtin (tree exp,
>  	      && (imm < 0 || imm > 32))
>  	    {
>  	      if (fcode == ARM_BUILTIN_WRORHI)
> -		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_rori_pi16 in code.");
> +		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_rori_pi16 in code");

I wonder if this shouldn't use a semicolon space in the middle
instead of dot space space (many times in the same file).
Also, for the benefit of translators, this might be better done as
		error ("the range of count should be in 0 to 32; please check the intrinsic %s in code",
		       "_mm_rori_pi16");
so that there are fewer of these messages.
Adding some ARM folks on this.

Perhaps commit everything except arm-builtins.c separately and deal with
this part with the ARM folks?

	Jakub

Re: [PATCH 4/7] c-indentation.c: workaround xgettext limitation (PR c/79921)

[Jakub Jelinek]

On Thu, Mar 09, 2017 at 12:45:26PM -0500, David Malcolm wrote:
> xgettext wasn't locating the message:
> 
> 	("...this statement, but the latter is misleadingly indented"
> 	 " as if it were guarded by the %qs"),
> 
> when building gcc.pot, and so this message was not being translated.
> 
> It appears the root cause is the parens around the concatenated
> literals.  A fix would be to convert it to usage of the G_ macro,
> but this patch simply removes the parens.
> 
> gcc/c-family/ChangeLog:
> 	PR c/79921
> 	* c-indentation.c (warn_for_misleading_indentation): Remove parens
> 	from inform's message, so that xgettext can locate it.

Ok, thanks.

	Jakub

Re: [PATCH 6/7] i386.c: make "sorry" message more amenable to translation (PR target/79926)

[Jakub Jelinek]

On Thu, Mar 09, 2017 at 12:45:28PM -0500, David Malcolm wrote:
> PR target/79926 notes that in:
> 
>     sorry ("%s instructions aren't allowed in %s service routine",
>            isa, (cfun->machine->func_type == TYPE_EXCEPTION
>                  ? "exception" : "interrupt"));
> 
> the text from the second %s won't be translated, but should be.
> 
> This patch reworks the diagnostic by breaking it out into two messages
> and marking them with G_() so they're seen by xgettext; a test run of
> xgettext confirms that both messages make it into po/gcc.pot (in an
> earlier version of the patch I attempted to do it without introducing
> a local, but xgettext only picked up on one of the strings).
> 
> gcc/ChangeLog:
> 	PR target/79926
> 	* config/i386/i386.c (ix86_set_current_function): Make "sorry"
> 	message more amenable to translation.
> ---
>  gcc/config/i386/i386.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index e705a3e..b8688e3 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -7271,9 +7271,15 @@ ix86_set_current_function (tree fndecl)
>        if (isa != NULL)
>  	{
>  	  if (cfun->machine->func_type != TYPE_NORMAL)
> -	    sorry ("%s instructions aren't allowed in %s service routine",
> -		   isa, (cfun->machine->func_type == TYPE_EXCEPTION
> -			 ? "exception" : "interrupt"));
> +	    {
> +	      const char *msgid
> +		= (cfun->machine->func_type == TYPE_EXCEPTION
> +		   ? G_("%s instructions aren't allowed in"
> +			" exception service routine")
> +		   : G_("%s instructions aren't allowed in"
> +			" interrupt service routine"));
> +	      sorry (msgid, isa);

1) aren't should be actually aren%'t
   (we should probably look through gcc.pot and patch all spots that
   have n't instead of n%'t in them and are in the gcc-internal-format)
2) I think it should be better to do:
	      sorry (cfun->machine->func_type == TYPE_EXCEPTION
		     ? G_("%s instructions aren%'t allowed in exception "
			  "service routine")
		     : G_("%s instructions aren%'t allowed in interrupt "
			  "service routine"));
   That way, you don't introduce another -Wformat-security issue
Ok for trunk with those changes.

	Jakub

Re: [PATCH 7/7] Simplify uses of "%<%s%>" to "%qs" (PR translation/79848)

[Jakub Jelinek]

On Thu, Mar 09, 2017 at 12:45:29PM -0500, David Malcolm wrote:
> PR translation/79848 requests that uses of "%<%s%>" in diagnostic messages
> be simplified to "%qs", presumably to make it easier for translators
> to deal with them.
> 
> This patch does this for all such strings found in the source tree.
> 
> gcc/c-family/ChangeLog:
> 	PR translation/79848
> 	* c-format.c (check_format_string): Simplify uses of "%<%s%>" to
> 	"%qs".
> 
> gcc/c/ChangeLog:
> 	PR translation/79848
> 	* c-decl.c (declspecs_add_type): Simplify uses of "%<%s%>" to
> 	"%qs".
> 	* c-parser.c (c_parser_oacc_shape_clause): Likewise.
> 
> gcc/cp/ChangeLog:
> 	PR translation/79848
> 	* decl.c (grokfndecl): Simplify uses of "%<%s%>" to "%qs".
> 
> gcc/ChangeLog:
> 	PR translation/79848
> 	* ipa-devirt.c (warn_types_mismatch): Simplify uses of "%<%s%>" to
> 	"%qs".
> 	* ipa-pure-const.c (suggest_attribute): Likewise.

Mostly ok, just:

> --- a/gcc/ipa-pure-const.c
> +++ b/gcc/ipa-pure-const.c
> @@ -184,8 +184,8 @@ suggest_attribute (int option, tree decl, bool known_finite,
>    warning_at (DECL_SOURCE_LOCATION (decl),
>  	      option,
>  	      known_finite
> -	      ? _("function might be candidate for attribute %<%s%>")
> -	      : _("function might be candidate for attribute %<%s%>"
> +	      ? _("function might be candidate for attribute %qs")
> +	      : _("function might be candidate for attribute %qs"
>  		  " if it is known to return normally"), attrib_name);
>    return warned_about;

I think we should replace _( with G_( here, we don't want to translate
twice.

Ok with that change.

	Jakub

Re: [PATCH 3/7] Remove trailing period from various diagnostic messages (PR translation/79923)

[Kyrill Tkachov]

On 10/03/17 06:24, Jakub Jelinek wrote:
> On Thu, Mar 09, 2017 at 12:45:25PM -0500, David Malcolm wrote:
>> gcc/ChangeLog:
>> 	PR translation/79923
>> 	* auto-profile.c (get_combined_location): Convert leading
>> 	character of diagnostics to lower case and remove trailing period.
>> 	(read_profile): Likewise for various diagnostics.
>> 	* config/arm/arm-builtins.c (arm_expand_builtin): Remove trailing
>> 	period from various diagnostics.
>> 	* config/arm/arm.c (arm_option_override): Likewise.
>> 	* config/msp430/msp430.c (msp430_expand_delay_cycles): Likewise.
>> 	(msp430_expand_delay_cycles): Likewise.
> Mostly ok, but for
>
>> --- a/gcc/config/arm/arm-builtins.c
>> +++ b/gcc/config/arm/arm-builtins.c
>> @@ -2990,60 +2990,60 @@ arm_expand_builtin (tree exp,
>>   	      && (imm < 0 || imm > 32))
>>   	    {
>>   	      if (fcode == ARM_BUILTIN_WRORHI)
>> -		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_rori_pi16 in code.");
>> +		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_rori_pi16 in code");
> I wonder if this shouldn't use a semicolon space in the middle
> instead of dot space space (many times in the same file).

Is there a convention in GCC to use semicolons?
I'm okay with changing it to a semicolon (it's slightly better IMO) as long as it's consistent
with the style GCC uses.

> Also, for the benefit of translators, this might be better done as
> 		error ("the range of count should be in 0 to 32; please check the intrinsic %s in code",
> 		       "_mm_rori_pi16");
> so that there are fewer of these messages.
> Adding some ARM folks on this.

These iWMMXt builtins haven't been touched in ages and could do with some TLC in general.
For example, I'm not a fan of having all these "please check the intrinsic ..." messages.
If we've got a reference to the tree we're expanding, isn't there some kind of error function
that will point to the location in the source that's causing the error? I'd rather use that than
hardcoding the intrinsic names. This would also allow us to collapse all these
if (fcode == <...>)
   error (...);
else if (fcode == <...>)
   error (...);
else if ...

constructs.

While we're at it:

  	      if (fcode == ARM_BUILTIN_WSRLHI)
-		error ("the count should be no less than 0.  please check the intrinsic _mm_srli_pi16 in code.");
+		error ("the count should be no less than 0.  please check the intrinsic _mm_srli_pi16 in code");


Let's use "the count should be a non-negative integer" to be consistent with the error reporting
for UInteger error messages.

> Perhaps commit everything except arm-builtins.c separately and deal with
> this part with the ARM folks?

I agree. David, if you want to clean up the error reporting in these intrinsics as a separate patch I'd be grateful.
Otherwise, could you please open a bugzilla ticket so we can track this?

Thanks,
Kyrill

> 	Jakub

Re: [PATCH 3/7] Remove trailing period from various diagnostic messages (PR translation/79923)

[Jakub Jelinek]

On Fri, Mar 10, 2017 at 09:24:18AM +0000, Kyrill Tkachov wrote:
> 
> On 10/03/17 06:24, Jakub Jelinek wrote:
> > On Thu, Mar 09, 2017 at 12:45:25PM -0500, David Malcolm wrote:
> > > gcc/ChangeLog:
> > > 	PR translation/79923
> > > 	* auto-profile.c (get_combined_location): Convert leading
> > > 	character of diagnostics to lower case and remove trailing period.
> > > 	(read_profile): Likewise for various diagnostics.
> > > 	* config/arm/arm-builtins.c (arm_expand_builtin): Remove trailing
> > > 	period from various diagnostics.
> > > 	* config/arm/arm.c (arm_option_override): Likewise.
> > > 	* config/msp430/msp430.c (msp430_expand_delay_cycles): Likewise.
> > > 	(msp430_expand_delay_cycles): Likewise.
> > Mostly ok, but for
> > 
> > > --- a/gcc/config/arm/arm-builtins.c
> > > +++ b/gcc/config/arm/arm-builtins.c
> > > @@ -2990,60 +2990,60 @@ arm_expand_builtin (tree exp,
> > >   	      && (imm < 0 || imm > 32))
> > >   	    {
> > >   	      if (fcode == ARM_BUILTIN_WRORHI)
> > > -		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_rori_pi16 in code.");
> > > +		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_rori_pi16 in code");
> > I wonder if this shouldn't use a semicolon space in the middle
> > instead of dot space space (many times in the same file).
> 
> Is there a convention in GCC to use semicolons?
> I'm okay with changing it to a semicolon (it's slightly better IMO) as long as it's consistent
> with the style GCC uses.

We have tons of messages like:
invalid --param name %qs; did you mean %qs?

	Jakub

Re: [PATCH 3/7] Remove trailing period from various diagnostic messages (PR translation/79923)

[Kyrill Tkachov]

On 10/03/17 09:30, Jakub Jelinek wrote:
> On Fri, Mar 10, 2017 at 09:24:18AM +0000, Kyrill Tkachov wrote:
>> On 10/03/17 06:24, Jakub Jelinek wrote:
>>> On Thu, Mar 09, 2017 at 12:45:25PM -0500, David Malcolm wrote:
>>>> gcc/ChangeLog:
>>>> 	PR translation/79923
>>>> 	* auto-profile.c (get_combined_location): Convert leading
>>>> 	character of diagnostics to lower case and remove trailing period.
>>>> 	(read_profile): Likewise for various diagnostics.
>>>> 	* config/arm/arm-builtins.c (arm_expand_builtin): Remove trailing
>>>> 	period from various diagnostics.
>>>> 	* config/arm/arm.c (arm_option_override): Likewise.
>>>> 	* config/msp430/msp430.c (msp430_expand_delay_cycles): Likewise.
>>>> 	(msp430_expand_delay_cycles): Likewise.
>>> Mostly ok, but for
>>>
>>>> --- a/gcc/config/arm/arm-builtins.c
>>>> +++ b/gcc/config/arm/arm-builtins.c
>>>> @@ -2990,60 +2990,60 @@ arm_expand_builtin (tree exp,
>>>>    	      && (imm < 0 || imm > 32))
>>>>    	    {
>>>>    	      if (fcode == ARM_BUILTIN_WRORHI)
>>>> -		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_rori_pi16 in code.");
>>>> +		error ("the range of count should be in 0 to 32.  please check the intrinsic _mm_rori_pi16 in code");
>>> I wonder if this shouldn't use a semicolon space in the middle
>>> instead of dot space space (many times in the same file).
>> Is there a convention in GCC to use semicolons?
>> I'm okay with changing it to a semicolon (it's slightly better IMO) as long as it's consistent
>> with the style GCC uses.
> We have tons of messages like:
> invalid --param name %qs; did you mean %qs?

Thanks, then using a semicolon here is the right thing to do.
Kyrill

> 	Jakub

Re: [PATCH 1/7] Add missing punctuation to message (PR driver/79875)

[David Malcolm]

On Thu, 2017-03-09 at 21:12 -0700, Martin Sebor wrote:
> On 03/09/2017 10:45 AM, David Malcolm wrote:
> > gcc/ChangeLog:
> > 	PR driver/79875
> > 	* opts.c (parse_sanitizer_options): Add missing question mark
> > to
> > 	"did you mean" message.
> > ---
> >  gcc/opts.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/gcc/opts.c b/gcc/opts.c
> > index 8274fab..6ea57af 100644
> > --- a/gcc/opts.c
> > +++ b/gcc/opts.c
> > @@ -1640,7 +1640,7 @@ parse_sanitizer_options (const char *p,
> > location_t loc, int scode,
> >  	  if (hint)
> >  	    error_at (loc,
> >  		      "unrecognized argument to -f%ssanitize%s=
> > option: %q.*s;"
> > -		      " did you mean %qs",
> > +		      " did you mean %qs?",
> 
> I have just an observation/question here for future consideration.
> If this sort of diagnostic is common (I count 23 instances of it)
> or if it is expected to become common, would it make sense to add
> a directive for it to the pretty printer to ensure consistency?
> I.e., to automatically prepend "; did you mean" and append the
> "?" to the new directive?

Interesting idea.  The "did you mean" messages tend to be of the form:

  if (hint)
    error ("SOME MESSAGE; did you mean SOME_FORMAT_ARG?",
           ARGS, TO, MESSAGE, HINT_THAT_MATCHES_FORMAT_ARG);
  else
    error ("SOME MESSAGE",
           ARGS, TO, MESSAGE);

which is kind of a pain due to the duplication.  It might be nicer to
eliminate the repetition by making the hint optional, replacing the
conditional with a new directive (e.g. %H):

  error ("SOME MESSAGE%H",
         ARGS, TO, MESSAGE, HINT);

where %H (or whatever) prepends the "; did you mean <FORMATTED_HINT>?"
if HINT is non-NULL, and doesn't if HINT is NULL.

Is this amenable to translation?

(I'm not sure if %H is already in use).

That said, I don't know how many more of these hints we'll add; I think
we've implemented all of the obvious ones now, so this might be over
-engineering at this point.

Some warts here, in addition to the ones you note below:

* it's usually %qs for the hint (11 places), but it's sometimes %qE (3
places), and in one place (gcc.c) it's "%<-%s%>".

> My search shows the following forms:
> 
> 1) Space before "?"
> gcc/c-family/c-common.c: "%<<<%> in boolean context, did you mean
> %<<%> ?");

That extra space feels like an error: the punctuation is quoted, so
it's not ambiguous.

> 2) No question mark at the end:
> gcc/git/gcc/opts.c: "unrecognized argument to -f%ssanitize%s= option:
> %q.*s;"
> 		      " did you mean %qs",
> 		      value ? "" : "no-",

Fixed by the patch.

> 3) Missing space after semicolon:
> 
> gcc/c/c-decl.c: G_("implicit declaration of function %qE;did you mean
> %qs?"),

I'd view that as a bug.

> 4) Explicit quotes:
> gcc/gensupport.c: message_at (loc, "(did you mean \"%s\"?)",
> 
> (though this won't be fixed by the suggested directive).

Indeed, this one ultimately uses vfprintf.

> Martin

Re: [PATCH 3/7] Remove trailing period from various diagnostic messages (PR translation/79923)

[David Malcolm]

On Fri, 2017-03-10 at 09:24 +0000, Kyrill Tkachov wrote:
> On 10/03/17 06:24, Jakub Jelinek wrote:
> > On Thu, Mar 09, 2017 at 12:45:25PM -0500, David Malcolm wrote:
> > > gcc/ChangeLog:
> > > 	PR translation/79923
> > > 	* auto-profile.c (get_combined_location): Convert leading
> > > 	character of diagnostics to lower case and remove trailing
> > > period.
> > > 	(read_profile): Likewise for various diagnostics.
> > > 	* config/arm/arm-builtins.c (arm_expand_builtin): Remove
> > > trailing
> > > 	period from various diagnostics.
> > > 	* config/arm/arm.c (arm_option_override): Likewise.
> > > 	* config/msp430/msp430.c (msp430_expand_delay_cycles):
> > > Likewise.
> > > 	(msp430_expand_delay_cycles): Likewise.
> > Mostly ok, but for
> > 
> > > --- a/gcc/config/arm/arm-builtins.c
> > > +++ b/gcc/config/arm/arm-builtins.c
> > > @@ -2990,60 +2990,60 @@ arm_expand_builtin (tree exp,
> > >   	      && (imm < 0 || imm > 32))
> > >   	    {
> > >   	      if (fcode == ARM_BUILTIN_WRORHI)
> > > -		error ("the range of count should be in 0 to 32.
> > >   please check the intrinsic _mm_rori_pi16 in code.");
> > > +		error ("the range of count should be in 0 to 32.
> > >   please check the intrinsic _mm_rori_pi16 in code");
> > I wonder if this shouldn't use a semicolon space in the middle
> > instead of dot space space (many times in the same file).
> 
> Is there a convention in GCC to use semicolons?
> I'm okay with changing it to a semicolon (it's slightly better IMO)
> as long as it's consistent
> with the style GCC uses.
> 
> > Also, for the benefit of translators, this might be better done as
> > 		error ("the range of count should be in 0 to 32; please
> > check the intrinsic %s in code",
> > 		       "_mm_rori_pi16");
> > so that there are fewer of these messages.
> > Adding some ARM folks on this.
> 
> These iWMMXt builtins haven't been touched in ages and could do with
> some TLC in general.
> For example, I'm not a fan of having all these "please check the
> intrinsic ..." messages.
> If we've got a reference to the tree we're expanding, isn't there
> some kind of error function
> that will point to the location in the source that's causing the
> error? I'd rather use that than
> hardcoding the intrinsic names. This would also allow us to collapse
> all these
> if (fcode == <...>)
>    error (...);
> else if (fcode == <...>)
>    error (...);
> else if ...
> 
> constructs.
> 
> While we're at it:
> 
>   	      if (fcode == ARM_BUILTIN_WSRLHI)
> -		error ("the count should be no less than 0.  please
> check the intrinsic _mm_srli_pi16 in code.");
> +		error ("the count should be no less than 0.  please
> check the intrinsic _mm_srli_pi16 in code");
> 
> 
> Let's use "the count should be a non-negative integer" to be
> consistent with the error reporting
> for UInteger error messages.
> 
> > Perhaps commit everything except arm-builtins.c separately and deal
> > with
> > this part with the ARM folks?
> 
> I agree. David, if you want to clean up the error reporting in these
> intrinsics as a separate patch I'd be grateful.
> Otherwise, could you please open a bugzilla ticket so we can track 
> this?

I started looking at this, but realized I don't have the arm ISA
expertise to touch this code (sorry), so I filed
  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79995
instead.

Re: [PATCH 0/7] Various i18n fixes (and questions)

[Joseph Myers]

On Thu, 9 Mar 2017, David Malcolm wrote:

> However, we're deep in stage 4 of the development cycle.  Looking at
> our development plan
>   https://gcc.gnu.org/develop.html
> it's not clear to me how such changes fit into our schedule: the plan
> seems to make no mention of how i18n and translation fit in to the
> stages (it talks about bugs and documentation, but not about translatable
> strings).
> 
> Do we have a "string freeze" in our schedule?  i.e. is there a point
> at which we avoid changing strings to avoid disrupting things for
> translators?

We don't have a string freeze; these strings are functionally considered 
much like documentation.

> Also, from a developer POV, when should we regenerate and check-in
> the .pot files?  The rules for submitting patches:
>   https://gcc.gnu.org/contribute.html
> and for committing them:
>   https://gcc.gnu.org/svnwrite.html
> seem to make no mention of gettext and .pot files.
> 
> Looking in libcpp/po/ChangeLog and gcc/po/ChangeLog I see that
> Joseph [CCed] seems to regularly commit regenerated copies of the
> .pot files; do we have a policy about this?

See translation.html.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 2/7] aarch64.c: tweaks to quoting in error messages (PR target/79925)

[Joseph Myers]

On Thu, 9 Mar 2017, David Malcolm wrote:

> gcc/ChangeLog:
> 	PR target/79925
> 	* config/aarch64/aarch64.c (aarch64_validate_mcpu): Quote the
> 	full command-line argument, rather than just "str".
> 	(aarch64_validate_march): Likewise.
> 	(aarch64_validate_mtune): Likewise.

OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

[PATCH 6/7 v2] i386.c: make "sorry" message more amenable to translation (PR target/79926)

[David Malcolm]

On Fri, 2017-03-10 at 07:33 +0100, Jakub Jelinek wrote:
> On Thu, Mar 09, 2017 at 12:45:28PM -0500, David Malcolm wrote:
> > PR target/79926 notes that in:
> >
> >     sorry ("%s instructions aren't allowed in %s service routine",
> >            isa, (cfun->machine->func_type == TYPE_EXCEPTION
> >                  ? "exception" : "interrupt"));
> >
> > the text from the second %s won't be translated, but should be.
> >
> > This patch reworks the diagnostic by breaking it out into two
> > messages
> > and marking them with G_() so they're seen by xgettext; a test run
> > of
> > xgettext confirms that both messages make it into po/gcc.pot (in an
> > earlier version of the patch I attempted to do it without
> > introducing
> > a local, but xgettext only picked up on one of the strings).
> >
> > gcc/ChangeLog:
> > 	PR target/79926
> > 	* config/i386/i386.c (ix86_set_current_function): Make "sorry"
> > 	message more amenable to translation.
> > ---
> >  gcc/config/i386/i386.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> > index e705a3e..b8688e3 100644
> > --- a/gcc/config/i386/i386.c
> > +++ b/gcc/config/i386/i386.c
> > @@ -7271,9 +7271,15 @@ ix86_set_current_function (tree fndecl)
> >        if (isa != NULL)
> >  	{
> >  	  if (cfun->machine->func_type != TYPE_NORMAL)
> > -	    sorry ("%s instructions aren't allowed in %s service
> > routine",
> > -		   isa, (cfun->machine->func_type ==
> > TYPE_EXCEPTION
> > -			 ? "exception" : "interrupt"));
> > +	    {
> > +	      const char *msgid
> > +		= (cfun->machine->func_type == TYPE_EXCEPTION
> > +		   ? G_("%s instructions aren't allowed in"
> > +			" exception service routine")
> > +		   : G_("%s instructions aren't allowed in"
> > +			" interrupt service routine"));
> > +	      sorry (msgid, isa);
>
> 1) aren't should be actually aren%'t
>    (we should probably look through gcc.pot and patch all spots that
>    have n't instead of n%'t in them and are in the gcc-internal
> -format)
> 2) I think it should be better to do:
> 	      sorry (cfun->machine->func_type == TYPE_EXCEPTION
> 		     ? G_("%s instructions aren%'t allowed in exception
> "
> 			  "service routine")
> 		     : G_("%s instructions aren%'t allowed in interrupt
> "
> 			  "service routine"));
>    That way, you don't introduce another -Wformat-security issue
> Ok for trunk with those changes.

Thanks.

Martin pointed out that the wording of these messages could be improved
by adding an article, and by adding quotes to "no_caller_saved_registers"

Here's a revised version that makes those changes (in addition to
the ones you suggested).

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/ChangeLog:
	PR target/79926
	* config/i386/i386.c (ix86_set_current_function): Make "sorry"
	messages more amenable to translation, and improve wording.

gcc/testsuite/ChangeLog:
	PR target/79926
	* gcc.target/i386/interrupt-387-err-1.c: Update expected message.
	* gcc.target/i386/interrupt-387-err-2.c: Likewise.
	* gcc.target/i386/interrupt-bnd-err-1.c: Likewise.
	* gcc.target/i386/interrupt-bnd-err-2.c: Likewise.
	* gcc.target/i386/interrupt-mmx-err-1.c: Likewise.
	* gcc.target/i386/interrupt-mmx-err-2.c: Likewise.
---
 gcc/config/i386/i386.c                              | 13 ++++++++-----
 gcc/testsuite/gcc.target/i386/interrupt-387-err-1.c |  4 ++--
 gcc/testsuite/gcc.target/i386/interrupt-387-err-2.c |  2 +-
 gcc/testsuite/gcc.target/i386/interrupt-bnd-err-1.c |  4 ++--
 gcc/testsuite/gcc.target/i386/interrupt-bnd-err-2.c |  2 +-
 gcc/testsuite/gcc.target/i386/interrupt-mmx-err-1.c |  4 ++--
 gcc/testsuite/gcc.target/i386/interrupt-mmx-err-2.c |  2 +-
 7 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index e705a3e..9fbf8d0 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -7271,12 +7271,15 @@ ix86_set_current_function (tree fndecl)
       if (isa != NULL)
 	{
 	  if (cfun->machine->func_type != TYPE_NORMAL)
-	    sorry ("%s instructions aren't allowed in %s service routine",
-		   isa, (cfun->machine->func_type == TYPE_EXCEPTION
-			 ? "exception" : "interrupt"));
+	    sorry (cfun->machine->func_type == TYPE_EXCEPTION
+		   ? G_("%s instructions aren%'t allowed in an"
+			" exception service routine")
+		   : G_("%s instructions aren%'t allowed in an"
+			" interrupt service routine"),
+		   isa);
 	  else
-	    sorry ("%s instructions aren't allowed in function with "
-		   "no_caller_saved_registers attribute", isa);
+	    sorry ("%s instructions aren%'t allowed in a function with "
+		   "the %<no_caller_saved_registers%> attribute", isa);
 	  /* Don't issue the same error twice.  */
 	  cfun->machine->func_type = TYPE_NORMAL;
 	  cfun->machine->no_caller_saved_registers = false;
diff --git a/gcc/testsuite/gcc.target/i386/interrupt-387-err-1.c b/gcc/testsuite/gcc.target/i386/interrupt-387-err-1.c
index 3fbdc88..8561a3c 100644
--- a/gcc/testsuite/gcc.target/i386/interrupt-387-err-1.c
+++ b/gcc/testsuite/gcc.target/i386/interrupt-387-err-1.c
@@ -6,11 +6,11 @@ typedef unsigned int uword_t __attribute__ ((mode (__word__)));
 void
 __attribute__((interrupt))
 fn1 (void *frame, uword_t error)
-{ /* { dg-message "80387 instructions aren't allowed in exception service routine" } */
+{ /* { dg-message "80387 instructions aren't allowed in an exception service routine" } */
 }
 
 void
 __attribute__((interrupt))
 fn2 (void *frame)
-{ /* { dg-message "80387 instructions aren't allowed in interrupt service routine" } */
+{ /* { dg-message "80387 instructions aren't allowed in an interrupt service routine" } */
 }
diff --git a/gcc/testsuite/gcc.target/i386/interrupt-387-err-2.c b/gcc/testsuite/gcc.target/i386/interrupt-387-err-2.c
index 3203d64..9810f18 100644
--- a/gcc/testsuite/gcc.target/i386/interrupt-387-err-2.c
+++ b/gcc/testsuite/gcc.target/i386/interrupt-387-err-2.c
@@ -4,5 +4,5 @@
 void
 __attribute__((no_caller_saved_registers))
 fn1 (void)
-{ /* { dg-message "80387 instructions aren't allowed in function with no_caller_saved_registers attribute" } */
+{ /* { dg-message "80387 instructions aren't allowed in a function with the 'no_caller_saved_registers' attribute" } */
 }
diff --git a/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-1.c b/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-1.c
index e07688e..1126fca 100644
--- a/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-1.c
+++ b/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-1.c
@@ -6,11 +6,11 @@ typedef unsigned int uword_t __attribute__ ((mode (__word__)));
 void
 __attribute__((interrupt))
 fn1 (void *frame)
-{ /* { dg-message "MPX instructions aren't allowed in interrupt service routine" } */
+{ /* { dg-message "MPX instructions aren't allowed in an interrupt service routine" } */
 }
 
 void
 __attribute__((interrupt))
 fn2 (void *frame, uword_t error)
-{ /* { dg-message "MPX instructions aren't allowed in exception service routine" } */
+{ /* { dg-message "MPX instructions aren't allowed in an exception service routine" } */
 }
diff --git a/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-2.c b/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-2.c
index 641ca63..5e2d1a6 100644
--- a/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-2.c
+++ b/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-2.c
@@ -4,5 +4,5 @@
 void
 __attribute__((no_caller_saved_registers))
 fn (void *frame)
-{ /* { dg-message "MPX instructions aren't allowed in function with no_caller_saved_registers attribute" } */
+{ /* { dg-message "MPX instructions aren't allowed in a function with the 'no_caller_saved_registers' attribute" } */
 }
diff --git a/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-1.c b/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-1.c
index cd1367b..8c14594 100644
--- a/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-1.c
+++ b/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-1.c
@@ -6,11 +6,11 @@ typedef unsigned int uword_t __attribute__ ((mode (__word__)));
 void
 __attribute__((interrupt))
 fn1 (void *frame)
-{ /* { dg-message "MMX/3Dnow instructions aren't allowed in interrupt service routine" } */
+{ /* { dg-message "MMX/3Dnow instructions aren't allowed in an interrupt service routine" } */
 }
 
 void
 __attribute__((interrupt))
 fn2 (void *frame, uword_t error)
-{ /* { dg-message "MMX/3Dnow instructions aren't allowed in exception service routine" } */
+{ /* { dg-message "MMX/3Dnow instructions aren't allowed in an exception service routine" } */
 }
diff --git a/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-2.c b/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-2.c
index 3e9f70c..073700e 100644
--- a/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-2.c
+++ b/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-2.c
@@ -4,5 +4,5 @@
 void
 __attribute__((no_caller_saved_registers))
 fn1 (void)
-{ /* { dg-message "MMX/3Dnow instructions aren't allowed in function with no_caller_saved_registers attribute" } */
+{ /* { dg-message "MMX/3Dnow instructions aren't allowed in a function with the 'no_caller_saved_registers' attribute" } */
 }
-- 
1.8.5.3

Re: [PATCH 1/7] Add missing punctuation to message (PR driver/79875)

[Roland Illig]

Am 10.03.2017 um 05:12 schrieb Martin Sebor:
> I have just an observation/question here for future consideration.
> If this sort of diagnostic is common (I count 23 instances of it)
> or if it is expected to become common, would it make sense to add
> a directive for it to the pretty printer to ensure consistency?
> I.e., to automatically prepend "; did you mean" and append the
> "?" to the new directive?

As a translator, I'm not too concerned about this one, although it looks
very similar to the repeated "%qs requires %qs", which often appears for
command line options.

My main argument is that the "did you mean" is not just a name for a
thing, but it is part of the sentence grammar. To me that feels like too
much magic for a rarely used case (compared to %D or %qE or %qT, which
appear so often that I'm now very familiar with them).

Also, when adding a placeholder like %H for it, there would be no
surrounding whitespace, similar to %K. That confused me as a translator
when I first saw it, since it is not nicely documented; neither in the
Gettext manual
(https://www.gnu.org/software/gettext/manual/gettext.html#gcc_002dinternal_002dformat)
nor in the GCC files implementing them. There should be at least some
examples of what each placeholder typically expands to.

Roland

Re: [PATCH 3/7] Remove trailing period from various diagnostic messages (PR translation/79923)

[Kyrill Tkachov]

On 10/03/17 23:36, David Malcolm wrote:
> On Fri, 2017-03-10 at 09:24 +0000, Kyrill Tkachov wrote:
>> On 10/03/17 06:24, Jakub Jelinek wrote:
>>> On Thu, Mar 09, 2017 at 12:45:25PM -0500, David Malcolm wrote:
>>>> gcc/ChangeLog:
>>>> 	PR translation/79923
>>>> 	* auto-profile.c (get_combined_location): Convert leading
>>>> 	character of diagnostics to lower case and remove trailing
>>>> period.
>>>> 	(read_profile): Likewise for various diagnostics.
>>>> 	* config/arm/arm-builtins.c (arm_expand_builtin): Remove
>>>> trailing
>>>> 	period from various diagnostics.
>>>> 	* config/arm/arm.c (arm_option_override): Likewise.
>>>> 	* config/msp430/msp430.c (msp430_expand_delay_cycles):
>>>> Likewise.
>>>> 	(msp430_expand_delay_cycles): Likewise.
>>> Mostly ok, but for
>>>
>>>> --- a/gcc/config/arm/arm-builtins.c
>>>> +++ b/gcc/config/arm/arm-builtins.c
>>>> @@ -2990,60 +2990,60 @@ arm_expand_builtin (tree exp,
>>>>    	      && (imm < 0 || imm > 32))
>>>>    	    {
>>>>    	      if (fcode == ARM_BUILTIN_WRORHI)
>>>> -		error ("the range of count should be in 0 to 32.
>>>>    please check the intrinsic _mm_rori_pi16 in code.");
>>>> +		error ("the range of count should be in 0 to 32.
>>>>    please check the intrinsic _mm_rori_pi16 in code");
>>> I wonder if this shouldn't use a semicolon space in the middle
>>> instead of dot space space (many times in the same file).
>> Is there a convention in GCC to use semicolons?
>> I'm okay with changing it to a semicolon (it's slightly better IMO)
>> as long as it's consistent
>> with the style GCC uses.
>>
>>> Also, for the benefit of translators, this might be better done as
>>> 		error ("the range of count should be in 0 to 32; please
>>> check the intrinsic %s in code",
>>> 		       "_mm_rori_pi16");
>>> so that there are fewer of these messages.
>>> Adding some ARM folks on this.
>> These iWMMXt builtins haven't been touched in ages and could do with
>> some TLC in general.
>> For example, I'm not a fan of having all these "please check the
>> intrinsic ..." messages.
>> If we've got a reference to the tree we're expanding, isn't there
>> some kind of error function
>> that will point to the location in the source that's causing the
>> error? I'd rather use that than
>> hardcoding the intrinsic names. This would also allow us to collapse
>> all these
>> if (fcode == <...>)
>>     error (...);
>> else if (fcode == <...>)
>>     error (...);
>> else if ...
>>
>> constructs.
>>
>> While we're at it:
>>
>>    	      if (fcode == ARM_BUILTIN_WSRLHI)
>> -		error ("the count should be no less than 0.  please
>> check the intrinsic _mm_srli_pi16 in code.");
>> +		error ("the count should be no less than 0.  please
>> check the intrinsic _mm_srli_pi16 in code");
>>
>>
>> Let's use "the count should be a non-negative integer" to be
>> consistent with the error reporting
>> for UInteger error messages.
>>
>>> Perhaps commit everything except arm-builtins.c separately and deal
>>> with
>>> this part with the ARM folks?
>> I agree. David, if you want to clean up the error reporting in these
>> intrinsics as a separate patch I'd be grateful.
>> Otherwise, could you please open a bugzilla ticket so we can track
>> this?
> I started looking at this, but realized I don't have the arm ISA
> expertise to touch this code (sorry), so I filed
>    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79995
> instead.

No worries, thanks for looking at this in the first place.

Kyrill

>

Re: [PATCH 1/7] Add missing punctuation to message (PR driver/79875)

[Bernhard Reutner-Fischer]

On 11 March 2017 11:28:46 CET, Roland Illig <roland.illig@gmx.de> wrote:
>Am 10.03.2017 um 05:12 schrieb Martin Sebor:
>> I have just an observation/question here for future consideration.
>> If this sort of diagnostic is common (I count 23 instances of it)
>> or if it is expected to become common, would it make sense to add
>> a directive for it to the pretty printer to ensure consistency?
>> I.e., to automatically prepend "; did you mean" and append the
>> "?" to the new directive?
>
>As a translator, I'm not too concerned about this one, although it
>looks
>very similar to the repeated "%qs requires %qs", which often appears
>for
>command line options.
>
>My main argument is that the "did you mean" is not just a name for a
>thing, but it is part of the sentence grammar. To me that feels like
>too
>much magic for a rarely used case (compared to %D or %qE or %qT, which
>appear so often that I'm now very familiar with them).
>
>Also, when adding a placeholder like %H for it, there would be no
>surrounding whitespace, similar to %K. That confused me as a translator
>when I first saw it, since it is not nicely documented; neither in the
>Gettext manual
>(https://www.gnu.org/software/gettext/manual/gettext.html#gcc_002dinternal_002dformat)
>nor in the GCC files implementing them. There should be at least some
>examples of what each placeholder typically expands to.

I think I suggested this a when the hints were introduced but Joseph declined the idea back then, FYI.

thanks,

Re: [PATCH 6/7 v2] i386.c: make "sorry" message more amenable to translation (PR target/79926)

[Jakub Jelinek]

On Fri, Mar 10, 2017 at 09:21:53PM -0500, David Malcolm wrote:
> Martin pointed out that the wording of these messages could be improved
> by adding an article, and by adding quotes to "no_caller_saved_registers"
> 
> Here's a revised version that makes those changes (in addition to
> the ones you suggested).
> 
> Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.
> 
> OK for trunk?
> 
> gcc/ChangeLog:
> 	PR target/79926
> 	* config/i386/i386.c (ix86_set_current_function): Make "sorry"
> 	messages more amenable to translation, and improve wording.
> 
> gcc/testsuite/ChangeLog:
> 	PR target/79926
> 	* gcc.target/i386/interrupt-387-err-1.c: Update expected message.
> 	* gcc.target/i386/interrupt-387-err-2.c: Likewise.
> 	* gcc.target/i386/interrupt-bnd-err-1.c: Likewise.
> 	* gcc.target/i386/interrupt-bnd-err-2.c: Likewise.
> 	* gcc.target/i386/interrupt-mmx-err-1.c: Likewise.
> 	* gcc.target/i386/interrupt-mmx-err-2.c: Likewise.

Sorry for the delay, I can't find this mail in my mailbox (grabbed it from
the archives).  This is ok for trunk.

> ---
>  gcc/config/i386/i386.c                              | 13 ++++++++-----
>  gcc/testsuite/gcc.target/i386/interrupt-387-err-1.c |  4 ++--
>  gcc/testsuite/gcc.target/i386/interrupt-387-err-2.c |  2 +-
>  gcc/testsuite/gcc.target/i386/interrupt-bnd-err-1.c |  4 ++--
>  gcc/testsuite/gcc.target/i386/interrupt-bnd-err-2.c |  2 +-
>  gcc/testsuite/gcc.target/i386/interrupt-mmx-err-1.c |  4 ++--
>  gcc/testsuite/gcc.target/i386/interrupt-mmx-err-2.c |  2 +-
>  7 files changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index e705a3e..9fbf8d0 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -7271,12 +7271,15 @@ ix86_set_current_function (tree fndecl)
>        if (isa != NULL)
>  	{
>  	  if (cfun->machine->func_type != TYPE_NORMAL)
> -	    sorry ("%s instructions aren't allowed in %s service routine",
> -		   isa, (cfun->machine->func_type == TYPE_EXCEPTION
> -			 ? "exception" : "interrupt"));
> +	    sorry (cfun->machine->func_type == TYPE_EXCEPTION
> +		   ? G_("%s instructions aren%'t allowed in an"
> +			" exception service routine")
> +		   : G_("%s instructions aren%'t allowed in an"
> +			" interrupt service routine"),
> +		   isa);
>  	  else
> -	    sorry ("%s instructions aren't allowed in function with "
> -		   "no_caller_saved_registers attribute", isa);
> +	    sorry ("%s instructions aren%'t allowed in a function with "
> +		   "the %<no_caller_saved_registers%> attribute", isa);
>  	  /* Don't issue the same error twice.  */
>  	  cfun->machine->func_type = TYPE_NORMAL;
>  	  cfun->machine->no_caller_saved_registers = false;
> diff --git a/gcc/testsuite/gcc.target/i386/interrupt-387-err-1.c b/gcc/testsuite/gcc.target/i386/interrupt-387-err-1.c
> index 3fbdc88..8561a3c 100644
> --- a/gcc/testsuite/gcc.target/i386/interrupt-387-err-1.c
> +++ b/gcc/testsuite/gcc.target/i386/interrupt-387-err-1.c
> @@ -6,11 +6,11 @@ typedef unsigned int uword_t __attribute__ ((mode (__word__)));
>  void
>  __attribute__((interrupt))
>  fn1 (void *frame, uword_t error)
> -{ /* { dg-message "80387 instructions aren't allowed in exception service routine" } */
> +{ /* { dg-message "80387 instructions aren't allowed in an exception service routine" } */
>  }
>  
>  void
>  __attribute__((interrupt))
>  fn2 (void *frame)
> -{ /* { dg-message "80387 instructions aren't allowed in interrupt service routine" } */
> +{ /* { dg-message "80387 instructions aren't allowed in an interrupt service routine" } */
>  }
> diff --git a/gcc/testsuite/gcc.target/i386/interrupt-387-err-2.c b/gcc/testsuite/gcc.target/i386/interrupt-387-err-2.c
> index 3203d64..9810f18 100644
> --- a/gcc/testsuite/gcc.target/i386/interrupt-387-err-2.c
> +++ b/gcc/testsuite/gcc.target/i386/interrupt-387-err-2.c
> @@ -4,5 +4,5 @@
>  void
>  __attribute__((no_caller_saved_registers))
>  fn1 (void)
> -{ /* { dg-message "80387 instructions aren't allowed in function with no_caller_saved_registers attribute" } */
> +{ /* { dg-message "80387 instructions aren't allowed in a function with the 'no_caller_saved_registers' attribute" } */
>  }
> diff --git a/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-1.c b/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-1.c
> index e07688e..1126fca 100644
> --- a/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-1.c
> +++ b/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-1.c
> @@ -6,11 +6,11 @@ typedef unsigned int uword_t __attribute__ ((mode (__word__)));
>  void
>  __attribute__((interrupt))
>  fn1 (void *frame)
> -{ /* { dg-message "MPX instructions aren't allowed in interrupt service routine" } */
> +{ /* { dg-message "MPX instructions aren't allowed in an interrupt service routine" } */
>  }
>  
>  void
>  __attribute__((interrupt))
>  fn2 (void *frame, uword_t error)
> -{ /* { dg-message "MPX instructions aren't allowed in exception service routine" } */
> +{ /* { dg-message "MPX instructions aren't allowed in an exception service routine" } */
>  }
> diff --git a/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-2.c b/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-2.c
> index 641ca63..5e2d1a6 100644
> --- a/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-2.c
> +++ b/gcc/testsuite/gcc.target/i386/interrupt-bnd-err-2.c
> @@ -4,5 +4,5 @@
>  void
>  __attribute__((no_caller_saved_registers))
>  fn (void *frame)
> -{ /* { dg-message "MPX instructions aren't allowed in function with no_caller_saved_registers attribute" } */
> +{ /* { dg-message "MPX instructions aren't allowed in a function with the 'no_caller_saved_registers' attribute" } */
>  }
> diff --git a/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-1.c b/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-1.c
> index cd1367b..8c14594 100644
> --- a/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-1.c
> +++ b/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-1.c
> @@ -6,11 +6,11 @@ typedef unsigned int uword_t __attribute__ ((mode (__word__)));
>  void
>  __attribute__((interrupt))
>  fn1 (void *frame)
> -{ /* { dg-message "MMX/3Dnow instructions aren't allowed in interrupt service routine" } */
> +{ /* { dg-message "MMX/3Dnow instructions aren't allowed in an interrupt service routine" } */
>  }
>  
>  void
>  __attribute__((interrupt))
>  fn2 (void *frame, uword_t error)
> -{ /* { dg-message "MMX/3Dnow instructions aren't allowed in exception service routine" } */
> +{ /* { dg-message "MMX/3Dnow instructions aren't allowed in an exception service routine" } */
>  }
> diff --git a/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-2.c b/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-2.c
> index 3e9f70c..073700e 100644
> --- a/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-2.c
> +++ b/gcc/testsuite/gcc.target/i386/interrupt-mmx-err-2.c
> @@ -4,5 +4,5 @@
>  void
>  __attribute__((no_caller_saved_registers))
>  fn1 (void)
> -{ /* { dg-message "MMX/3Dnow instructions aren't allowed in function with no_caller_saved_registers attribute" } */
> +{ /* { dg-message "MMX/3Dnow instructions aren't allowed in a function with the 'no_caller_saved_registers' attribute" } */
>  }
> -- 
> 1.8.5.3

	Jakub