On June 5, 2018 4:49:21 PM GMT+02:00, David Malcolm <dmalc...@redhat.com> wrote: >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?
Yes. Richard. >(keeping a more general bitflags enum patch as followup work) Note to >self: still needs bootstrap-and-testing. > >> > Thanks, >> > Richard. > >Thanks >Dave
