diff mbox series

s390: Fix bootstrap -Wformat-diag errors

Message ID 47637d16-2b7d-7813-b83a-4bc590221b7f@linux.ibm.com
State New
Headers
Series s390: Fix bootstrap -Wformat-diag errors |

Commit Message

Robin Dapp Feb. 2, 2022, 4:35 p.m. UTC 
  Hi,

this fixes the s390 bootstrap errors caused by -Werror=format-diag.  It
simply splits the problematic format strings.

Bootstrapped and regtested with -march=z15.

Is it OK?

Regards
 Robin

--

gcc/ChangeLog:

        * config/s390/s390.cc (s390_valid_target_attribute_inner_p): Split
        format strings.

Comments

Martin Sebor Feb. 2, 2022, 7:07 p.m. UTC | #1 
On 2/2/22 09:35, Robin Dapp via Gcc-patches wrote:
> Hi,
> 
> this fixes the s390 bootstrap errors caused by -Werror=format-diag.  It
> simply splits the problematic format strings.

Either this:

   error ("%<attribute(target(\"%s\"))%> is unknown", orig_p);

or this would be better:

   error ("attribute %<target(\"%s\")%> is unknown", orig_p);

The %< %> directives will render it in single quotes like keywords and
identifiers.  Using %qs would render it in double quotes like a string,
which wouldn't be quite right.

Martin

> 
> Bootstrapped and regtested with -march=z15.
> 
> Is it OK?
> 
> Regards
>   Robin
> 
> --
> 
> gcc/ChangeLog:
> 
>          * config/s390/s390.cc (s390_valid_target_attribute_inner_p): Split
>          format strings.
Robin Dapp Feb. 3, 2022, 8:24 a.m. UTC | #2 
Hi Martin,

> Either this:
> 
>    error ("%<attribute(target(\"%s\"))%> is unknown", orig_p);
> 
> or this would be better:
> 
>    error ("attribute %<target(\"%s\")%> is unknown", orig_p);
> 
> The %< %> directives will render it in single quotes like keywords and
> identifiers.  Using %qs would render it in double quotes like a string,
> which wouldn't be quite right.

the x86 backend will print the error message in a different format using
single quotes e.g.

  bla.cc:2:5: error: attribute ‘bleh’ argument ‘target’ is unknown

I don't find that better to be frank but maybe it would make sense for
us to also adopt this error message style and not use a different one?
They use %qs, though - or is there an intention to also change x86's
error messages?  I guess in general it's better to have similar error
messages across targets for the same problem.

Regards
 Robin
Martin Liška Feb. 3, 2022, 8:59 a.m. UTC | #3 
On 2/3/22 09:24, Robin Dapp via Gcc-patches wrote:
> Hi Martin,
> 
>> Either this:
>>
>>     error ("%<attribute(target(\"%s\"))%> is unknown", orig_p);
>>
>> or this would be better:
>>
>>     error ("attribute %<target(\"%s\")%> is unknown", orig_p);
>>
>> The %< %> directives will render it in single quotes like keywords and
>> identifiers.  Using %qs would render it in double quotes like a string,
>> which wouldn't be quite right.
> 
> the x86 backend will print the error message in a different format using
> single quotes e.g.
> 
>    bla.cc:2:5: error: attribute ‘bleh’ argument ‘target’ is unknown
> 
> I don't find that better to be frank but maybe it would make sense for
> us to also adopt this error message style and not use a different one?
> They use %qs, though - or is there an intention to also change x86's
> error messages?  I guess in general it's better to have similar error
> messages across targets for the same problem.
> 
> Regards
>   Robin

Hello.

I've just coincidentally fixed the issue with r12-7013-g0415470c8d6620
where I use the same error format as i386 does.

