From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 55017 invoked by alias); 1 Nov 2015 04:04:21 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 55003 invoked by uid 89); 1 Nov 2015 04:04:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-ig0-f181.google.com Received: from mail-ig0-f181.google.com (HELO mail-ig0-f181.google.com) (209.85.213.181) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sun, 01 Nov 2015 04:04:17 +0000 Received: by igbdj2 with SMTP id dj2so34795146igb.1 for ; Sat, 31 Oct 2015 21:04:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type:content-transfer-encoding; bh=7UWC9VdCHlQ+3Nmyt79XNk5ZJbu8dAg70SxaXFZK9HE=; b=IYiSmLm4P/sIsqIxzLspIr1WjJIape/BNw1Vh1OEnnxRBu5zKtW8QwA3RF+OLVfU+9 /xAD/YeqWvfWSkYJyxUNWVlA1v6ZDsF1cofRE3mYegX3s+ORimH2QMATuAgLaedX1pFG WJn6Y4/ecEaI0AcDQTFjH/TuLKDZYxFVrTd+bGKToyO3jl111dNl0dOWnzJB8a7Rnz1M qU6BrdfqCTTF0lqumGy62dDwvlDLJ2tBNoLcUSR5anwd2djCSSOE/+ZFhVo0sUO7tlkV pHSSBwLRxneuLyMN6B9GcD5zxXRyHziIhTr3cYQ0VE3l3XcRwrpx/d2BxoaDTruR0KRP eiLQ== X-Gm-Message-State: ALoCoQnMZKUuqV7lv64C4PvW++7AQf3HBNhkrPu5dSs8TASQJahcGlSi3x2URPQQXesSd7aX78zb X-Received: by 10.50.107.3 with SMTP id gy3mr5233148igb.48.1446350655550; Sat, 31 Oct 2015 21:04:15 -0700 (PDT) MIME-Version: 1.0 Received: by 10.64.159.195 with HTTP; Sat, 31 Oct 2015 21:03:35 -0700 (PDT) In-Reply-To: <1446144341-21267-1-git-send-email-palves@redhat.com> References: <1446144341-21267-1-git-send-email-palves@redhat.com> From: Doug Evans Date: Sun, 01 Nov 2015 04:04:00 -0000 Message-ID: Subject: Re: [PATCH] Type-safe wrapper for enum flags To: Pedro Alves Cc: gdb-patches Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg00002.txt.bz2 On Thu, Oct 29, 2015 at 11:45 AM, Pedro Alves wrote: > This patch fixes C++ build errors like this: > > /home/pedro/gdb/mygit/cxx-convertion/src/gdb/linux-tdep.c:1126:35: error:= invalid conversion from =E2=80=98int=E2=80=99 to =E2=80=98filterflags=E2= =80=99 [-fpermissive] > | COREFILTER_HUGETLB_PRIVATE); >... > > diff --git a/gdb/common/enum_flags.h b/gdb/common/enum_flags.h > new file mode 100644 > index 0000000..030fd2f > --- /dev/null > +++ b/gdb/common/enum_flags.h > @@ -0,0 +1,211 @@ > +/* Copyright (C) 2015 Free Software Foundation, Inc. > + > + This file is part of GDB. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see .= */ > + > +#ifndef COMMON_ENUM_FLAGS_H > +#define COMMON_ENUM_FLAGS_H > + > +/* Type-safe wrapper for enum flags. enum flags are enums where the > + values are bits that are meant to be ORed together. > + > + This allows writing code like the below, while with raw enums this > + would fail to compile without casts to enum type at the assignments > + to 'f': > + > + enum some_flag > + { > + flag_val1 =3D 1 << 1, > + flag_val2 =3D 1 << 2, > + flag_val3 =3D 1 << 3, > + flag_val4 =3D 1 << 4, > + }; > + DEF_ENUM_FLAGS_TYPE(enum some_flag, some_flags) > + > + some_flags f =3D flag_val1 | flag_val2; > + f |=3D flag_val3; > + > + It's also possible to assign literal zero to an enum flags variable > + (meaning, no flags), dispensing adding an awkward explicit "no > + value" value to the enumeration. For example: > + > + some_flags f =3D 0; > + f |=3D flag_val3 | flag_val4; > + > + Note that literal integers other than zero fail to compile: > + > + some_flags f =3D 1; // error > +*/ > + > +#ifdef __cplusplus > + > +/* Traits type used to prevent the global operator overloads from > + instantiating for non-flag enums. */ > +template struct enum_flags_type {}; > + > +/* Use this to mark an enum as flags enum. It defines FLAGS as > + enum_flags wrapper class for ENUM, and enables the global operator > + overloads for ENUM. */ > +#define DEF_ENUM_FLAGS_TYPE(enum_type, flags_type) \ > + typedef enum_flags flags_type; \ > + template<> \ > + struct enum_flags_type \ > + { \ > + typedef enum_flags type; \ > + } > + > +/* Until we can rely on std::underlying type being universally > + available (C++11), roll our own for enums. */ > +template class integer_for_size { typedef void type= ; }; > +template<> struct integer_for_size<1, 0> { typedef uint8_t type; }; > +template<> struct integer_for_size<2, 0> { typedef uint16_t type; }; > +template<> struct integer_for_size<4, 0> { typedef uint32_t type; }; > +template<> struct integer_for_size<8, 0> { typedef uint64_t type; }; > +template<> struct integer_for_size<1, 1> { typedef int8_t type; }; > +template<> struct integer_for_size<2, 1> { typedef int16_t type; }; > +template<> struct integer_for_size<4, 1> { typedef int32_t type; }; > +template<> struct integer_for_size<8, 1> { typedef int64_t type; }; > + > +template > +struct enum_underlying_type > +{ > + typedef typename > + integer_for_size(T (-1) < T (0))>::type > + type; > +}; > + > +template > +class enum_flags > +{ > +public: I see the gcc c++ coding standards say to not indent public/etc. Emacs indents one space by default, something we can fix in .dir-locals.el I hope. Note: I didn't review the order of things in the class to see if they follow convention. [better to fix on a convention from the beginning] > + typedef E enum_type; > + typedef typename enum_underlying_type::type underlying_type; > + > +private: > + /* Private type used to support initializing flag types with zero: > + > + foo_flags f =3D 0; > + > + but not other integers: > + > + foo_flags f =3D 1; > + > + The way this works is that we define an implicit constructor that > + takes a pointer to this private type. Since nothing can > + instantiate an object of this type, the only possible pointer to > + pass to the constructor is the NULL pointer, or, zero. */ > + struct zero_type; > + > + underlying_type > + underlying_value () const > + { > + return enum_value; > + } > + > +public: > + /* Allow default construction, just like raw enums. */ > + enum_flags () > + {} > + > + enum_flags (const enum_flags &other) > + : enum_value (other.enum_value) > + {} > + > + enum_flags &operator=3D (const enum_flags &other) > + { > + enum_value =3D other.enum_value; > + return *this; > + } > + > + /* If you get an error saying these two overloads are ambiguous, > + then you tried to mix values of different enum types. */ > + enum_flags (enum_type e) > + : enum_value (e) gcc c++ coding convention says to put : at column zero. > + {} > + enum_flags (struct enum_flags::zero_type *zero) > + : enum_value ((enum_type) 0) > + {} > + > + enum_flags &operator&=3D (enum_type e) > + { > + enum_value =3D (enum_type) (underlying_value () & e); > + return *this; > + } > + enum_flags &operator|=3D (enum_type e) > + { > + enum_value =3D (enum_type) (underlying_value () | e); > + return *this; > + } > + enum_flags &operator^=3D (enum_type e) > + { > + enum_value =3D (enum_type) (underlying_value () ^ e); > + return *this; > + } > + > + operator enum_type () const > + { > + return enum_value; > + } > + > + enum_flags operator& (enum_type e) const > + { > + return (enum_type) (underlying_value () & e); > + } > + enum_flags operator| (enum_type e) const > + { > + return (enum_type) (underlying_value () | e); > + } > + enum_flags operator^ (enum_type e) const > + { > + return (enum_type) (underlying_value () ^ e); > + } > + > + enum_flags operator~ () const > + { > + return (enum_type) ~underlying_value (); > + } > + > +private: > + /* Stored as enum_type because GDB knows to print the bit flags > + neatly if the enum values look like bit flags. */ > + enum_type enum_value; Sorry another nit. I see gcc doesn't use an m_ prefix for members 100% of the time. I'm not sure how we're going to decide when to use it and when not to, but we should have something clear. > +}; > + > +/* Global operator overloads. */ > + > +template > +typename enum_flags_type::type > +operator| (enum_type e1, enum_type e2) > +{ > + return enum_flags (e1) | e2; > +} > + > +template > +typename enum_flags_type::type > +operator~ (enum_type e) > +{ > + return ~enum_flags (e); > +} > + > +#else /* __cplusplus */ > + > +/* In C, the flags type is just a typedef for the enum type. */ > + > +#define DEF_ENUM_FLAGS_TYPE(enum_type, flags_type) \ > + typedef enum_type flags_type > + > +#endif /* __cplusplus */ > + > +#endif /* COMMON_ENUM_FLAGS_H */ Beyond the nits, seems fine to me (not that I'm a c++ authority).