Hi, 

After more study of all the discussion so far and the corresponding code for 
suppress_warning, I think the following suggestion
Should be the best approach right now for this issue:

>       SET_EXPR_LOCATION (tmp_dst, UNKNOWN_LOCATION);
>       suppress_warning (tmp_dst, OPT_Wuninitialized);
> with a comment explaing why we do that?


The reason is:

After “SET_EXPR_LOCATION (tmp_dst, UNKNOWN_LOCATION)”, 

152 /* Enable, or by default disable, a warning for the expression.
153    The wildcard OPT of -1 controls all warnings.  */
154 
155 void
156 suppress_warning (tree expr, opt_code opt /* = all_warnings */,
157                   bool supp /* = true */)
158 {
159   if (opt == no_warning)
160     return;
161 
162   const location_t loc = get_location (expr);
163 
164   if (!RESERVED_LOCATION_P (loc))
165     supp = suppress_warning_at (loc, opt, supp) || supp;
166   set_no_warning_bit (expr, supp);
167 }

Suppress_warning will NOT call “suppress_warning_at” to involve any operation 
on the hash tables. It just
simply call “set_no_warning_bit” to set the no_warning bit for the MEM_REF 
expr. 

And later during the routine “maybe_warn_operand” in tree-sea-uninit.cc, 
“get_no_uninit_warning” will also
Simply check the no_warning bit of the MEM_REF to see whether the warning need 
to be issued. 

This resolved all the concerns we have so far.

I will prepare the patch based on this approach.

Let me know your opinion.

Qing

> On Feb 25, 2022, at 4:04 AM, Jakub Jelinek via Gcc-patches 
> <gcc-patches@gcc.gnu.org> wrote:
> 
> On Fri, Feb 25, 2022 at 10:31:50AM +0100, Richard Biener wrote:
>> I think it's used as fallback for UNKNOWN_LOCATION, but if we "invent"
>> a creative location for the artificial stmt it will of course
>> affect other stmts/expressions using that location.
>> 
>>> I think it will work.
>> 
>> Yes, I think so.  OTOH the uninit pass does
>> 
>>  /* Avoid warning if we've already done so or if the warning has been
>>     suppressed.  */
>>  if (((warning_suppressed_p (context, OPT_Wuninitialized)
>>        || (gimple_assign_single_p (context)
>>            && get_no_uninit_warning (gimple_assign_rhs1 (context)))))
>>      || (var && get_no_uninit_warning (var))
>>      || (var_name_str
>>          && warning_suppressed_p (var_def_stmt, OPT_Wuninitialized)))
>>    return;
>> 
>> that's a mightly complicated way to test and I'm not sure we get
>> to the bit on the stmt reliably.  So maybe TREE_NO_WARNING on the
>> reference (or making sure it has UNKNOWN_LOCATION and using
>> suppress_warning on it) is a better idea after all...
> 
> So
>       SET_EXPR_LOCATION (tmp_dst, UNKNOWN_LOCATION);
>       suppress_warning (tmp_dst, OPT_Wuninitialized);
> with a comment explaing why we do that?
> LGTM.
> 
>       Jakub
> 

Reply via email to