Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Simon Marchi <simon.marchi@ericsson.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH] Default initialize enum flags to 0
Date: Mon, 20 Feb 2017 23:18:00 -0000	[thread overview]
Message-ID: <d8c3b970-00ac-cd5b-b786-8d2b944d9d08@redhat.com> (raw)
In-Reply-To: <20170220214548.18024-1-simon.marchi@ericsson.com>

On 02/20/2017 09:45 PM, Simon Marchi wrote:
> ... so that we don't need to do it manually, and potentially forget.
> For example, this allows to do:
> 
>   my_flags flags;
> 
>   ...
> 
>   flags |= some_flag;
> 
> gdb/ChangeLog:
> 
> 	* common/enum-flags.h (enum_flags::enum_flags): Initialize
> 	m_enum_value to 0 in default constructor.

Not sure I really like this.  3 reasons off hand.  None are
very strong, but let me put them out nonetheless:

#1 - I have some desire of creating a "gdb/gnu template library" dir
and moving these utilities there, in order to share them with more
projects (e.g., gcc, and who knows, gold and who knows other parts
of binutils that might want to convert to C++ in the future), and
it'd be nice to keep the type behaving the same in C and C++
modes (that's why I had left the !__cplusplus branch in
the file).  [1].

#2 - The other reason is that it's nice IMO to leave enums and enum flags
easily interchangeable -- i.e., make them behave as close as possible.
Having one be default initialized, and the other value initialized
means that when changing variables from one type to the other
one needs to consider that aspect.

#3 - Default initializing to zero can hide bugs that would otherwise
be caught with -Winitialized.

[1] - Trevor expressed something similar before too [2]:
https://sourceware.org/ml/gdb-patches/2016-11/msg00114.html

[2] - Yes, I need to find some time to post a v2 of that
      series.  :-P  (It's baking slowing on my github.)

Thanks,
Pedro Alves


  reply	other threads:[~2017-02-20 23:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-20 21:46 Simon Marchi
2017-02-20 23:18 ` Pedro Alves [this message]
2017-02-21  3:01   ` Simon Marchi
2017-02-21 11:16     ` Pedro Alves
2017-02-21 16:51       ` Simon Marchi

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=d8c3b970-00ac-cd5b-b786-8d2b944d9d08@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=simon.marchi@ericsson.com \
    /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