On 6/5/18, David Malcolm <dmalc...@redhat.com> wrote:
> On Tue, 2018-06-05 at 17:13 +0100, Pedro Alves wrote:
>> On 06/05/2018 03:49 PM, David Malcolm wrote:
>> > On Tue, 2018-06-05 at 04:40 -0400, Trevor Saunders wrote:
>> > > 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=commit
>> > diff;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.
>>
>> It should -- it was written/added while GDB still required C++98.
>
> Thanks.
>
>> Note I have a WIP series here:
>>
>>  https://github.com/palves/gdb/commits/palves/cxx11-enum-flags
>>
>> that fixes a few things, and adds a bunch of unit tests.  In
>> the process, it uses C++11 constructs (hence the branch's name),
>> but I think that can be reverted to still work with C++98.
>>
>> I had seen some noises recently about GCC maybe considering
>> requiring C++11.  Is that reasonable to expect in this cycle?
>> (GDB requires GCC 4.8 or later, FWIW.)
>
> I'm expecting that GCC 9 will still require C++98.

Not that I particularly want GCC to move to C++11 myself, but could
GCC at least switch from building with -Wno-narrowing to building with
-Wnarrowing? That'd make it easier to switch to C++11 when the time
comes, if anyone actually wants to do that.

>
>> Getting that branch into master had fallen lower on my TODO,
>> but if there's interest, I can try to finish it off soon, so we
>> can share a better baseline.  (I posted it once, but found
>> some issues which I fixed since but never managed to repost.)
>
> Interestingly, it looks like gdb is using Google Test for selftests;
> gcc is using a hand-rolled API that is similar, but has numerous
> differences (e.g. explicit calling of test functions, rather than
> implicit test discovery).  So that's another area of drift between
> the projects.
>
>> >
>> > 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 )
>>
>> Sure!
>
> Alternatively, if GCC needs to stay at C++98 and GDB moves on to C++11,
> then we can at least take advantage of this tested and FSF-assigned
> C++98 code (better than writing it from scratch).
>
> I ran into one issue with the header, e.g. with:
>
>   /* Dump flags type.  */
>   DEF_ENUM_FLAGS_TYPE(enum dump_flag, dump_flags_t);
>
> This doesn't work:
>   TDF_SLIM | flags
> but this does:
>   flags | TDF_SLIM
> where TDF_SLIM is one of the values of "enum dump_flag".
>
> For example, the former leads to:
>
> ../../src/gcc/c-family/c-gimplify.c: In function ‘void c_genericize(tree)’:
> ../../src/gcc/c-family/c-gimplify.c:145:44: error: conversion from ‘int’ to
> ‘dump_flags_t’ {aka ‘enum_flags<dump_flag>’} is ambiguous
>       TDF_SLIM | local_dump_flags, dump_orig);
>                                             ^
> In file included from ../../src/gcc/dumpfile.h:24,
>                  from ../../src/gcc/coretypes.h:428,
>                  from ../../src/gcc/c-family/c-gimplify.c:28:
> ../../src/gcc/../include/enum-flags.h:128:3: note: candidate:
> ‘enum_flags<E>::enum_flags(enum_flags<E>::zero_type*) [with E = dump_flag]’
> <near match>
>    enum_flags (struct enum_flags::zero_type *zero)
>    ^~~~~~~~~~
> ../../src/gcc/../include/enum-flags.h:128:3: note:   conversion of argument
> 1 would be ill-formed:
> ../../src/gcc/c-family/c-gimplify.c:145:15: error: invalid conversion from
> ‘int’ to ‘enum_flags<dump_flag>::zero_type*’ [-fpermissive]
>       TDF_SLIM | local_dump_flags, dump_orig);
>       ~~~~~~~~~^~~~~~~~~~~~~~~~~~
> In file included from ../../src/gcc/dumpfile.h:24,
>                  from ../../src/gcc/coretypes.h:428,
>                  from ../../src/gcc/c-family/c-gimplify.c:28:
> ../../src/gcc/../include/enum-flags.h:125:3: note: candidate:
> ‘enum_flags<E>::enum_flags(enum_flags<E>::enum_type) [with E = dump_flag;
> enum_flags<E>::enum_type = dump_flag]’ <near match>
>    enum_flags (enum_type e)
>    ^~~~~~~~~~
> ../../src/gcc/../include/enum-flags.h:125:3: note:   conversion of argument
> 1 would be ill-formed:
> ../../src/gcc/c-family/c-gimplify.c:145:15: error: invalid conversion from
> ‘int’ to ‘enum_flags<dump_flag>::enum_type’ {aka ‘dump_flag’}
> [-fpermissive]
>       TDF_SLIM | local_dump_flags, dump_orig);
>       ~~~~~~~~~^~~~~~~~~~~~~~~~~~
> In file included from ../../src/gcc/coretypes.h:428,
>                  from ../../src/gcc/c-family/c-gimplify.c:28:
> ../../src/gcc/dumpfile.h:248:13: note:   initializing argument 2 of ‘void
> dump_node(const_tree, dump_flags_t, FILE*)’
>  extern void dump_node (const_tree, dump_flags_t, FILE *);
>              ^~~~~~~~~
>
> I'm not quite sure why it doesn't work that way around, but reversing
> the order so that the dump_flags_t is on the left-hand-side lets it
> work (it only affects 3 sites in gcc's source tree).
>
>
>> Thanks,
>> Pedro Alves
>
> Thanks.
>
> BTW, I spotted this trivial issue in a comment in enum-flags.h whilst
> trying it out for gcc:
>
>
>
> The DEF_ENUM_FLAGS_TYPE macro should be used with a trailing
> semicolon, but the example in the comment lacks one.
>
>       * enum-flags.h: Add trailing semicolon to example in comment.
> ---
>  include/enum-flags.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/enum-flags.h b/include/enum-flags.h
> index 65732c1..82568a5 100644
> --- a/include/enum-flags.h
> +++ b/include/enum-flags.h
> @@ -32,7 +32,7 @@
>         flag_val3 = 1 << 3,
>         flag_val4 = 1 << 4,
>      };
> -    DEF_ENUM_FLAGS_TYPE(enum some_flag, some_flags)
> +    DEF_ENUM_FLAGS_TYPE(enum some_flag, some_flags);
>
>      some_flags f = flag_val1 | flag_val2;
>      f |= flag_val3;
> --
> 1.8.5.3
>
>

Reply via email to