From: Jason Merrill <jason@redhat.com>
To: Marek Polacek <polacek@redhat.com>,
GCC Patches <gcc-patches@gcc.gnu.org>
Cc: Joseph Myers <joseph@codesourcery.com>,
Richard Biener <rguenther@suse.de>
Subject: Re: [PATCH] c-family: Honor -Wno-init-self for cv-qual vars [PR102633]
Date: Tue, 26 Jul 2022 16:24:18 -0400 [thread overview]
Message-ID: <51485fa8-a1c0-c019-c2bb-f4599fe34ca4@redhat.com> (raw)
In-Reply-To: <20220726190340.432777-1-polacek@redhat.com>
On 7/26/22 15:03, Marek Polacek wrote:
> Since r11-5188-g32934a4f45a721, we drop qualifiers during l-to-r
> conversion by creating a NOP_EXPR. For e.g.
>
> const int i = i;
>
> that means that the DECL_INITIAL is '(int) i' and not 'i' anymore.
> Consequently, we don't suppress_warning here:
>
> 711 case DECL_EXPR:
> 715 if (VAR_P (DECL_EXPR_DECL (*expr_p))
> 716 && !DECL_EXTERNAL (DECL_EXPR_DECL (*expr_p))
> 717 && !TREE_STATIC (DECL_EXPR_DECL (*expr_p))
> 718 && (DECL_INITIAL (DECL_EXPR_DECL (*expr_p)) == DECL_EXPR_DECL (*expr_p))
> 719 && !warn_init_self)
> 720 suppress_warning (DECL_EXPR_DECL (*expr_p), OPT_Winit_self);
>
> because of the check on line 718 -- (int) i is not i. So -Wno-init-self
> doesn't disable the warning as it's supposed to.
>
> The following patch fixes it...except it doesn't, for volatile variables
> in C++. The problem is that for
>
> volatile int k = k;
>
> we see that the initializer has TREE_SIDE_EFFECTS, so we perform dynamic
> initialization. So there's no DECL_INITIAL and the suppress_warning
> call above is never done. I suppose we could amend get_no_uninit_warning
> to return true for volatile-qualified expressions. I mean, can we ever
> say for a fact that a volatile variable is uninitialized?
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> PR middle-end/102633
>
> gcc/c-family/ChangeLog:
>
> * c-gimplify.cc (c_gimplify_expr): Strip NOPs of DECL_INITIAL.
I wonder if we want to handle this i = i case earlier, like in finish_decl.
> gcc/testsuite/ChangeLog:
>
> * c-c++-common/Winit-self1.c: New test.
> * c-c++-common/Winit-self2.c: New test.
> ---
> gcc/c-family/c-gimplify.cc | 18 +++++++------
> gcc/testsuite/c-c++-common/Winit-self1.c | 32 ++++++++++++++++++++++++
> gcc/testsuite/c-c++-common/Winit-self2.c | 31 +++++++++++++++++++++++
> 3 files changed, 74 insertions(+), 7 deletions(-)
> create mode 100644 gcc/testsuite/c-c++-common/Winit-self1.c
> create mode 100644 gcc/testsuite/c-c++-common/Winit-self2.c
>
> diff --git a/gcc/c-family/c-gimplify.cc b/gcc/c-family/c-gimplify.cc
> index a6f26c9b0d3..2e011830846 100644
> --- a/gcc/c-family/c-gimplify.cc
> +++ b/gcc/c-family/c-gimplify.cc
> @@ -712,13 +712,17 @@ c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED,
> /* This is handled mostly by gimplify.cc, but we have to deal with
> not warning about int x = x; as it is a GCC extension to turn off
> this warning but only if warn_init_self is zero. */
> - if (VAR_P (DECL_EXPR_DECL (*expr_p))
> - && !DECL_EXTERNAL (DECL_EXPR_DECL (*expr_p))
> - && !TREE_STATIC (DECL_EXPR_DECL (*expr_p))
> - && (DECL_INITIAL (DECL_EXPR_DECL (*expr_p)) == DECL_EXPR_DECL (*expr_p))
> - && !warn_init_self)
> - suppress_warning (DECL_EXPR_DECL (*expr_p), OPT_Winit_self);
> - break;
> + {
> + tree &decl = DECL_EXPR_DECL (*expr_p);
> + if (VAR_P (decl)
> + && !DECL_EXTERNAL (decl)
> + && !TREE_STATIC (decl)
> + && (DECL_INITIAL (decl)
> + && tree_strip_nop_conversions (DECL_INITIAL (decl)) == decl)
> + && !warn_init_self)
> + suppress_warning (decl, OPT_Winit_self);
> + break;
> + }
>
> case PREINCREMENT_EXPR:
> case PREDECREMENT_EXPR:
> diff --git a/gcc/testsuite/c-c++-common/Winit-self1.c b/gcc/testsuite/c-c++-common/Winit-self1.c
> new file mode 100644
> index 00000000000..2a1a755fc71
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/Winit-self1.c
> @@ -0,0 +1,32 @@
> +/* PR middle-end/102633 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wuninitialized -Wno-init-self" } */
> +
> +int
> +fn1 (void)
> +{
> + int i = i;
> + return i;
> +}
> +
> +int
> +fn2 ()
> +{
> + const int j = j;
> + return j;
> +}
> +
> +int
> +fn3 ()
> +{
> + /* ??? Do we want this warning in C++? Probably not with -Wno-init-self. */
> + volatile int k = k; /* { dg-warning "used uninitialized" "" { target c++ } } */
> + return k;
> +}
> +
> +int
> +fn4 ()
> +{
> + const volatile int l = l; /* { dg-warning "used uninitialized" "" { target c++ } } */
> + return l;
> +}
> diff --git a/gcc/testsuite/c-c++-common/Winit-self2.c b/gcc/testsuite/c-c++-common/Winit-self2.c
> new file mode 100644
> index 00000000000..13aa9efdf26
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/Winit-self2.c
> @@ -0,0 +1,31 @@
> +/* PR middle-end/102633 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wuninitialized -Winit-self" } */
> +
> +int
> +fn1 (void)
> +{
> + int i = i; /* { dg-warning "used uninitialized" } */
> + return i;
> +}
> +
> +int
> +fn2 ()
> +{
> + const int j = j; /* { dg-warning "used uninitialized" } */
> + return j;
> +}
> +
> +int
> +fn3 ()
> +{
> + volatile int k = k; /* { dg-warning "used uninitialized" } */
> + return k;
> +}
> +
> +int
> +fn4 ()
> +{
> + const volatile int l = l; /* { dg-warning "used uninitialized" } */
> + return l;
> +}
>
> base-commit: 600956c81c784f4a0cc9d10f6e03e01847afd961
next prev parent reply other threads:[~2022-07-26 20:24 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-26 19:03 Marek Polacek
2022-07-26 20:24 ` Jason Merrill [this message]
2022-07-26 21:31 ` Marek Polacek
2022-08-06 22:29 ` Jason Merrill
2022-08-08 19:06 ` [PATCH v2] " Marek Polacek
2022-08-11 2:05 ` Jason Merrill
2022-08-11 22:30 ` Jason Merrill
2022-07-27 6:41 ` [PATCH] " Richard Biener
2022-07-27 11:37 ` Marek Polacek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51485fa8-a1c0-c019-c2bb-f4599fe34ca4@redhat.com \
--to=jason@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=joseph@codesourcery.com \
--cc=polacek@redhat.com \
--cc=rguenther@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).