Message ID | 611a47aa-3f45-a4be-3aff-03879cf43c69@suse.cz |
---|---|
State | New |
Headers | show |
Series | fix diagnostic quoting/spelling in rs6000 | expand |
On Mon, May 20, 2019 at 11:48:45AM +0200, Martin Liška wrote: > --- a/gcc/config/rs6000/driver-rs6000.c > +++ b/gcc/config/rs6000/driver-rs6000.c > @@ -266,7 +266,7 @@ elf_platform (void) > fatal_error ( > input_location, > "Unsupported cpu name returned from kernel for " > - "%<-mcpu=native%>: %s\n" > + "%<-mcpu=native%>: %s. " > "Please use an explicit cpu name. Valid cpu names are: %s", > cpu, s); This one is still incorrect, shouldn't start with capital letter and there shouldn't be separate sentences and full stops. Either it can be split into two diagnostics, error ("unsupported cpu name returned from kernel for %<-mcpu=native%>: %s", cpu); fatal_error (input_location, "please use an explicit cpu name; valid cpu names are: %s", s); or reword somehow. Jakub
On 5/20/19 12:06 PM, Jakub Jelinek wrote: > On Mon, May 20, 2019 at 11:48:45AM +0200, Martin Liška wrote: >> --- a/gcc/config/rs6000/driver-rs6000.c >> +++ b/gcc/config/rs6000/driver-rs6000.c >> @@ -266,7 +266,7 @@ elf_platform (void) >> fatal_error ( >> input_location, >> "Unsupported cpu name returned from kernel for " >> - "%<-mcpu=native%>: %s\n" >> + "%<-mcpu=native%>: %s. " >> "Please use an explicit cpu name. Valid cpu names are: %s", >> cpu, s); > > This one is still incorrect, shouldn't start with capital letter and there > shouldn't be separate sentences and full stops. > Either it can be split into two diagnostics, > error ("unsupported cpu name returned from kernel for %<-mcpu=native%>: %s", cpu); > fatal_error (input_location, "please use an explicit cpu name; valid cpu names are: %s", s); > or reword somehow. > > Jakub > You are right. I'm fixing that in updated patch. Thanks, Martin
Hi! On Mon, May 20, 2019 at 11:48:45AM +0200, Martin Liška wrote: > * config/rs6000/driver-rs6000.c (elf_platform): Do not use > an extra newline. > * config/rs6000/rs6000-c.c (SYNTAX_ERROR): Wrap pragma in %<%>. Please say this is in rs6000_pragma_longcall. You might want to move the macro to inside the function, too. Or get rid of it, for bonus points :-) > (rs6000_pragma_longcall): Likewise/ Typo (slash instead of dot). > (altivec_resolve_overloaded_builtin): Likewise for vec_lvs*. Please spell out lvsl and lvsr. > * config/rs6000/rs6000.c (rs6000_option_override_internal): > Quote a C type. > (rs6000_function_arg): Likewise. > (rs6000_expand_set_fpscr_drn_builtin): Remove trailing dot. > (rs6000_expand_ternop_builtin): Use interval syntax. I find that syntax less readable, btw. Square brackets have a different meaning already, in C programs. Interval syntax isn't normally used for integers, fwiw (you use set notation, like {0...15}, instead). 0..15 is all of shorter, more readable, and less confusing than [0,15] or [0, 15] (ugh) or {0...15}. So yeah, I disapprove of that. But please do the same in rs6000 as in the rest of the compiler ;-) Okay for trunk, with the changelog nits, and Jakub's comment addressed. Thanks, Segher
On 5/20/19 12:52 PM, Segher Boessenkool wrote: > Hi! > > On Mon, May 20, 2019 at 11:48:45AM +0200, Martin Liška wrote: >> * config/rs6000/driver-rs6000.c (elf_platform): Do not use >> an extra newline. >> * config/rs6000/rs6000-c.c (SYNTAX_ERROR): Wrap pragma in %<%>. > > Please say this is in rs6000_pragma_longcall. You might want to move the > macro to inside the function, too. Or get rid of it, for bonus points :-) I've done the movement. > >> (rs6000_pragma_longcall): Likewise/ > > Typo (slash instead of dot). Fixed. > >> (altivec_resolve_overloaded_builtin): Likewise for vec_lvs*. > > Please spell out lvsl and lvsr. Likewise. > >> * config/rs6000/rs6000.c (rs6000_option_override_internal): >> Quote a C type. >> (rs6000_function_arg): Likewise. >> (rs6000_expand_set_fpscr_drn_builtin): Remove trailing dot. >> (rs6000_expand_ternop_builtin): Use interval syntax. > > I find that syntax less readable, btw. Square brackets have a different > meaning already, in C programs. Interval syntax isn't normally used for > integers, fwiw (you use set notation, like {0...15}, instead). 0..15 > is all of shorter, more readable, and less confusing than [0,15] or > [0, 15] (ugh) or {0...15}. > > So yeah, I disapprove of that. But please do the same in rs6000 as in > the rest of the compiler ;-) Yeah, the format was selected by Martin Sebor, where he transformed e.g. - error ("requested init_priority is out of range"); + error ("requested %<init_priority%> %i is out of range [0, %i]", I'm just following that and I bet there are multiple existing occurrences of the range notation at different places. > > > Okay for trunk, with the changelog nits, and Jakub's comment addressed. > > Thanks, Thanks for review. Martin > > > Segher >
Where you refer to 'homogeneous %<float%> aggregates', are such aggregates in the rs6000 case (or in the case where the ABI changed) really restricted to the type float, or do they apply more generally to some other floating-point types so that expanding "float" to a longer description would be the appropriate fix?
On Mon, May 20, 2019 at 11:31:17PM +0000, Joseph Myers wrote: > Where you refer to 'homogeneous %<float%> aggregates', are such aggregates > in the rs6000 case (or in the case where the ABI changed) really > restricted to the type float, or do they apply more generally to some > other floating-point types so that expanding "float" to a longer > description would be the appropriate fix? See https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00632.html . It's about single precision floating point only. Segher
diff --git a/gcc/config/rs6000/driver-rs6000.c b/gcc/config/rs6000/driver-rs6000.c index b290eea4bdd..b58c1c91fc0 100644 --- a/gcc/config/rs6000/driver-rs6000.c +++ b/gcc/config/rs6000/driver-rs6000.c @@ -266,7 +266,7 @@ elf_platform (void) fatal_error ( input_location, "Unsupported cpu name returned from kernel for " - "%<-mcpu=native%>: %s\n" + "%<-mcpu=native%>: %s. " "Please use an explicit cpu name. Valid cpu names are: %s", cpu, s); } diff --git a/gcc/config/rs6000/rs6000-c.c b/gcc/config/rs6000/rs6000-c.c index 7c28d4d8176..a898cf6c55a 100644 --- a/gcc/config/rs6000/rs6000-c.c +++ b/gcc/config/rs6000/rs6000-c.c @@ -49,7 +49,7 @@ #define SYNTAX_ERROR(gmsgid) do { \ warning (OPT_Wpragmas, gmsgid); \ - warning (OPT_Wpragmas, "ignoring malformed #pragma longcall"); \ + warning (OPT_Wpragmas, "ignoring malformed %<#pragma longcall%>"); \ return; \ } while (0) @@ -72,7 +72,7 @@ rs6000_pragma_longcall (cpp_reader *pfile ATTRIBUTE_UNUSED) SYNTAX_ERROR ("number must be 0 or 1"); if (pragma_lex (&x) != CPP_EOF) - warning (OPT_Wpragmas, "junk at end of #pragma longcall"); + warning (OPT_Wpragmas, "junk at end of %<#pragma longcall%>"); rs6000_default_long_calls = (n == integer_one_node); } @@ -6140,11 +6140,11 @@ altivec_resolve_overloaded_builtin (location_t loc, tree fndecl, /* vec_lvsl and vec_lvsr are deprecated for use with LE element order. */ if (fcode == ALTIVEC_BUILTIN_VEC_LVSL && !BYTES_BIG_ENDIAN) warning (OPT_Wdeprecated, - "vec_lvsl is deprecated for little endian; use " + "%<vec_lvsl%> is deprecated for little endian; use " "assignment for unaligned loads and stores"); else if (fcode == ALTIVEC_BUILTIN_VEC_LVSR && !BYTES_BIG_ENDIAN) warning (OPT_Wdeprecated, - "vec_lvsr is deprecated for little endian; use " + "%<vec_lvsr%> is deprecated for little endian; use " "assignment for unaligned loads and stores"); if (fcode == ALTIVEC_BUILTIN_VEC_MUL) @@ -6846,7 +6846,7 @@ altivec_resolve_overloaded_builtin (location_t loc, tree fndecl, { if (TYPE_READONLY (TREE_TYPE (type)) && !TYPE_READONLY (TREE_TYPE (decl_type))) - warning (0, "passing arg %d of %qE discards qualifiers from " + warning (0, "passing argument %d of %qE discards qualifiers from " "pointer target type", n + 1, fndecl); type = build_pointer_type (build_qualified_type (TREE_TYPE (type), 0)); diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 2992ba545d7..7e1b11ad43a 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -4275,7 +4275,7 @@ rs6000_option_override_internal (bool global_init_p) if (main_target_opt != NULL && (main_target_opt->x_rs6000_long_double_type_size != default_long_double_size)) - error ("target attribute or pragma changes long double size"); + error ("target attribute or pragma changes %<long double%> size"); else rs6000_long_double_type_size = default_long_double_size; } @@ -4310,9 +4310,11 @@ rs6000_option_override_internal (bool global_init_p) { warned_change_long_double = true; if (TARGET_IEEEQUAD) - warning (OPT_Wpsabi, "Using IEEE extended precision long double"); + warning (OPT_Wpsabi, "Using IEEE extended precision " + "%<long double%>"); else - warning (OPT_Wpsabi, "Using IBM extended precision long double"); + warning (OPT_Wpsabi, "Using IBM extended precision " + "%<long double%>"); } } } @@ -11791,7 +11793,7 @@ rs6000_function_arg (cumulative_args_t cum_v, machine_mode mode, { warned = true; inform (input_location, - "the ABI of passing homogeneous float aggregates" + "the ABI of passing homogeneous %<float%> aggregates" " has changed in GCC 5"); } } @@ -13227,7 +13229,7 @@ rs6000_expand_set_fpscr_drn_builtin (enum insn_code icode, tree exp) /* Builtin not supported in 32-bit mode. */ fatal_error (input_location, "%<__builtin_set_fpscr_drn%> is not supported " - "in 32-bit mode."); + "in 32-bit mode"); if (rs6000_isa_flags & OPTION_MASK_SOFT_FLOAT) { @@ -14250,7 +14252,7 @@ rs6000_expand_ternop_builtin (enum insn_code icode, tree exp, rtx target) if (TREE_CODE (arg2) != INTEGER_CST || wi::geu_p (wi::to_wide (arg2), 16)) { - error ("argument 3 must be in the range 0..15"); + error ("argument 3 must be in the range [0, 15]"); return CONST0_RTX (tmode); } } @@ -14383,7 +14385,7 @@ get_element_number (tree vec_type, tree arg) if (!tree_fits_uhwi_p (arg) || (elt = tree_to_uhwi (arg), elt > max)) { - error ("selector must be an integer constant in the range 0..%wi", max); + error ("selector must be an integer constant in the range [0, %wi]", max); return 0; } @@ -14703,7 +14705,7 @@ altivec_expand_builtin (tree exp, rtx target, bool *expandedp) if (TREE_CODE (arg1) != INTEGER_CST || TREE_INT_CST_LOW (arg1) > 12) { - error ("second argument to %qs must be 0..12", "vec_vextract4b"); + error ("second argument to %qs must be [0, 12]", "vec_vextract4b"); return expand_call (exp, target, false); } break; @@ -14718,7 +14720,7 @@ altivec_expand_builtin (tree exp, rtx target, bool *expandedp) if (TREE_CODE (arg2) != INTEGER_CST || TREE_INT_CST_LOW (arg2) > 12) { - error ("third argument to %qs must be 0..12", "vec_vinsert4b"); + error ("third argument to %qs must be [0, 12]", "vec_vinsert4b"); return expand_call (exp, target, false); } break; @@ -37238,7 +37240,7 @@ rs6000_get_function_versions_dispatcher (void *decl) #ifndef TARGET_LIBC_PROVIDES_HWCAP_IN_TCB error_at (DECL_SOURCE_LOCATION (default_node->decl), - "target_clones attribute needs GLIBC (2.23 and newer) that " + "%<target_clones%> attribute needs GLIBC (2.23 and newer) that " "exports hardware capability bits"); #else diff --git a/gcc/testsuite/gcc.target/powerpc/ppc64-abi-warn-1.c b/gcc/testsuite/gcc.target/powerpc/ppc64-abi-warn-1.c index c70c14c2858..ae7d5746b0d 100644 --- a/gcc/testsuite/gcc.target/powerpc/ppc64-abi-warn-1.c +++ b/gcc/testsuite/gcc.target/powerpc/ppc64-abi-warn-1.c @@ -6,7 +6,7 @@ struct f8 float x[8]; }; -void test (struct f8 a, struct f8 b) /* { dg-message "note: the ABI of passing homogeneous float aggregates has changed" } */ +void test (struct f8 a, struct f8 b) /* { dg-message "note: the ABI of passing homogeneous 'float' aggregates has changed" } */ { } diff --git a/gcc/testsuite/gcc.target/powerpc/pr80315-1.c b/gcc/testsuite/gcc.target/powerpc/pr80315-1.c index d0fb6dc1c06..2f670a0c6c7 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr80315-1.c +++ b/gcc/testsuite/gcc.target/powerpc/pr80315-1.c @@ -10,6 +10,6 @@ main() int mask; /* Argument 2 must be 0 or 1. Argument 3 must be in range 0..15. */ - res = __builtin_crypto_vshasigmaw (test, 1, 0xff); /* { dg-error "argument 3 must be in the range 0..15" } */ + res = __builtin_crypto_vshasigmaw (test, 1, 0xff); /* { dg-error "argument 3 must be in the range \\[0, 15\\]" } */ return 0; } diff --git a/gcc/testsuite/gcc.target/powerpc/pr80315-2.c b/gcc/testsuite/gcc.target/powerpc/pr80315-2.c index 30df7f18658..e95187e2179 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr80315-2.c +++ b/gcc/testsuite/gcc.target/powerpc/pr80315-2.c @@ -10,6 +10,6 @@ main () int mask; /* Argument 2 must be 0 or 1. Argument 3 must be in range 0..15. */ - res = __builtin_crypto_vshasigmad (test, 1, 0xff); /* { dg-error "argument 3 must be in the range 0..15" } */ + res = __builtin_crypto_vshasigmad (test, 1, 0xff); /* { dg-error "argument 3 must be in the range \\[0, 15\\]" } */ return 0; } diff --git a/gcc/testsuite/gcc.target/powerpc/pr80315-3.c b/gcc/testsuite/gcc.target/powerpc/pr80315-3.c index 5365f1d516c..f1a782663a1 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr80315-3.c +++ b/gcc/testsuite/gcc.target/powerpc/pr80315-3.c @@ -12,6 +12,6 @@ main () int mask; /* Argument 2 must be 0 or 1. Argument 3 must be in range 0..15. */ - res = vec_shasigma_be (test, 1, 0xff); /* { dg-error "argument 3 must be in the range 0..15" } */ + res = vec_shasigma_be (test, 1, 0xff); /* { dg-error "argument 3 must be in the range \\[0, 15\\]" } */ return res; } diff --git a/gcc/testsuite/gcc.target/powerpc/pr80315-4.c b/gcc/testsuite/gcc.target/powerpc/pr80315-4.c index eb513639939..66a3efce9c5 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr80315-4.c +++ b/gcc/testsuite/gcc.target/powerpc/pr80315-4.c @@ -12,6 +12,6 @@ main () int mask; /* Argument 2 must be 0 or 1. Argument 3 must be in range 0..15. */ - res = vec_shasigma_be (test, 1, 0xff); /* { dg-error "argument 3 must be in the range 0..15" } */ + res = vec_shasigma_be (test, 1, 0xff); /* { dg-error "argument 3 must be in the range \\[0, 15\\]" } */ return res; } diff --git a/gcc/testsuite/gcc.target/powerpc/warn-lvsl-lvsr.c b/gcc/testsuite/gcc.target/powerpc/warn-lvsl-lvsr.c index bf889aaa22b..62b37a9c335 100644 --- a/gcc/testsuite/gcc.target/powerpc/warn-lvsl-lvsr.c +++ b/gcc/testsuite/gcc.target/powerpc/warn-lvsl-lvsr.c @@ -9,6 +9,6 @@ float f[20]; void foo () { - vector unsigned char a = vec_lvsl (4, f); /* { dg-warning "vec_lvsl is deprecated for little endian; use assignment for unaligned loads and stores" } */ - vector unsigned char b = vec_lvsr (8, f); /* { dg-warning "vec_lvsr is deprecated for little endian; use assignment for unaligned loads and stores" } */ + vector unsigned char a = vec_lvsl (4, f); /* { dg-warning "'vec_lvsl' is deprecated for little endian; use assignment for unaligned loads and stores" } */ + vector unsigned char b = vec_lvsr (8, f); /* { dg-warning "'vec_lvsr' is deprecated for little endian; use assignment for unaligned loads and stores" } */ }