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 <dmalc...@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
