On Wed, Nov 06 2024, Michal Jires wrote:
> On Wed, 2024-11-06 at 17:33:50 +0000, Jonathan Wakely wrote:
>> 
>> If there's going to be a constructor then it should initialize the members.
>> 
>> Otherwise, your original patch was better, because you could write
>> this to get an all-zeros object:
>> 
>>   lto_encoder_entry e{};
>> 
>> Now you can't safely initialize it, because the default constructor
>> leaves everything indeterminate. That's just a bug waiting to happen.
>> 
>
> Using all-zeros would be probably bug anyway and explicitly initializing
> might encourage thinking that such default values are supposed to be
> used.
>
> Anyway, I have misglanced the code for which this was needed, and we can
> trivially get rid of it.
>
> Is this now OK?
>

The patch is OK (this should still fall under the "callgraph" category
and in any case I think the change is rather obvious.)

You need to commit this with a ChangeLog entry, though.

Thanks,

Martin


> ---
>  gcc/lto-cgraph.cc  | 3 +--
>  gcc/lto-streamer.h | 3 +--
>  2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/lto-cgraph.cc b/gcc/lto-cgraph.cc
> index b18d2b34e46..c9b846a04d6 100644
> --- a/gcc/lto-cgraph.cc
> +++ b/gcc/lto-cgraph.cc
> @@ -142,7 +142,6 @@ lto_symtab_encoder_delete_node (lto_symtab_encoder_t 
> encoder,
>                               symtab_node *node)
>  {
>    int index;
> -  lto_encoder_entry last_node;
>  
>    size_t *slot = encoder->map->get (node);
>    if (slot == NULL || !*slot)
> @@ -153,7 +152,7 @@ lto_symtab_encoder_delete_node (lto_symtab_encoder_t 
> encoder,
>  
>    /* Remove from vector. We do this by swapping node with the last element
>       of the vector.  */
> -  last_node = encoder->nodes.pop ();
> +  lto_encoder_entry last_node = encoder->nodes.pop ();
>    if (last_node.node != node)
>      {
>        bool existed = encoder->map->put (last_node.node, index + 1);
> diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
> index 1c416a7a1b9..294e7b3e328 100644
> --- a/gcc/lto-streamer.h
> +++ b/gcc/lto-streamer.h
> @@ -443,8 +443,7 @@ struct lto_stats_d
>  /* Entry of LTO symtab encoder.  */
>  struct lto_encoder_entry
>  {
> -  /* Constructors.  */
> -  lto_encoder_entry () {}
> +  /* Constructor.  */
>    lto_encoder_entry (symtab_node* n)
>      : node (n), in_partition (false), body (false), only_for_inlining (true),
>        initializer (false)
> -- 
> 2.47.0

Reply via email to