Martin
Martin Sebor Feb. 3, 2022, 4:43 p.m. UTC | #4 
On 2/3/22 01:59, Martin Liška wrote:
> On 2/3/22 09:24, Robin Dapp via Gcc-patches wrote:
>> Hi Martin,
>>
>>> Either this:
>>>
>>>     error ("%<attribute(target(\"%s\"))%> is unknown", orig_p);
>>>
>>> or this would be better:
>>>
>>>     error ("attribute %<target(\"%s\")%> is unknown", orig_p);
>>>
>>> The %< %> directives will render it in single quotes like keywords and
>>> identifiers.  Using %qs would render it in double quotes like a string,
>>> which wouldn't be quite right.
>>
>> the x86 backend will print the error message in a different format using
>> single quotes e.g.
>>
>>    bla.cc:2:5: error: attribute ‘bleh’ argument ‘target’ is unknown

That's a bug in the i386 back end:
   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90524

>> I don't find that better to be frank but maybe it would make sense for
>> us to also adopt this error message style and not use a different one?
>> They use %qs, though - or is there an intention to also change x86's
>> error messages?  I guess in general it's better to have similar error
>> messages across targets for the same problem.
>>
>> Regards
>>   Robin
> 
> Hello.
> 
> I've just coincidentally fixed the issue with r12-7013-g0415470c8d6620
> where I use the same error format as i386 does.

The change repeats the i386 mistake in the s360 back end.  If
the error is supposed to point out that orig_p is not a valid
argument to attribute target as I believe is the case then
the new call should be:

   error ("attribute %<target%> argument %qs is unknown", orig_p);

Martin
Martin Liška Feb. 3, 2022, 4:45 p.m. UTC | #5 
On 2/3/22 17:43, Martin Sebor wrote:
> On 2/3/22 01:59, Martin Liška wrote:
>> On 2/3/22 09:24, Robin Dapp via Gcc-patches wrote:
>>> Hi Martin,
>>>
>>>> Either this:
>>>>
>>>>     error ("%<attribute(target(\"%s\"))%> is unknown", orig_p);
>>>>
>>>> or this would be better:
>>>>
>>>>     error ("attribute %<target(\"%s\")%> is unknown", orig_p);
>>>>
>>>> The %< %> directives will render it in single quotes like keywords and
>>>> identifiers.  Using %qs would render it in double quotes like a string,
>>>> which wouldn't be quite right.
>>>
>>> the x86 backend will print the error message in a different format using
>>> single quotes e.g.
>>>
>>>    bla.cc:2:5: error: attribute ‘bleh’ argument ‘target’ is unknown
> 
> That's a bug in the i386 back end:
>    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90524
> 
>>> I don't find that better to be frank but maybe it would make sense for
>>> us to also adopt this error message style and not use a different one?
>>> They use %qs, though - or is there an intention to also change x86's
>>> error messages?  I guess in general it's better to have similar error
>>> messages across targets for the same problem.
>>>
>>> Regards
>>>   Robin
>>
>> Hello.
>>
>> I've just coincidentally fixed the issue with r12-7013-g0415470c8d6620
>> where I use the same error format as i386 does.
> 
> The change repeats the i386 mistake in the s360 back end.  If
> the error is supposed to point out that orig_p is not a valid
> argument to attribute target as I believe is the case then
> the new call should be:
> 
>    error ("attribute %<target%> argument %qs is unknown", orig_p);
> 
> Martin

Yeah, fixed with g:9db03cd0caf6bbde1de302bf3509dc26ca8bff2b.

