This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 3/7] Remove trailing period from various diagnostic messages (PR translation/79923)
- From: Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>
- To: Jakub Jelinek <jakub at redhat dot com>, David Malcolm <dmalcolm at redhat dot com>, Richard Earnshaw <richard dot earnshaw at arm dot com>, Ramana Radhakrishnan <ramana dot radhakrishnan at arm dot com>
- Cc: gcc-patches at gcc dot gnu dot org, roland dot illig at gmx dot de, Joseph Myers <joseph at codesourcery dot com>
- Date: Fri, 10 Mar 2017 09:24:18 +0000
- Subject: Re: [PATCH 3/7] Remove trailing period from various diagnostic messages (PR translation/79923)
- Authentication-results: sourceware.org; auth=none
- References: <1489081529-22256-1-git-send-email-dmalcolm@redhat.com> <1489081529-22256-4-git-send-email-dmalcolm@redhat.com> <20170310062415.GQ22703@tucnak>
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