Sharing gdb's enum-flags.h with gcc? (was Re: [PATCH 01/10] Convert dump and optgroup flags to enums)

[David Malcolm <dmalcolm at redhat dot com>]

On Tue, 2018-06-05 at 04:40 -0400, Trevor Saunders wrote:
> On Fri, Jun 01, 2018 at 12:00:09PM +0200, Richard Biener wrote:
> > On Tue, May 29, 2018 at 10:32 PM David Malcolm <dmalcolm@redhat.com
> > > wrote:
> > > 
> > > The dump machinery uses "int" in a few places, for two different
> > > sets of bitmasks.
> > > 
> > > This patch makes things more self-documenting and type-safe by
> > > using
> > > a new pair of enums: one for the dump_flags_t and another for the
> > > optgroup_flags.
> > 
> > Great!  This should also make them accessible in gdb w/o using -g3.
> > 
> > > This requires adding some overloaded bit operations to the enums
> > > in question, which, in this patch is done for each enum .  If the
> > > basic
> > > idea is OK, should I add a template for this?  (with some kind of
> > > magic to express that bitmasking operations are only supported on
> > > certain opt-in enums).
> 
> You may want to look at gdb's enum-flags.h which I think already
> implements what your doing here.

Aha!  Thanks!

Browsing the git web UI, that gdb header was introduced by Pedro Alves
in:
  https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=8d297bbf604c8318ffc72d5a7b3db654409c5ed9
and the current state is here:
  https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/common/enum-flags.h;hb=HEAD

I'll try this out with GCC; hopefully it works with C++98.

Presumably it would be good to share this header between GCC and GDB;
CCing Pedro; Pedro: hi!  Does this sound sane?
(for reference, the GCC patch we're discussing here is:
  https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01685.html )

Presumably gcc's copy should live in the gcc's top-level "include"
subdirectory?

Would we need to change that "This file is part of GDB." comment, and
the include guards' "COMMON_ENUM_FLAGS_H"?

> > Does C++ allow > int enums?  I think we want some way of knowing
> > when either enum exceeds int (dump_flags_t was already uint64_t
> > but you now make it effectively int again).  That is, general
> > wrapping
> > for enum ops should chose an appropriate unsigned integer for
> > the operation.  So yes, a common implementation looks useful to me.
> 
> I don't remember very well, but istr C++ will actually use a 8 byte
> integer if the enum contains constants larger than 2^32.  Testing
> sizeof enum x { A =0x400000000 }; gives the desired thing for me, but
> it
> would still be good to check the standard.

FWIW C++11 onwards has a std::underlying_type for enums:
  http://en.cppreference.com/w/cpp/types/underlying_type
(GCC is on C++98).  The gdb header seems to emulate this via the use of
sizeof(T) to select an appropriate integer_for_size specialization and
thus the appropriate struct enum_underlying_type specialization (if I'm
reading it right).

> Trev
> 
> 
> > 
> > I think this patch is independently useful.

Richard: by this, did you mean that the patch is OK for trunk as-is?
(keeping a more general bitflags enum patch as followup work)  Note to
self: still needs bootstrap-and-testing.

> > Thanks,
> > Richard.

Thanks
Dave