* [PATCH] Fix Werror=format-diag with --disable-nls. @ 2022-01-20 9:43 Martin Liška 2022-01-20 10:17 ` Jakub Jelinek 0 siblings, 1 reply; 9+ messages in thread From: Martin Liška @ 2022-01-20 9:43 UTC (permalink / raw) To: gcc-patches The patch disables "-Wformat-diag" for dump_aggr_type. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin PR c++/104134 gcc/cp/ChangeLog: * error.cc (dump_aggr_type): Partially disable the warning. --- gcc/cp/error.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc index 1ab0c25a477..c031c75cc5e 100644 --- a/gcc/cp/error.cc +++ b/gcc/cp/error.cc @@ -768,6 +768,11 @@ class_key_or_enum_as_string (tree t) return "struct"; } +#if __GNUC__ >= 10 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat-diag" +#endif + /* Print out a class declaration T under the control of FLAGS, in the form `class foo'. */ @@ -851,6 +856,10 @@ dump_aggr_type (cxx_pretty_printer *pp, tree t, int flags) flags & ~TFF_TEMPLATE_HEADER); } +#if __GNUC__ >= 10 +#pragma GCC diagnostic pop +#endif + /* Dump into the obstack the initial part of the output for a given type. This is necessary when dealing with things like functions returning functions. Examples: -- 2.34.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix Werror=format-diag with --disable-nls. 2022-01-20 9:43 [PATCH] Fix Werror=format-diag with --disable-nls Martin Liška @ 2022-01-20 10:17 ` Jakub Jelinek 2022-01-20 10:28 ` Jakub Jelinek 0 siblings, 1 reply; 9+ messages in thread From: Jakub Jelinek @ 2022-01-20 10:17 UTC (permalink / raw) To: Martin Liška; +Cc: gcc-patches On Thu, Jan 20, 2022 at 10:43:55AM +0100, Martin Liška wrote: > The patch disables "-Wformat-diag" for dump_aggr_type. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Thanks, > Martin > > PR c++/104134 > > gcc/cp/ChangeLog: > > * error.cc (dump_aggr_type): Partially disable the warning. > --- > gcc/cp/error.cc | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc > index 1ab0c25a477..c031c75cc5e 100644 > --- a/gcc/cp/error.cc > +++ b/gcc/cp/error.cc > @@ -768,6 +768,11 @@ class_key_or_enum_as_string (tree t) > return "struct"; > } > +#if __GNUC__ >= 10 > +#pragma GCC diagnostic push > +#pragma GCC diagnostic ignored "-Wformat-diag" > +#endif > + > /* Print out a class declaration T under the control of FLAGS, > in the form `class foo'. */ > @@ -851,6 +856,10 @@ dump_aggr_type (cxx_pretty_printer *pp, tree t, int flags) > flags & ~TFF_TEMPLATE_HEADER); > } > +#if __GNUC__ >= 10 > +#pragma GCC diagnostic pop > +#endif > + Please add an empty line above #if lines. Also, it would be nice to use the same style of these at least in the same file. The others are: /* Disable warnings about missing quoting in GCC diagnostics for the pp_verbatim calls. Their format strings deliberately don't follow GCC diagnostic conventions. */ #if __GNUC__ >= 10 # pragma GCC diagnostic push # pragma GCC diagnostic ignored "-Wformat-diag" #endif and #if __GNUC__ >= 10 # pragma GCC diagnostic pop #endif The 2 spaces between # and pragma look just weird, so either use in all the 4 spaces 1 space between # and pragma, or 0 spaces. And also the copy the comment from above the other diagnostic push, perhaps with small tweak (pp_verbatim -> pp_printf)? Otherwise LGTM. Jakub ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix Werror=format-diag with --disable-nls. 2022-01-20 10:17 ` Jakub Jelinek @ 2022-01-20 10:28 ` Jakub Jelinek 2022-01-20 16:33 ` Martin Sebor 0 siblings, 1 reply; 9+ messages in thread From: Jakub Jelinek @ 2022-01-20 10:28 UTC (permalink / raw) To: Martin Liška, gcc-patches, Martin Sebor On Thu, Jan 20, 2022 at 11:17:28AM +0100, Jakub Jelinek via Gcc-patches wrote: > > --- a/gcc/cp/error.cc > > +++ b/gcc/cp/error.cc > > @@ -768,6 +768,11 @@ class_key_or_enum_as_string (tree t) > > return "struct"; > > } > > +#if __GNUC__ >= 10 > > +#pragma GCC diagnostic push > > +#pragma GCC diagnostic ignored "-Wformat-diag" > > +#endif > > + > > /* Print out a class declaration T under the control of FLAGS, > > in the form `class foo'. */ > > @@ -851,6 +856,10 @@ dump_aggr_type (cxx_pretty_printer *pp, tree t, int flags) > > flags & ~TFF_TEMPLATE_HEADER); > > } > > +#if __GNUC__ >= 10 > > +#pragma GCC diagnostic pop > > +#endif Oh, and one more thing, but this time not about this source file but about the warning. Does it handle the gettext case? I think -Wformat generally does, gettext has format_arg attribute. If the warning handles pp_printf ("<unnamed %s>", str); and pp_printf (cond ? "<unnamed %s>" : "<unnamed %s>", str); and pp_printf (cond ? "<unnamed %s>" : "something %s", str); and pp_printf (gettext ("<unnamed %s>"), str); then maybe it should also handle pp_printf (cond ? gettext ("<unnamed %s>") : "<unnamed %s>, str); and pp_printf (cond ? gettext ("<unnamed %s>") : "something %s, str); too? Jakub ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix Werror=format-diag with --disable-nls. 2022-01-20 10:28 ` Jakub Jelinek @ 2022-01-20 16:33 ` Martin Sebor 2022-01-20 16:43 ` Jakub Jelinek 0 siblings, 1 reply; 9+ messages in thread From: Martin Sebor @ 2022-01-20 16:33 UTC (permalink / raw) To: Jakub Jelinek, Martin Liška, gcc-patches On 1/20/22 03:28, Jakub Jelinek wrote: > On Thu, Jan 20, 2022 at 11:17:28AM +0100, Jakub Jelinek via Gcc-patches wrote: >>> --- a/gcc/cp/error.cc >>> +++ b/gcc/cp/error.cc >>> @@ -768,6 +768,11 @@ class_key_or_enum_as_string (tree t) >>> return "struct"; >>> } >>> +#if __GNUC__ >= 10 >>> +#pragma GCC diagnostic push >>> +#pragma GCC diagnostic ignored "-Wformat-diag" >>> +#endif >>> + >>> /* Print out a class declaration T under the control of FLAGS, >>> in the form `class foo'. */ >>> @@ -851,6 +856,10 @@ dump_aggr_type (cxx_pretty_printer *pp, tree t, int flags) >>> flags & ~TFF_TEMPLATE_HEADER); >>> } >>> +#if __GNUC__ >= 10 >>> +#pragma GCC diagnostic pop >>> +#endif > > Oh, and one more thing, but this time not about this source file but about > the warning. Does it handle the gettext case? > I think -Wformat generally does, gettext has format_arg attribute. > If the warning handles > pp_printf ("<unnamed %s>", str); > and > pp_printf (cond ? "<unnamed %s>" : "<unnamed %s>", str); > and > pp_printf (cond ? "<unnamed %s>" : "something %s", str); > and > pp_printf (gettext ("<unnamed %s>"), str); > then maybe it should also handle > pp_printf (cond ? gettext ("<unnamed %s>") : "<unnamed %s>, str); > and > pp_printf (cond ? gettext ("<unnamed %s>") : "something %s, str); > too? -Wformat-diag is part of -Wformat so they both should handle the same things. Do you see a difference between what they handle? Martin > > Jakub > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix Werror=format-diag with --disable-nls. 2022-01-20 16:33 ` Martin Sebor @ 2022-01-20 16:43 ` Jakub Jelinek 2022-01-20 16:56 ` Martin Sebor 0 siblings, 1 reply; 9+ messages in thread From: Jakub Jelinek @ 2022-01-20 16:43 UTC (permalink / raw) To: Martin Sebor; +Cc: Martin Liška, gcc-patches On Thu, Jan 20, 2022 at 09:33:30AM -0700, Martin Sebor wrote: > > Oh, and one more thing, but this time not about this source file but about > > the warning. Does it handle the gettext case? > > I think -Wformat generally does, gettext has format_arg attribute. > > If the warning handles > > pp_printf ("<unnamed %s>", str); > > and > > pp_printf (cond ? "<unnamed %s>" : "<unnamed %s>", str); > > and > > pp_printf (cond ? "<unnamed %s>" : "something %s", str); > > and > > pp_printf (gettext ("<unnamed %s>"), str); > > then maybe it should also handle > > pp_printf (cond ? gettext ("<unnamed %s>") : "<unnamed %s>, str); > > and > > pp_printf (cond ? gettext ("<unnamed %s>") : "something %s, str); > > too? > > -Wformat-diag is part of -Wformat so they both should handle the same > things. Do you see a difference between what they handle? With normal -Wformat I see all expected warnings in: char *foo (const char *) __attribute__((format_arg(1))); void bar (const char *, ...) __attribute__((format(printf, 1, 2))); void baz (int x) { bar ("%ld", x); bar (x ? "%ld" : "%ld", x); bar (x ? "%ld" : "%lld", x); bar (foo ("%ld"), x); bar (x ? foo ("%ld") : "%ld", x); bar (x ? foo ("%ld") : "%lld", x); bar (foo (x ? "%ld" : "%ld"), x); bar (foo (x ? "%ld" : "%lld"), x); } (on all bar calls, on those with different strings or one in foo and other not 2). From the fact that -Wformat-diag didn't warn on the pp_printf (cond ? gettext ("<unnamed %s>") : "<unnamed %s>, str); case I assume -Wformat-diag doesn't handle this. Jakub ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix Werror=format-diag with --disable-nls. 2022-01-20 16:43 ` Jakub Jelinek @ 2022-01-20 16:56 ` Martin Sebor 2022-01-20 17:03 ` Jakub Jelinek 0 siblings, 1 reply; 9+ messages in thread From: Martin Sebor @ 2022-01-20 16:56 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Martin Liška, gcc-patches On 1/20/22 09:43, Jakub Jelinek wrote: > On Thu, Jan 20, 2022 at 09:33:30AM -0700, Martin Sebor wrote: >>> Oh, and one more thing, but this time not about this source file but about >>> the warning. Does it handle the gettext case? >>> I think -Wformat generally does, gettext has format_arg attribute. >>> If the warning handles >>> pp_printf ("<unnamed %s>", str); >>> and >>> pp_printf (cond ? "<unnamed %s>" : "<unnamed %s>", str); >>> and >>> pp_printf (cond ? "<unnamed %s>" : "something %s", str); >>> and >>> pp_printf (gettext ("<unnamed %s>"), str); >>> then maybe it should also handle >>> pp_printf (cond ? gettext ("<unnamed %s>") : "<unnamed %s>, str); >>> and >>> pp_printf (cond ? gettext ("<unnamed %s>") : "something %s, str); >>> too? >> >> -Wformat-diag is part of -Wformat so they both should handle the same >> things. Do you see a difference between what they handle? > > With normal -Wformat I see all expected warnings in: > char *foo (const char *) __attribute__((format_arg(1))); > void bar (const char *, ...) __attribute__((format(printf, 1, 2))); -Wformat-diag is internal to GCC and needs one of the GCC-internal attributes to enable, like __gcc_cxxdiag__, for example like this: __attribute__ ((format (__gcc_cxxdiag__, 1, 2))) void bar (const char *, ...); With that it triggers in all the same instances as -Wformat below (as near I can tell for a modified test case). Martin > > void > baz (int x) > { > bar ("%ld", x); > bar (x ? "%ld" : "%ld", x); > bar (x ? "%ld" : "%lld", x); > bar (foo ("%ld"), x); > bar (x ? foo ("%ld") : "%ld", x); > bar (x ? foo ("%ld") : "%lld", x); > bar (foo (x ? "%ld" : "%ld"), x); > bar (foo (x ? "%ld" : "%lld"), x); > } > (on all bar calls, on those with different strings or one in foo and other > not 2). > From the fact that -Wformat-diag didn't warn on the > pp_printf (cond ? gettext ("<unnamed %s>") : "<unnamed %s>, str); > case I assume -Wformat-diag doesn't handle this. > > Jakub > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix Werror=format-diag with --disable-nls. 2022-01-20 16:56 ` Martin Sebor @ 2022-01-20 17:03 ` Jakub Jelinek 2022-01-20 17:52 ` Martin Sebor 0 siblings, 1 reply; 9+ messages in thread From: Jakub Jelinek @ 2022-01-20 17:03 UTC (permalink / raw) To: Martin Sebor; +Cc: Martin Liška, gcc-patches On Thu, Jan 20, 2022 at 09:56:59AM -0700, Martin Sebor wrote: > > With normal -Wformat I see all expected warnings in: > > char *foo (const char *) __attribute__((format_arg(1))); > > void bar (const char *, ...) __attribute__((format(printf, 1, 2))); > > -Wformat-diag is internal to GCC and needs one of the GCC-internal > attributes to enable, like __gcc_cxxdiag__, for example like this: > > __attribute__ ((format (__gcc_cxxdiag__, 1, 2))) > void bar (const char *, ...); > > With that it triggers in all the same instances as -Wformat below > (as near I can tell for a modified test case). Glad to hear that, but then I don't understand why we didn't warn on cp/error.cc before Martin L.'s change when --disable-nls wasn't used. Jakub ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix Werror=format-diag with --disable-nls. 2022-01-20 17:03 ` Jakub Jelinek @ 2022-01-20 17:52 ` Martin Sebor 2022-01-20 18:18 ` Jakub Jelinek 0 siblings, 1 reply; 9+ messages in thread From: Martin Sebor @ 2022-01-20 17:52 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Martin Liška, gcc-patches On 1/20/22 10:03, Jakub Jelinek wrote: > On Thu, Jan 20, 2022 at 09:56:59AM -0700, Martin Sebor wrote: >>> With normal -Wformat I see all expected warnings in: >>> char *foo (const char *) __attribute__((format_arg(1))); >>> void bar (const char *, ...) __attribute__((format(printf, 1, 2))); >> >> -Wformat-diag is internal to GCC and needs one of the GCC-internal >> attributes to enable, like __gcc_cxxdiag__, for example like this: >> >> __attribute__ ((format (__gcc_cxxdiag__, 1, 2))) >> void bar (const char *, ...); >> >> With that it triggers in all the same instances as -Wformat below >> (as near I can tell for a modified test case). > > Glad to hear that, but then I don't understand why we didn't warn on > cp/error.cc before Martin L.'s change when --disable-nls wasn't used. Good question! There does seem to be some strange interplay between parentheses and -Wformat for __gcc_cdiag__ functions in the C++ front end: __attribute__ ((format (__gcc_cxxdiag__, 1, 2))) void bar (const char *, ...); void baz (int x) { bar (x ? "<%s" : "%i", x); // -Wformat-diag bar ((x ? "<%s" : "%i"), x); // silence bar ((x ? ("<%s") : ("%i")), x); // silence } The C front end warns on all three calls. With attribute printf both the C and C++ front ends issue a -Wformat for all three calls as expected (passing an int to %s). Martin ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix Werror=format-diag with --disable-nls. 2022-01-20 17:52 ` Martin Sebor @ 2022-01-20 18:18 ` Jakub Jelinek 0 siblings, 0 replies; 9+ messages in thread From: Jakub Jelinek @ 2022-01-20 18:18 UTC (permalink / raw) To: Martin Sebor; +Cc: Martin Liška, gcc-patches On Thu, Jan 20, 2022 at 10:52:10AM -0700, Martin Sebor wrote: > On 1/20/22 10:03, Jakub Jelinek wrote: > > On Thu, Jan 20, 2022 at 09:56:59AM -0700, Martin Sebor wrote: > > > > With normal -Wformat I see all expected warnings in: > > > > char *foo (const char *) __attribute__((format_arg(1))); > > > > void bar (const char *, ...) __attribute__((format(printf, 1, 2))); > > > > > > -Wformat-diag is internal to GCC and needs one of the GCC-internal > > > attributes to enable, like __gcc_cxxdiag__, for example like this: > > > > > > __attribute__ ((format (__gcc_cxxdiag__, 1, 2))) > > > void bar (const char *, ...); > > > > > > With that it triggers in all the same instances as -Wformat below > > > (as near I can tell for a modified test case). > > > > Glad to hear that, but then I don't understand why we didn't warn on > > cp/error.cc before Martin L.'s change when --disable-nls wasn't used. > > Good question! There does seem to be some strange interplay between > parentheses and -Wformat for __gcc_cdiag__ functions in the C++ front > end: > > __attribute__ ((format (__gcc_cxxdiag__, 1, 2))) > void bar (const char *, ...); > > void > baz (int x) > { > bar (x ? "<%s" : "%i", x); // -Wformat-diag > bar ((x ? "<%s" : "%i"), x); // silence > bar ((x ? ("<%s") : ("%i")), x); // silence > } > > The C front end warns on all three calls. > > With attribute printf both the C and C++ front ends issue a -Wformat > for all three calls as expected (passing an int to %s). Filed PR104148 now. Jakub ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-01-20 18:19 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-20 9:43 [PATCH] Fix Werror=format-diag with --disable-nls Martin Liška 2022-01-20 10:17 ` Jakub Jelinek 2022-01-20 10:28 ` Jakub Jelinek 2022-01-20 16:33 ` Martin Sebor 2022-01-20 16:43 ` Jakub Jelinek 2022-01-20 16:56 ` Martin Sebor 2022-01-20 17:03 ` Jakub Jelinek 2022-01-20 17:52 ` Martin Sebor 2022-01-20 18:18 ` Jakub Jelinek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).