[ARC,COMMITTED] Fix diagnostic messages.

Message ID 20190417110933.11244-1-claziss@gmail.com
State New
Headers show
Series
  • [ARC,COMMITTED] Fix diagnostic messages.
Related show

Commit Message

Claudiu Zissulescu April 17, 2019, 11:09 a.m.
Apply upper/dot rule on diagnostic messages.

gcc/
xxxx-xx-xx  Claudiu Zissulescu  <claziss@synopsys.com>

	* config/arc/arc.c (arc_init): Format diagnostic string.
	(arc_override_options): Likewise.
	(check_if_valid_regno_const): Likewise.
	(arc_reorg): Likewise.
---
 gcc/ChangeLog        |  7 +++++++
 gcc/config/arc/arc.c | 22 ++++++++++++----------
 2 files changed, 19 insertions(+), 10 deletions(-)

-- 
2.20.1

Comments

Jakub Jelinek April 17, 2019, 11:25 a.m. | #1
On Wed, Apr 17, 2019 at 02:09:33PM +0300, Claudiu Zissulescu wrote:
>    /* Warn for unimplemented PIC in pre-ARC700 cores, and disable flag_pic.  */

>    if (flag_pic && TARGET_ARC600_FAMILY)

>      {

>        warning (0,

> -	       "PIC is not supported for %s. Generating non-PIC code only..",

> +	       "PIC is not supported for %s.  Generating non-PIC code only",

>  	       arc_cpu_string);


I believe this is undesirable too.  Either use something like
"PIC is not supported for %s; generating non-PIC code only"
or split that into two messages
if (warning (0, "PIC is not supported for %s", arc_cpu_string))
  inform (input_location, "generating non-PIC code only");

> @@ -1222,26 +1222,26 @@ arc_override_options (void)

>    do {								\

>      if ((!(arc_selected_cpu->arch_info->flags & CODE))		\

>  	&& (VAR == VAL))					\

> -      error ("Option %s=%s is not available for %s CPU.",	\

> +      error ("option %s=%s is not available for %s CPU",	\

>  	     DOC0, DOC1, arc_selected_cpu->name);		\


I think another complaint in the PR was that it is unclear what
those DOC0/DOC1/DOC strings stand for, if they are keywords on what
one writes on the command line or similar (then it should be quoted,
%qs or %<%s=%s%>), if it is something different, then maybe it is
not the right thing to construct a translatable sentence from that
error/warning gmsgid string and one or more words that are inserted
somewhere into the sentence.  At least for the ARC_OPT the latter seems to
be the case, given e.g.:
ARC_OPT (FL_LL64,     (1ULL << 5), MASK_LL64,              "double load/store")
ARC_OPT (FL_BS,       (1ULL << 6), MASK_BARREL_SHIFTER,    "barrel shifter")
Is barrel shifter a keyword, or just random words added into the sentence?
If the latter, then the translators might want to translate that too, but in
that case together with the surroundings too.
ARC_OPT (FL_SPFP,     (1ULL << 12), MASK_SPFP_COMPACT_SET, "single precission FPX")
ARC_OPT (FL_DPFP,     (1ULL << 13), MASK_DPFP_COMPACT_SET, "double precission FPX")
has spelling errors,
s/precission/precision/g

>      if ((arc_selected_cpu->arch_info->dflags & CODE)		\

>  	&& (VAR != DEFAULT_##VAR)				\

>  	&& (VAR != VAL))					\

> -      warning (0, "Option %s is ignored, the default value %s"	\

> -	       " is considered for %s CPU.", DOC0, DOC1,	\

> +      warning (0, "option %s is ignored, the default value %s"	\

> +	       " is considered for %s CPU", DOC0, DOC1,		\

>  	       arc_selected_cpu->name);				\

>   } while (0);

>  #define ARC_OPT(NAME, CODE, MASK, DOC)				\

>    do {								\

>      if ((!(arc_selected_cpu->arch_info->flags & CODE))		\

>  	&& (target_flags & MASK))				\

> -      error ("Option %s is not available for %s CPU",		\

> +      error ("option %s is not available for %s CPU",		\

>  	     DOC, arc_selected_cpu->name);			\

>      if ((arc_selected_cpu->arch_info->dflags & CODE)		\

>  	&& (target_flags_explicit & MASK)			\

>  	&& (!(target_flags & MASK)))				\

> -      warning (0, "Unset option %s is ignored, it is always"	\

> -	       " enabled for %s CPU.", DOC,			\

> +      warning (0, "unset option %s is ignored, it is always"	\

> +	       " enabled for %s CPU", DOC,			\

>  	       arc_selected_cpu->name);				\

>    } while (0);

>  

> @@ -7268,7 +7268,8 @@ check_if_valid_regno_const (rtx *operands, int opno)

>      case CONST_INT :

>        return true;

>      default:

> -	error ("register number must be a compile-time constant. Try giving higher optimization levels");

> +	error ("register number must be a compile-time constant.  "

> +	       "Try giving higher optimization levels");


Similarly to the above case.

	Jakub
Marek Polacek April 17, 2019, 6:53 p.m. | #2
On Wed, Apr 17, 2019 at 01:25:05PM +0200, Jakub Jelinek wrote:
> On Wed, Apr 17, 2019 at 02:09:33PM +0300, Claudiu Zissulescu wrote:

> >    /* Warn for unimplemented PIC in pre-ARC700 cores, and disable flag_pic.  */

> >    if (flag_pic && TARGET_ARC600_FAMILY)

> >      {

> >        warning (0,

> > -	       "PIC is not supported for %s. Generating non-PIC code only..",

> > +	       "PIC is not supported for %s.  Generating non-PIC code only",

> >  	       arc_cpu_string);

> 

> I believe this is undesirable too.  Either use something like

> "PIC is not supported for %s; generating non-PIC code only"

> or split that into two messages

> if (warning (0, "PIC is not supported for %s", arc_cpu_string))

>   inform (input_location, "generating non-PIC code only");


And I suppose we should avoid pleonasm like "PIC code" ;).

Marek
Claudiu Zissulescu April 24, 2019, 9:08 a.m. | #3
The DOC/DOC0/DOC1 are like keywords to be placed into the warning
message. They shouldn't be translated as they can referenced directly
in the specific processor architectural options. So, I will use %qs
for them, and fix the other signalized problems.
Thank you

On Wed, Apr 17, 2019 at 2:25 PM Jakub Jelinek <jakub@redhat.com> wrote:
>

> On Wed, Apr 17, 2019 at 02:09:33PM +0300, Claudiu Zissulescu wrote:

> >    /* Warn for unimplemented PIC in pre-ARC700 cores, and disable flag_pic.  */

> >    if (flag_pic && TARGET_ARC600_FAMILY)

> >      {

> >        warning (0,

> > -            "PIC is not supported for %s. Generating non-PIC code only..",

> > +            "PIC is not supported for %s.  Generating non-PIC code only",

> >              arc_cpu_string);

>

> I believe this is undesirable too.  Either use something like

> "PIC is not supported for %s; generating non-PIC code only"

> or split that into two messages

> if (warning (0, "PIC is not supported for %s", arc_cpu_string))

>   inform (input_location, "generating non-PIC code only");

>

> > @@ -1222,26 +1222,26 @@ arc_override_options (void)

> >    do {                                                               \

> >      if ((!(arc_selected_cpu->arch_info->flags & CODE))               \

> >       && (VAR == VAL))                                        \

> > -      error ("Option %s=%s is not available for %s CPU.",    \

> > +      error ("option %s=%s is not available for %s CPU",     \

> >            DOC0, DOC1, arc_selected_cpu->name);               \

>

> I think another complaint in the PR was that it is unclear what

> those DOC0/DOC1/DOC strings stand for, if they are keywords on what

> one writes on the command line or similar (then it should be quoted,

> %qs or %<%s=%s%>), if it is something different, then maybe it is

> not the right thing to construct a translatable sentence from that

> error/warning gmsgid string and one or more words that are inserted

> somewhere into the sentence.  At least for the ARC_OPT the latter seems to

> be the case, given e.g.:

> ARC_OPT (FL_LL64,     (1ULL << 5), MASK_LL64,              "double load/store")

> ARC_OPT (FL_BS,       (1ULL << 6), MASK_BARREL_SHIFTER,    "barrel shifter")

> Is barrel shifter a keyword, or just random words added into the sentence?

> If the latter, then the translators might want to translate that too, but in

> that case together with the surroundings too.

> ARC_OPT (FL_SPFP,     (1ULL << 12), MASK_SPFP_COMPACT_SET, "single precission FPX")

> ARC_OPT (FL_DPFP,     (1ULL << 13), MASK_DPFP_COMPACT_SET, "double precission FPX")

> has spelling errors,

> s/precission/precision/g

>

> >      if ((arc_selected_cpu->arch_info->dflags & CODE)         \

> >       && (VAR != DEFAULT_##VAR)                               \

> >       && (VAR != VAL))                                        \

> > -      warning (0, "Option %s is ignored, the default value %s"       \

> > -            " is considered for %s CPU.", DOC0, DOC1,        \

> > +      warning (0, "option %s is ignored, the default value %s"       \

> > +            " is considered for %s CPU", DOC0, DOC1,         \

> >              arc_selected_cpu->name);                         \

> >   } while (0);

> >  #define ARC_OPT(NAME, CODE, MASK, DOC)                               \

> >    do {                                                               \

> >      if ((!(arc_selected_cpu->arch_info->flags & CODE))               \

> >       && (target_flags & MASK))                               \

> > -      error ("Option %s is not available for %s CPU",                \

> > +      error ("option %s is not available for %s CPU",                \

> >            DOC, arc_selected_cpu->name);                      \

> >      if ((arc_selected_cpu->arch_info->dflags & CODE)         \

> >       && (target_flags_explicit & MASK)                       \

> >       && (!(target_flags & MASK)))                            \

> > -      warning (0, "Unset option %s is ignored, it is always" \

> > -            " enabled for %s CPU.", DOC,                     \

> > +      warning (0, "unset option %s is ignored, it is always" \

> > +            " enabled for %s CPU", DOC,                      \

> >              arc_selected_cpu->name);                         \

> >    } while (0);

> >

> > @@ -7268,7 +7268,8 @@ check_if_valid_regno_const (rtx *operands, int opno)

> >      case CONST_INT :

> >        return true;

> >      default:

> > -     error ("register number must be a compile-time constant. Try giving higher optimization levels");

> > +     error ("register number must be a compile-time constant.  "

> > +            "Try giving higher optimization levels");

>

> Similarly to the above case.

>

>         Jakub

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 9480e693c08..3820fae8ee7 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@ 
+2019-04-17  Claudiu Zissulescu  <claziss@synopsys.com>
+
+	* config/arc/arc.c (arc_init): Format diagnostic string.
+	(arc_override_options): Likewise.
+	(check_if_valid_regno_const): Likewise.
+	(arc_reorg): Likewise.
+
 2019-04-17  Segher Boessenkool  <segher@kernel.crashing.org>
 
 	PR target/17108
diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index 65eef30747a..1a04f9ef793 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -950,13 +950,13 @@  arc_init (void)
   /* FPX-4.  No FPX extensions mixed with FPU extensions.  */
   if ((TARGET_DPFP_FAST_SET || TARGET_DPFP_COMPACT_SET || TARGET_SPFP)
       && TARGET_HARD_FLOAT)
-    error ("No FPX/FPU mixing allowed");
+    error ("no FPX/FPU mixing allowed");
 
   /* Warn for unimplemented PIC in pre-ARC700 cores, and disable flag_pic.  */
   if (flag_pic && TARGET_ARC600_FAMILY)
     {
       warning (0,
-	       "PIC is not supported for %s. Generating non-PIC code only..",
+	       "PIC is not supported for %s.  Generating non-PIC code only",
 	       arc_cpu_string);
       flag_pic = 0;
     }
@@ -1222,26 +1222,26 @@  arc_override_options (void)
   do {								\
     if ((!(arc_selected_cpu->arch_info->flags & CODE))		\
 	&& (VAR == VAL))					\
-      error ("Option %s=%s is not available for %s CPU.",	\
+      error ("option %s=%s is not available for %s CPU",	\
 	     DOC0, DOC1, arc_selected_cpu->name);		\
     if ((arc_selected_cpu->arch_info->dflags & CODE)		\
 	&& (VAR != DEFAULT_##VAR)				\
 	&& (VAR != VAL))					\
-      warning (0, "Option %s is ignored, the default value %s"	\
-	       " is considered for %s CPU.", DOC0, DOC1,	\
+      warning (0, "option %s is ignored, the default value %s"	\
+	       " is considered for %s CPU", DOC0, DOC1,		\
 	       arc_selected_cpu->name);				\
  } while (0);
 #define ARC_OPT(NAME, CODE, MASK, DOC)				\
   do {								\
     if ((!(arc_selected_cpu->arch_info->flags & CODE))		\
 	&& (target_flags & MASK))				\
-      error ("Option %s is not available for %s CPU",		\
+      error ("option %s is not available for %s CPU",		\
 	     DOC, arc_selected_cpu->name);			\
     if ((arc_selected_cpu->arch_info->dflags & CODE)		\
 	&& (target_flags_explicit & MASK)			\
 	&& (!(target_flags & MASK)))				\
-      warning (0, "Unset option %s is ignored, it is always"	\
-	       " enabled for %s CPU.", DOC,			\
+      warning (0, "unset option %s is ignored, it is always"	\
+	       " enabled for %s CPU", DOC,			\
 	       arc_selected_cpu->name);				\
   } while (0);
 
@@ -7268,7 +7268,8 @@  check_if_valid_regno_const (rtx *operands, int opno)
     case CONST_INT :
       return true;
     default:
-	error ("register number must be a compile-time constant. Try giving higher optimization levels");
+	error ("register number must be a compile-time constant.  "
+	       "Try giving higher optimization levels");
 	break;
     }
   return false;
@@ -8261,7 +8262,8 @@  arc_reorg (void)
       cfun->machine->ccfsm_current_insn = NULL_RTX;
 
       if (!INSN_ADDRESSES_SET_P())
-	  fatal_error (input_location, "Insn addresses not set after shorten_branches");
+	  fatal_error (input_location,
+		       "insn addresses not set after shorten_branches");
 
       for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
 	{