Cheers,
Martin
Martin Sebor Feb. 3, 2022, 5:55 p.m. UTC | #6 
On 2/3/22 09:45, Martin Liška wrote:
> On 2/3/22 17:43, Martin Sebor wrote:
>> On 2/3/22 01:59, Martin Liška wrote:
>>> On 2/3/22 09:24, Robin Dapp via Gcc-patches wrote:
>>>> Hi Martin,
>>>>
>>>>> Either this:
>>>>>
>>>>>     error ("%<attribute(target(\"%s\"))%> is unknown", orig_p);
>>>>>
>>>>> or this would be better:
>>>>>
>>>>>     error ("attribute %<target(\"%s\")%> is unknown", orig_p);
>>>>>
>>>>> The %< %> directives will render it in single quotes like keywords and
>>>>> identifiers.  Using %qs would render it in double quotes like a 
>>>>> string,
>>>>> which wouldn't be quite right.
>>>>
>>>> the x86 backend will print the error message in a different format 
>>>> using
>>>> single quotes e.g.
>>>>
>>>>    bla.cc:2:5: error: attribute ‘bleh’ argument ‘target’ is unknown
>>
>> That's a bug in the i386 back end:
>>    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90524
>>
>>>> I don't find that better to be frank but maybe it would make sense for
>>>> us to also adopt this error message style and not use a different one?
>>>> They use %qs, though - or is there an intention to also change x86's
>>>> error messages?  I guess in general it's better to have similar error
>>>> messages across targets for the same problem.
>>>>
>>>> Regards
>>>>   Robin
>>>
>>> Hello.
>>>
>>> I've just coincidentally fixed the issue with r12-7013-g0415470c8d6620
>>> where I use the same error format as i386 does.
>>
>> The change repeats the i386 mistake in the s360 back end.  If
>> the error is supposed to point out that orig_p is not a valid
>> argument to attribute target as I believe is the case then
>> the new call should be:
>>
>>    error ("attribute %<target%> argument %qs is unknown", orig_p);
>>
>> Martin
> 
> Yeah, fixed with g:9db03cd0caf6bbde1de302bf3509dc26ca8bff2b.

Great, thank you!

Martin

> 
> Cheers,
> Martin
diff mbox series

Patch

commit 41270791b1d1235d580b6d81c315c74ad07c1807
Author: Robin Dapp <rdapp@linux.ibm.com>
Date:   Tue Feb 1 10:13:52 2022 +0100

    s390: Fix bootstrap by splitting format string.
    
    Recently -Werror=format-diag was turned on for bootstrap which broke
    s390 bootstrap.
    
    This patch splits the problematic format strings and changes quoting
    from explicit \" to %qs.
    
    Bootstrapped and regtested on s390x.

diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc
index 58064bfb525..d2faa1371eb 100644
--- a/gcc/config/s390/s390.cc
+++ b/gcc/config/s390/s390.cc
@@ -15879,6 +15879,9 @@  s390_valid_target_attribute_inner_p (tree args,
   /* Handle multiple arguments separated by commas.  */
   next_optstr = ASTRDUP (TREE_STRING_POINTER (args));
 
+  const char err_prefix[] = "attribute(target(";
+  const char err_suffix[] = "))";
+
   while (next_optstr && *next_optstr != '\0')
     {
       char *p = next_optstr;
@@ -15938,7 +15941,7 @@  s390_valid_target_attribute_inner_p (tree args,
       /* Process the option.  */
       if (!found)
 	{
-	  error ("attribute(target(\"%s\")) is unknown", orig_p);
+	  error ("%s%qs%s is unknown", err_prefix, orig_p, err_suffix);
 	  return false;
 	}
       else if (attrs[i].only_as_pragma && !force_pragma)
@@ -15988,7 +15991,7 @@  s390_valid_target_attribute_inner_p (tree args,
 	    }
 	  else
 	    {
-	      error ("attribute(target(\"%s\")) is unknown", orig_p);
+	      error ("%s%qs%s is unknown", err_prefix, orig_p, err_suffix);
 	      ret = false;
 	    }
 	}
@@ -16005,7 +16008,7 @@  s390_valid_target_attribute_inner_p (tree args,
 			global_dc);
 	  else
 	    {
-	      error ("attribute(target(\"%s\")) is unknown", orig_p);
+	      error ("%s%qs%s is unknown", err_prefix, orig_p, err_suffix);
 	      ret = false;
 	    }
 	}