* [PATCH] c-family: Honor -Wno-init-self for cv-qual vars [PR102633] @ 2022-07-26 19:03 Marek Polacek 2022-07-26 20:24 ` Jason Merrill 2022-07-27 6:41 ` [PATCH] " Richard Biener 0 siblings, 2 replies; 9+ messages in thread From: Marek Polacek @ 2022-07-26 19:03 UTC (permalink / raw) To: GCC Patches; +Cc: Jason Merrill, Joseph Myers, Richard Biener 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. 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 -- 2.37.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c-family: Honor -Wno-init-self for cv-qual vars [PR102633] 2022-07-26 19:03 [PATCH] c-family: Honor -Wno-init-self for cv-qual vars [PR102633] Marek Polacek @ 2022-07-26 20:24 ` Jason Merrill 2022-07-26 21:31 ` Marek Polacek 2022-07-27 6:41 ` [PATCH] " Richard Biener 1 sibling, 1 reply; 9+ messages in thread From: Jason Merrill @ 2022-07-26 20:24 UTC (permalink / raw) To: Marek Polacek, GCC Patches; +Cc: Joseph Myers, Richard Biener 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c-family: Honor -Wno-init-self for cv-qual vars [PR102633] 2022-07-26 20:24 ` Jason Merrill @ 2022-07-26 21:31 ` Marek Polacek 2022-08-06 22:29 ` Jason Merrill 0 siblings, 1 reply; 9+ messages in thread From: Marek Polacek @ 2022-07-26 21:31 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches, Joseph Myers, Richard Biener On Tue, Jul 26, 2022 at 04:24:18PM -0400, Jason Merrill wrote: > 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. I could, something like @@ -5381,7 +5381,14 @@ finish_decl (tree decl, location_t init_loc, tree init, init = NULL_TREE; if (init) - store_init_value (init_loc, decl, init, origtype); + { + /* In the self-init case, undo the artificial NOP_EXPR we may have + added in convert_lvalue_to_rvalue so that c_gimplify_expr/DECL_EXPR + can perform suppress_warning. */ + if (TREE_CODE (init) == NOP_EXPR && TREE_OPERAND (init, 0) == decl) + init = TREE_OPERAND (init, 0); + store_init_value (init_loc, decl, init, origtype); + } but then I'd have to do the same thing in cp_finish_decl because decay_conversion also adds a NOP_EXPR for cv-qualified non-class prvalues. Is that what we want? To me that seems less clean than having c_gimplify_expr see through NOP_EXPRs. Marek ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c-family: Honor -Wno-init-self for cv-qual vars [PR102633] 2022-07-26 21:31 ` Marek Polacek @ 2022-08-06 22:29 ` Jason Merrill 2022-08-08 19:06 ` [PATCH v2] " Marek Polacek 0 siblings, 1 reply; 9+ messages in thread From: Jason Merrill @ 2022-08-06 22:29 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches, Joseph Myers, Richard Biener On 7/26/22 14:31, Marek Polacek wrote: > On Tue, Jul 26, 2022 at 04:24:18PM -0400, Jason Merrill wrote: >> 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. > > I could, something like > > @@ -5381,7 +5381,14 @@ finish_decl (tree decl, location_t init_loc, tree init, > init = NULL_TREE; > > if (init) > - store_init_value (init_loc, decl, init, origtype); > + { > + /* In the self-init case, undo the artificial NOP_EXPR we may have > + added in convert_lvalue_to_rvalue so that c_gimplify_expr/DECL_EXPR > + can perform suppress_warning. */ > + if (TREE_CODE (init) == NOP_EXPR && TREE_OPERAND (init, 0) == decl) > + init = TREE_OPERAND (init, 0); > + store_init_value (init_loc, decl, init, origtype); > + } > > but then I'd have to do the same thing in cp_finish_decl because > decay_conversion also adds a NOP_EXPR for cv-qualified non-class prvalues. > Is that what we want? To me that seems less clean than having c_gimplify_expr > see through NOP_EXPRs. I was thinking of checking the form of the initializer before decay_conversion or anything else messes with it, and calling suppress_warning at that point instead of in c_gimplify_expr. Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] c-family: Honor -Wno-init-self for cv-qual vars [PR102633] 2022-08-06 22:29 ` Jason Merrill @ 2022-08-08 19:06 ` Marek Polacek 2022-08-11 2:05 ` Jason Merrill 2022-08-11 22:30 ` Jason Merrill 0 siblings, 2 replies; 9+ messages in thread From: Marek Polacek @ 2022-08-08 19:06 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches, Joseph Myers, Richard Biener On Sat, Aug 06, 2022 at 03:29:05PM -0700, Jason Merrill wrote: > On 7/26/22 14:31, Marek Polacek wrote: > > On Tue, Jul 26, 2022 at 04:24:18PM -0400, Jason Merrill wrote: > > > 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. > > > > I could, something like > > > > @@ -5381,7 +5381,14 @@ finish_decl (tree decl, location_t init_loc, tree init, > > init = NULL_TREE; > > > > if (init) > > - store_init_value (init_loc, decl, init, origtype); > > + { > > + /* In the self-init case, undo the artificial NOP_EXPR we may have > > + added in convert_lvalue_to_rvalue so that c_gimplify_expr/DECL_EXPR > > + can perform suppress_warning. */ > > + if (TREE_CODE (init) == NOP_EXPR && TREE_OPERAND (init, 0) == decl) > > + init = TREE_OPERAND (init, 0); > > + store_init_value (init_loc, decl, init, origtype); > > + } > > > > but then I'd have to do the same thing in cp_finish_decl because > > decay_conversion also adds a NOP_EXPR for cv-qualified non-class prvalues. > > Is that what we want? To me that seems less clean than having c_gimplify_expr > > see through NOP_EXPRs. > > I was thinking of checking the form of the initializer before > decay_conversion or anything else messes with it, and calling > suppress_warning at that point instead of in c_gimplify_expr. Aaah, okay. Here's a patch that does it. In the C FE it has to happen really early. Now both front ends behave the same wrt volatiles! Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- 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 by moving the suppress_warning call from c_gimplify_expr to the front ends, at points where we haven't created the NOP_EXPR yet. PR middle-end/102633 gcc/c-family/ChangeLog: * c-gimplify.cc (c_gimplify_expr) <case DECL_EXPR>: Don't call suppress_warning here. gcc/c/ChangeLog: * c-parser.cc (c_parser_initializer): Add new tree parameter. Use it. Call suppress_warning. (c_parser_declaration_or_fndef): Pass d down to c_parser_initializer. (c_parser_omp_declare_reduction): Pass omp_priv down to c_parser_initializer. gcc/cp/ChangeLog: * decl.cc (cp_finish_decl): Call suppress_warning. 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 | 12 --------- gcc/c/c-parser.cc | 19 ++++++++++++--- gcc/cp/decl.cc | 8 ++++++ gcc/testsuite/c-c++-common/Winit-self1.c | 31 ++++++++++++++++++++++++ gcc/testsuite/c-c++-common/Winit-self2.c | 31 ++++++++++++++++++++++++ 5 files changed, 85 insertions(+), 16 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..039a4b93230 100644 --- a/gcc/c-family/c-gimplify.cc +++ b/gcc/c-family/c-gimplify.cc @@ -708,18 +708,6 @@ c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED, break; } - case DECL_EXPR: - /* 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; - case PREINCREMENT_EXPR: case PREDECREMENT_EXPR: case POSTINCREMENT_EXPR: diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index 92049d1a101..6ed17ed9c48 100644 --- a/gcc/c/c-parser.cc +++ b/gcc/c/c-parser.cc @@ -1521,7 +1521,7 @@ static struct c_arg_info *c_parser_parms_list_declarator (c_parser *, tree, static struct c_parm *c_parser_parameter_declaration (c_parser *, tree, bool); static tree c_parser_simple_asm_expr (c_parser *); static tree c_parser_gnu_attributes (c_parser *); -static struct c_expr c_parser_initializer (c_parser *); +static struct c_expr c_parser_initializer (c_parser *, tree); static struct c_expr c_parser_braced_init (c_parser *, tree, bool, struct obstack *); static void c_parser_initelt (c_parser *, struct obstack *); @@ -2286,7 +2286,7 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, int flag_sanitize_save = flag_sanitize; if (TREE_CODE (d) == PARM_DECL) flag_sanitize = 0; - init = c_parser_initializer (parser); + init = c_parser_initializer (parser, d); flag_sanitize = flag_sanitize_save; finish_init (); } @@ -5211,11 +5211,13 @@ c_parser_type_name (c_parser *parser, bool alignas_ok) Any expression without commas is accepted in the syntax for the constant-expressions, with non-constant expressions rejected later. + DECL is the declaration we're parsing this initializer for. + This function is only used for top-level initializers; for nested ones, see c_parser_initval. */ static struct c_expr -c_parser_initializer (c_parser *parser) +c_parser_initializer (c_parser *parser, tree decl) { if (c_parser_next_token_is (parser, CPP_OPEN_BRACE)) return c_parser_braced_init (parser, NULL_TREE, false, NULL); @@ -5224,6 +5226,15 @@ c_parser_initializer (c_parser *parser) struct c_expr ret; location_t loc = c_parser_peek_token (parser)->location; ret = c_parser_expr_no_commas (parser, NULL); + /* 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) + && !DECL_EXTERNAL (decl) + && !TREE_STATIC (decl) + && ret.value == decl + && !warn_init_self) + suppress_warning (decl, OPT_Winit_self); if (TREE_CODE (ret.value) != STRING_CST && TREE_CODE (ret.value) != COMPOUND_LITERAL_EXPR) ret = convert_lvalue_to_rvalue (loc, ret, true, true); @@ -22576,7 +22587,7 @@ c_parser_omp_declare_reduction (c_parser *parser, enum pragma_context context) location_t loc = c_parser_peek_token (parser)->location; rich_location richloc (line_table, loc); start_init (omp_priv, NULL_TREE, 0, &richloc); - struct c_expr init = c_parser_initializer (parser); + struct c_expr init = c_parser_initializer (parser, omp_priv); finish_init (); finish_decl (omp_priv, loc, init.value, init.original_type, NULL_TREE); diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 70ad681467e..ff56fddba54 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -8253,6 +8253,14 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p, && !TYPE_REF_P (type)) TREE_CONSTANT (decl) = 1; } + /* 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 (!DECL_EXTERNAL (decl) + && !TREE_STATIC (decl) + && decl == tree_strip_any_location_wrapper (init) + && !warn_init_self) + suppress_warning (decl, OPT_Winit_self); } if (flag_openmp 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..740b83b5e9f --- /dev/null +++ b/gcc/testsuite/c-c++-common/Winit-self1.c @@ -0,0 +1,31 @@ +/* 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 () +{ + volatile int k = k; + return k; +} + +int +fn4 () +{ + const volatile int l = l; + 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: 21c7aab09805d0c8c7695c8a69c8715d673a739a -- 2.37.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] c-family: Honor -Wno-init-self for cv-qual vars [PR102633] 2022-08-08 19:06 ` [PATCH v2] " Marek Polacek @ 2022-08-11 2:05 ` Jason Merrill 2022-08-11 22:30 ` Jason Merrill 1 sibling, 0 replies; 9+ messages in thread From: Jason Merrill @ 2022-08-11 2:05 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches, Joseph Myers, Richard Biener On 8/8/22 12:06, Marek Polacek wrote: > On Sat, Aug 06, 2022 at 03:29:05PM -0700, Jason Merrill wrote: >> On 7/26/22 14:31, Marek Polacek wrote: >>> On Tue, Jul 26, 2022 at 04:24:18PM -0400, Jason Merrill wrote: >>>> 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. >>> >>> I could, something like >>> >>> @@ -5381,7 +5381,14 @@ finish_decl (tree decl, location_t init_loc, tree init, >>> init = NULL_TREE; >>> >>> if (init) >>> - store_init_value (init_loc, decl, init, origtype); >>> + { >>> + /* In the self-init case, undo the artificial NOP_EXPR we may have >>> + added in convert_lvalue_to_rvalue so that c_gimplify_expr/DECL_EXPR >>> + can perform suppress_warning. */ >>> + if (TREE_CODE (init) == NOP_EXPR && TREE_OPERAND (init, 0) == decl) >>> + init = TREE_OPERAND (init, 0); >>> + store_init_value (init_loc, decl, init, origtype); >>> + } >>> >>> but then I'd have to do the same thing in cp_finish_decl because >>> decay_conversion also adds a NOP_EXPR for cv-qualified non-class prvalues. >>> Is that what we want? To me that seems less clean than having c_gimplify_expr >>> see through NOP_EXPRs. >> >> I was thinking of checking the form of the initializer before >> decay_conversion or anything else messes with it, and calling >> suppress_warning at that point instead of in c_gimplify_expr. > > Aaah, okay. Here's a patch that does it. In the C FE it has to > happen really early. Now both front ends behave the same wrt volatiles! > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? LGTM. > -- >8 -- > 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 by moving the suppress_warning call from > c_gimplify_expr to the front ends, at points where we haven't created > the NOP_EXPR yet. > > PR middle-end/102633 > > gcc/c-family/ChangeLog: > > * c-gimplify.cc (c_gimplify_expr) <case DECL_EXPR>: Don't call > suppress_warning here. > > gcc/c/ChangeLog: > > * c-parser.cc (c_parser_initializer): Add new tree parameter. Use it. > Call suppress_warning. > (c_parser_declaration_or_fndef): Pass d down to c_parser_initializer. > (c_parser_omp_declare_reduction): Pass omp_priv down to > c_parser_initializer. > > gcc/cp/ChangeLog: > > * decl.cc (cp_finish_decl): Call suppress_warning. > > 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 | 12 --------- > gcc/c/c-parser.cc | 19 ++++++++++++--- > gcc/cp/decl.cc | 8 ++++++ > gcc/testsuite/c-c++-common/Winit-self1.c | 31 ++++++++++++++++++++++++ > gcc/testsuite/c-c++-common/Winit-self2.c | 31 ++++++++++++++++++++++++ > 5 files changed, 85 insertions(+), 16 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..039a4b93230 100644 > --- a/gcc/c-family/c-gimplify.cc > +++ b/gcc/c-family/c-gimplify.cc > @@ -708,18 +708,6 @@ c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED, > break; > } > > - case DECL_EXPR: > - /* 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; > - > case PREINCREMENT_EXPR: > case PREDECREMENT_EXPR: > case POSTINCREMENT_EXPR: > diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc > index 92049d1a101..6ed17ed9c48 100644 > --- a/gcc/c/c-parser.cc > +++ b/gcc/c/c-parser.cc > @@ -1521,7 +1521,7 @@ static struct c_arg_info *c_parser_parms_list_declarator (c_parser *, tree, > static struct c_parm *c_parser_parameter_declaration (c_parser *, tree, bool); > static tree c_parser_simple_asm_expr (c_parser *); > static tree c_parser_gnu_attributes (c_parser *); > -static struct c_expr c_parser_initializer (c_parser *); > +static struct c_expr c_parser_initializer (c_parser *, tree); > static struct c_expr c_parser_braced_init (c_parser *, tree, bool, > struct obstack *); > static void c_parser_initelt (c_parser *, struct obstack *); > @@ -2286,7 +2286,7 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, > int flag_sanitize_save = flag_sanitize; > if (TREE_CODE (d) == PARM_DECL) > flag_sanitize = 0; > - init = c_parser_initializer (parser); > + init = c_parser_initializer (parser, d); > flag_sanitize = flag_sanitize_save; > finish_init (); > } > @@ -5211,11 +5211,13 @@ c_parser_type_name (c_parser *parser, bool alignas_ok) > Any expression without commas is accepted in the syntax for the > constant-expressions, with non-constant expressions rejected later. > > + DECL is the declaration we're parsing this initializer for. > + > This function is only used for top-level initializers; for nested > ones, see c_parser_initval. */ > > static struct c_expr > -c_parser_initializer (c_parser *parser) > +c_parser_initializer (c_parser *parser, tree decl) > { > if (c_parser_next_token_is (parser, CPP_OPEN_BRACE)) > return c_parser_braced_init (parser, NULL_TREE, false, NULL); > @@ -5224,6 +5226,15 @@ c_parser_initializer (c_parser *parser) > struct c_expr ret; > location_t loc = c_parser_peek_token (parser)->location; > ret = c_parser_expr_no_commas (parser, NULL); > + /* 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) > + && !DECL_EXTERNAL (decl) > + && !TREE_STATIC (decl) > + && ret.value == decl > + && !warn_init_self) > + suppress_warning (decl, OPT_Winit_self); > if (TREE_CODE (ret.value) != STRING_CST > && TREE_CODE (ret.value) != COMPOUND_LITERAL_EXPR) > ret = convert_lvalue_to_rvalue (loc, ret, true, true); > @@ -22576,7 +22587,7 @@ c_parser_omp_declare_reduction (c_parser *parser, enum pragma_context context) > location_t loc = c_parser_peek_token (parser)->location; > rich_location richloc (line_table, loc); > start_init (omp_priv, NULL_TREE, 0, &richloc); > - struct c_expr init = c_parser_initializer (parser); > + struct c_expr init = c_parser_initializer (parser, omp_priv); > finish_init (); > finish_decl (omp_priv, loc, init.value, > init.original_type, NULL_TREE); > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > index 70ad681467e..ff56fddba54 100644 > --- a/gcc/cp/decl.cc > +++ b/gcc/cp/decl.cc > @@ -8253,6 +8253,14 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p, > && !TYPE_REF_P (type)) > TREE_CONSTANT (decl) = 1; > } > + /* 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 (!DECL_EXTERNAL (decl) > + && !TREE_STATIC (decl) > + && decl == tree_strip_any_location_wrapper (init) > + && !warn_init_self) > + suppress_warning (decl, OPT_Winit_self); > } > > if (flag_openmp > 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..740b83b5e9f > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/Winit-self1.c > @@ -0,0 +1,31 @@ > +/* 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 () > +{ > + volatile int k = k; > + return k; > +} > + > +int > +fn4 () > +{ > + const volatile int l = l; > + 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: 21c7aab09805d0c8c7695c8a69c8715d673a739a ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] c-family: Honor -Wno-init-self for cv-qual vars [PR102633] 2022-08-08 19:06 ` [PATCH v2] " Marek Polacek 2022-08-11 2:05 ` Jason Merrill @ 2022-08-11 22:30 ` Jason Merrill 1 sibling, 0 replies; 9+ messages in thread From: Jason Merrill @ 2022-08-11 22:30 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches, Joseph Myers, Richard Biener On 8/8/22 12:06, Marek Polacek wrote: > On Sat, Aug 06, 2022 at 03:29:05PM -0700, Jason Merrill wrote: >> On 7/26/22 14:31, Marek Polacek wrote: >>> On Tue, Jul 26, 2022 at 04:24:18PM -0400, Jason Merrill wrote: >>>> 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. >>> >>> I could, something like >>> >>> @@ -5381,7 +5381,14 @@ finish_decl (tree decl, location_t init_loc, tree init, >>> init = NULL_TREE; >>> >>> if (init) >>> - store_init_value (init_loc, decl, init, origtype); >>> + { >>> + /* In the self-init case, undo the artificial NOP_EXPR we may have >>> + added in convert_lvalue_to_rvalue so that c_gimplify_expr/DECL_EXPR >>> + can perform suppress_warning. */ >>> + if (TREE_CODE (init) == NOP_EXPR && TREE_OPERAND (init, 0) == decl) >>> + init = TREE_OPERAND (init, 0); >>> + store_init_value (init_loc, decl, init, origtype); >>> + } >>> >>> but then I'd have to do the same thing in cp_finish_decl because >>> decay_conversion also adds a NOP_EXPR for cv-qualified non-class prvalues. >>> Is that what we want? To me that seems less clean than having c_gimplify_expr >>> see through NOP_EXPRs. >> >> I was thinking of checking the form of the initializer before >> decay_conversion or anything else messes with it, and calling >> suppress_warning at that point instead of in c_gimplify_expr. > > Aaah, okay. Here's a patch that does it. In the C FE it has to > happen really early. Now both front ends behave the same wrt volatiles! > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? LGTM. > -- >8 -- > 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 by moving the suppress_warning call from > c_gimplify_expr to the front ends, at points where we haven't created > the NOP_EXPR yet. > > PR middle-end/102633 > > gcc/c-family/ChangeLog: > > * c-gimplify.cc (c_gimplify_expr) <case DECL_EXPR>: Don't call > suppress_warning here. > > gcc/c/ChangeLog: > > * c-parser.cc (c_parser_initializer): Add new tree parameter. Use it. > Call suppress_warning. > (c_parser_declaration_or_fndef): Pass d down to c_parser_initializer. > (c_parser_omp_declare_reduction): Pass omp_priv down to > c_parser_initializer. > > gcc/cp/ChangeLog: > > * decl.cc (cp_finish_decl): Call suppress_warning. > > 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 | 12 --------- > gcc/c/c-parser.cc | 19 ++++++++++++--- > gcc/cp/decl.cc | 8 ++++++ > gcc/testsuite/c-c++-common/Winit-self1.c | 31 ++++++++++++++++++++++++ > gcc/testsuite/c-c++-common/Winit-self2.c | 31 ++++++++++++++++++++++++ > 5 files changed, 85 insertions(+), 16 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..039a4b93230 100644 > --- a/gcc/c-family/c-gimplify.cc > +++ b/gcc/c-family/c-gimplify.cc > @@ -708,18 +708,6 @@ c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED, > break; > } > > - case DECL_EXPR: > - /* 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; > - > case PREINCREMENT_EXPR: > case PREDECREMENT_EXPR: > case POSTINCREMENT_EXPR: > diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc > index 92049d1a101..6ed17ed9c48 100644 > --- a/gcc/c/c-parser.cc > +++ b/gcc/c/c-parser.cc > @@ -1521,7 +1521,7 @@ static struct c_arg_info *c_parser_parms_list_declarator (c_parser *, tree, > static struct c_parm *c_parser_parameter_declaration (c_parser *, tree, bool); > static tree c_parser_simple_asm_expr (c_parser *); > static tree c_parser_gnu_attributes (c_parser *); > -static struct c_expr c_parser_initializer (c_parser *); > +static struct c_expr c_parser_initializer (c_parser *, tree); > static struct c_expr c_parser_braced_init (c_parser *, tree, bool, > struct obstack *); > static void c_parser_initelt (c_parser *, struct obstack *); > @@ -2286,7 +2286,7 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, > int flag_sanitize_save = flag_sanitize; > if (TREE_CODE (d) == PARM_DECL) > flag_sanitize = 0; > - init = c_parser_initializer (parser); > + init = c_parser_initializer (parser, d); > flag_sanitize = flag_sanitize_save; > finish_init (); > } > @@ -5211,11 +5211,13 @@ c_parser_type_name (c_parser *parser, bool alignas_ok) > Any expression without commas is accepted in the syntax for the > constant-expressions, with non-constant expressions rejected later. > > + DECL is the declaration we're parsing this initializer for. > + > This function is only used for top-level initializers; for nested > ones, see c_parser_initval. */ > > static struct c_expr > -c_parser_initializer (c_parser *parser) > +c_parser_initializer (c_parser *parser, tree decl) > { > if (c_parser_next_token_is (parser, CPP_OPEN_BRACE)) > return c_parser_braced_init (parser, NULL_TREE, false, NULL); > @@ -5224,6 +5226,15 @@ c_parser_initializer (c_parser *parser) > struct c_expr ret; > location_t loc = c_parser_peek_token (parser)->location; > ret = c_parser_expr_no_commas (parser, NULL); > + /* 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) > + && !DECL_EXTERNAL (decl) > + && !TREE_STATIC (decl) > + && ret.value == decl > + && !warn_init_self) > + suppress_warning (decl, OPT_Winit_self); > if (TREE_CODE (ret.value) != STRING_CST > && TREE_CODE (ret.value) != COMPOUND_LITERAL_EXPR) > ret = convert_lvalue_to_rvalue (loc, ret, true, true); > @@ -22576,7 +22587,7 @@ c_parser_omp_declare_reduction (c_parser *parser, enum pragma_context context) > location_t loc = c_parser_peek_token (parser)->location; > rich_location richloc (line_table, loc); > start_init (omp_priv, NULL_TREE, 0, &richloc); > - struct c_expr init = c_parser_initializer (parser); > + struct c_expr init = c_parser_initializer (parser, omp_priv); > finish_init (); > finish_decl (omp_priv, loc, init.value, > init.original_type, NULL_TREE); > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > index 70ad681467e..ff56fddba54 100644 > --- a/gcc/cp/decl.cc > +++ b/gcc/cp/decl.cc > @@ -8253,6 +8253,14 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p, > && !TYPE_REF_P (type)) > TREE_CONSTANT (decl) = 1; > } > + /* 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 (!DECL_EXTERNAL (decl) > + && !TREE_STATIC (decl) > + && decl == tree_strip_any_location_wrapper (init) > + && !warn_init_self) > + suppress_warning (decl, OPT_Winit_self); > } > > if (flag_openmp > 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..740b83b5e9f > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/Winit-self1.c > @@ -0,0 +1,31 @@ > +/* 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 () > +{ > + volatile int k = k; > + return k; > +} > + > +int > +fn4 () > +{ > + const volatile int l = l; > + 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: 21c7aab09805d0c8c7695c8a69c8715d673a739a ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c-family: Honor -Wno-init-self for cv-qual vars [PR102633] 2022-07-26 19:03 [PATCH] c-family: Honor -Wno-init-self for cv-qual vars [PR102633] Marek Polacek 2022-07-26 20:24 ` Jason Merrill @ 2022-07-27 6:41 ` Richard Biener 2022-07-27 11:37 ` Marek Polacek 1 sibling, 1 reply; 9+ messages in thread From: Richard Biener @ 2022-07-27 6:41 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches, Jason Merrill, Joseph Myers On Tue, 26 Jul 2022, 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? As I said in the bug the bug is probably that we emit uninitialized diagnostics for volatiles at all? OTOH 'volatile' is recommended for vars live around setjmp/longjmp and there diagnostics would be welcome. It's probably the difference between "compiler-hands-off" and "hardware-controlled" :/ > > 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. > > 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 > -- Richard Biener <rguenther@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] c-family: Honor -Wno-init-self for cv-qual vars [PR102633] 2022-07-27 6:41 ` [PATCH] " Richard Biener @ 2022-07-27 11:37 ` Marek Polacek 0 siblings, 0 replies; 9+ messages in thread From: Marek Polacek @ 2022-07-27 11:37 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches, Joseph Myers On Wed, Jul 27, 2022 at 06:41:09AM +0000, Richard Biener via Gcc-patches wrote: > On Tue, 26 Jul 2022, 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? > > As I said in the bug the bug is probably that we emit uninitialized > diagnostics for volatiles at all? Non-volatile const variables also have this problem, so I think we still need this patch (or something like it). > OTOH 'volatile' is recommended > for vars live around setjmp/longjmp and there diagnostics would be > welcome. It's probably the difference between "compiler-hands-off" > and "hardware-controlled" :/ Ah, you're right. Then I guess it's better to leave the warning be, (but we still have a discrepancy between C and C++). Marek ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-08-11 22:31 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-26 19:03 [PATCH] c-family: Honor -Wno-init-self for cv-qual vars [PR102633] Marek Polacek 2022-07-26 20:24 ` Jason Merrill 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
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).