Type-safe wrapper for enum flags
Commit Message
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 <simon.marchi@polymtl.ca>
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(-)
Comments
On 10/30/2015 02:15 PM, Simon Marchi wrote:
> 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);
Good idea, indeed looks like we can.
> --- 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);
Looks like c-lang.h should include common/enum_flags.h.
Otherwise LGTM.
I'd like to collect at least one Ack on the enum flags patch
before pushing it.
Thanks,
Pedro Alves
@@ -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
@@ -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:
@@ -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 *);