This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, rs6000] More diagnostic cleanup, addressing PR79845
- From: Bill Schmidt <wschmidt at linux dot vnet dot ibm dot com>
- To: Segher Boessenkool <segher at kernel dot crashing dot org>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, David Edelsohn <dje dot gcc at gmail dot com>
- Date: Fri, 11 Aug 2017 14:21:17 -0500
- Subject: Re: [PATCH, rs6000] More diagnostic cleanup, addressing PR79845
- Authentication-results: sourceware.org; auth=none
- References: <e6da7858-91d3-8b4b-8686-b6591dbd902f@linux.vnet.ibm.com> <20170811191430.GD13471@gate.crashing.org>
Hi Segher,
Thanks for the review!
> On Aug 11, 2017, at 2:14 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
>
> Hi Bill,
>
> On Thu, Aug 10, 2017 at 04:28:49PM -0500, Bill Schmidt wrote:
>> This continues the cleanup of diagnostic messages in the rs6000 back end. The
>> primary focus is to make sure that we use quoted strings with %qs, %<, %> as
>> appropriate, and that option strings are separated from translatable strings
>> to make things easier on the internationalization folks, as requested in
>> PR79845. While working on this, I noticed a couple of places where the
>> diagnostic strings result in excessively long lines, and cleaned these up as
>> well.
>>
>> One peculiarity I noticed, but did not attempt to address, is that a small
>> handful of diagnostic strings are tagged with _N (). There doesn't seem to
>> be any rhyme or reason to this. I expect it's a result of copy/paste from
>> somewhere and most of these strings should be translated. But that's for
>> another day (and probably another person).
>>
>> Bootstrapped and tested on powerpc64le-linux-gnu (POWER8 64-bit) and on
>> powerpc64-linux-gnu (POWER7 32- and 64-bit) with no regressions. Is this
>> okay for trunk?
>
> Yes, thank you! One comment:
>
>> @@ -6128,17 +6128,18 @@ altivec_resolve_overloaded_builtin (location_t loc
>>
>> if (nargs == 0)
>> {
>> - error ("%s only accepts %d arguments", name, (fcode == ALTIVEC_BUILTIN_VEC_PROMOTE)+1 );
>> + error ("builtin %qs only accepts %d arguments", name,
>> + (fcode == ALTIVEC_BUILTIN_VEC_PROMOTE)+1 );
>> return error_mark_node;
>> }
>> if (fcode == ALTIVEC_BUILTIN_VEC_SPLATS && nargs != 1)
>> {
>> - error ("%s only accepts 1 argument", name);
>> + error ("builtin %qs only accepts 1 argument", name);
>> return error_mark_node;
>> }
>> if (fcode == ALTIVEC_BUILTIN_VEC_PROMOTE && nargs != 2)
>> {
>> - error ("%s only accepts 2 arguments", name);
>> + error ("builtin %qs only accepts 2 arguments", name);
>> return error_mark_node;
>> }
>
> This last "if" is redundant with the handling of VEC_PROMOTE about ten
> lines up. Maybe the "if (nargs == 0)" should be ordered later and things
> simplified?
Looks to me like it's equivalent to remove the whole "if (nargs == 0) {...}" clause.
I'll regstrap that and commit if it's clean. Thanks again!
Bill
>
> Okay with or without making that nicer.
>
>
> Segher
>