From patchwork Fri Oct 30 14:15:49 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 9487 Received: (qmail 16090 invoked by alias); 30 Oct 2015 14:15:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 16066 invoked by uid 89); 30 Oct 2015 14:15:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: usevmg21.ericsson.net Received: from usevmg21.ericsson.net (HELO usevmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Fri, 30 Oct 2015 14:15:52 +0000 Received: from EUSAAHC006.ericsson.se (Unknown_Domain [147.117.188.90]) by usevmg21.ericsson.net (Symantec Mail Security) with SMTP id 03.E4.26730.76E03365; Fri, 30 Oct 2015 07:30:00 +0100 (CET) Received: from [142.133.110.144] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.92) with Microsoft SMTP Server id 14.3.248.2; Fri, 30 Oct 2015 10:15:49 -0400 Subject: Re: [PATCH] Type-safe wrapper for enum flags To: References: <1446144341-21267-1-git-send-email-palves@redhat.com> From: Simon Marchi Message-ID: <56337B95.6070809@ericsson.com> Date: Fri, 30 Oct 2015 10:15:49 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <1446144341-21267-1-git-send-email-palves@redhat.com> X-IsSubscribed: yes On 15-10-29 02:45 PM, 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 ‘int’ to ‘filterflags’ [-fpermissive] > | COREFILTER_HUGETLB_PRIVATE); > ^ > > This is a case of enums used as bit flags. Unlike "regular" enums, > these values are supposed to be or'ed together. However, in C++, the > type of "(ENUM1 | ENUM2)" is int, and you then can't assign an int to > an enum variable without a cast. That means that this: > > enum foo_flags flags = 0; > > if (...) > flags |= FOO_FLAG1; > if (...) > flags |= FOO_FLAG2; > > ... would have to be written as: > > enum foo_flags flags = (enum foo_flags) 0; > > if (...) > flags = (enum foo_flags) (flags | FOO_FLAG1); > if (...) > flags = (enum foo_flags) (flags | FOO_FLAG2); > > which is ... ugly. Alternatively, we'd have to use an int for the > variable's type, which isn't ideal either. > > This patch instead adds an "enum flags" class. "enum flags" are > exactly the 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 = 1 << 1, > flag_val2 = 1 << 2, > flag_val3 = 1 << 3, > flag_val4 = 1 << 4, > }; > DEF_ENUM_FLAGS_TYPE(enum some_flag, some_flags) > > some_flags f = flag_val1 | flag_val2; > f |= flag_val3; > > It's also possible to assign literal zero to an enum flags variable > (meaning, no flags), dispensing either adding an awkward explicit "no > value" value to the enumeration or the cast to assignments from 0. > For example: > > some_flags f = 0; > f |= flag_val3 | flag_val4; > > Note that literal integers other than zero do fail to compile: > > some_flags f = 1; // error > > C is still supported -- DEF_ENUM_FLAGS_TYPE is just a typedef in that > case. Could we also make c_string_type an enum flags type? Like so? There are some ugly casts remaining in the branch, like str_type = (enum c_string_type) (str_type & ~C_CHAR); From 2b1ee0d61aafa948243e809614522353b9e88e3c Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 29 Oct 2015 21:54:35 -0400 Subject: [PATCH] Convert c_string_type to an enum flags type c_string_type contains values meant to be OR'ed together (even though some bits are mutually exclusive), so it makes sense to make it an enum flags type. gdb/ChangeLog: * c-exp.y (exp): Adjust, change enum c_string_type to c_string_type. (parse_string_or_char): Likewise. * c-lang.c (charset_for_string_type): Likewise. (classify_type): Likewise. (c_printchar): Likewise. (c_printstr): Likewise. (evaluate_subexp_c): Likewise. And change cast to enum c_string_type_values. * c-lang.h (enum c_string_type): Rename to... (enum c_string_type_values): ...this. (c_string_type): Define new enum flags type. --- gdb/c-exp.y | 6 +++--- gdb/c-lang.c | 17 ++++++++--------- gdb/c-lang.h | 4 +++- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/gdb/c-exp.y b/gdb/c-exp.y index f2de0ae..9127756 100644 --- a/gdb/c-exp.y +++ b/gdb/c-exp.y @@ -864,7 +864,7 @@ string_exp: exp : string_exp { int i; - enum c_string_type type = C_STRING; + c_string_type type = C_STRING; for (i = 0; i < $1.len; ++i) { @@ -878,7 +878,7 @@ exp : string_exp if (type != C_STRING && type != $1.tokens[i].type) error (_("Undefined string concatenation.")); - type = (enum c_string_type) $1.tokens[i].type; + type = (enum c_string_type_values) $1.tokens[i].type; break; default: /* internal error */ @@ -2164,7 +2164,7 @@ parse_string_or_char (const char *tokptr, const char **outptr, struct typed_stoken *value, int *host_chars) { int quote; - enum c_string_type type; + c_string_type type; int is_objc = 0; /* Build the gdb internal form of the input string in tempbuf. Note diff --git a/gdb/c-lang.c b/gdb/c-lang.c index 384783c..2646b05 100644 --- a/gdb/c-lang.c +++ b/gdb/c-lang.c @@ -41,8 +41,7 @@ extern void _initialize_c_language (void); character set name. */ static const char * -charset_for_string_type (enum c_string_type str_type, - struct gdbarch *gdbarch) +charset_for_string_type (c_string_type str_type, struct gdbarch *gdbarch) { switch (str_type & ~C_CHAR) { @@ -72,11 +71,11 @@ charset_for_string_type (enum c_string_type str_type, characters of this type in target BYTE_ORDER to the host character set. */ -static enum c_string_type +static c_string_type classify_type (struct type *elttype, struct gdbarch *gdbarch, const char **encoding) { - enum c_string_type result; + c_string_type result; /* We loop because ELTTYPE may be a typedef, and we want to successively peel each typedef until we reach a type we @@ -155,7 +154,7 @@ c_emit_char (int c, struct type *type, void c_printchar (int c, struct type *type, struct ui_file *stream) { - enum c_string_type str_type; + c_string_type str_type; str_type = classify_type (type, get_type_arch (type), NULL); switch (str_type) @@ -191,7 +190,7 @@ c_printstr (struct ui_file *stream, struct type *type, const char *user_encoding, int force_ellipses, const struct value_print_options *options) { - enum c_string_type str_type; + c_string_type str_type; const char *type_encoding; const char *encoding; @@ -577,7 +576,7 @@ evaluate_subexp_c (struct type *expect_type, struct expression *exp, struct obstack output; struct cleanup *cleanup; struct value *result; - enum c_string_type dest_type; + c_string_type dest_type; const char *dest_charset; int satisfy_expected = 0; @@ -589,8 +588,8 @@ evaluate_subexp_c (struct type *expect_type, struct expression *exp, ++*pos; limit = *pos + BYTES_TO_EXP_ELEM (oplen + 1); - dest_type - = (enum c_string_type) longest_to_int (exp->elts[*pos].longconst); + dest_type = ((enum c_string_type_values) + longest_to_int (exp->elts[*pos].longconst)); switch (dest_type & ~C_CHAR) { case C_STRING: diff --git a/gdb/c-lang.h b/gdb/c-lang.h index 5e30481..b1823ce 100644 --- a/gdb/c-lang.h +++ b/gdb/c-lang.h @@ -34,7 +34,7 @@ struct parser_state; /* The various kinds of C string and character. Note that these values are chosen so that they may be or'd together in certain ways. */ -enum c_string_type +enum c_string_type_values { /* An ordinary string: "value". */ C_STRING = 0, @@ -56,6 +56,8 @@ enum c_string_type C_CHAR_32 = 7 }; +DEF_ENUM_FLAGS_TYPE (enum c_string_type_values, c_string_type); + /* Defined in c-exp.y. */ extern int c_parse (struct parser_state *);