Message ID | 9532d734-eac1-eecb-6dbe-cd0a7fc55b36@suse.cz |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B78823857817 for <patchwork@sourceware.org>; Thu, 20 Jan 2022 09:44:50 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 0FF063857809 for <gcc-patches@gcc.gnu.org>; Thu, 20 Jan 2022 09:43:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0FF063857809 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.cz Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id DD0B21F394 for <gcc-patches@gcc.gnu.org>; Thu, 20 Jan 2022 09:43:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1642671835; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=3Yoh9ml5JPzKmJQqqsMttG5KMjzmjZc4u9BJsTISAv8=; b=WKW3YPhXvFfuGxWyVPb4Ur9WKf6A1IZS4nqEg041DcTepDe7xbw4dbjcwWbyXnJUJ0ibCl Slkxa3QBO3QOex/eMNypH4hLuxPlU0PyGgQQoNpwmZ67oAaRgI8NTgbJa+35QO8fQ5yATU D2aS3HzKoHWo+qamSuRI+4ZJdIDOgrg= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1642671835; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=3Yoh9ml5JPzKmJQqqsMttG5KMjzmjZc4u9BJsTISAv8=; b=UatqfoMy173+cNNThLXW/ybYZuqF4d/NQi2Pmz6YIsR/rteKKoaqr6URF9n2e78YllG5jD vjD+rnYk46gElnCA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id CDAD113D27 for <gcc-patches@gcc.gnu.org>; Thu, 20 Jan 2022 09:43:55 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id WcI6Mdsu6WEOEQAAMHmgww (envelope-from <mliska@suse.cz>) for <gcc-patches@gcc.gnu.org>; Thu, 20 Jan 2022 09:43:55 +0000 Message-ID: <9532d734-eac1-eecb-6dbe-cd0a7fc55b36@suse.cz> Date: Thu, 20 Jan 2022 10:43:55 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 From: =?utf-8?q?Martin_Li=C5=A1ka?= <mliska@suse.cz> Subject: [PATCH] Fix Werror=format-diag with --disable-nls. To: gcc-patches@gcc.gnu.org Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
Fix Werror=format-diag with --disable-nls.
|
|
Commit Message
Martin Liška
Jan. 20, 2022, 9:43 a.m. UTC
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(+)
Comments
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
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
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 >
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
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 >
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
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
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
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: