Re: [PATCH] c-family: Honor -Wno-init-self for cv-qual vars [PR102633]

[Jason Merrill <jason@redhat.com>]

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