* [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).