Message ID | 47637d16-2b7d-7813-b83a-4bc590221b7f@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | s390: Fix bootstrap -Wformat-diag errors | expand |
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.
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
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
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
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
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
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; } }