digitalmars.D.learn - EnumToFlags
I created a type that makes working with flags much easier. Please review for issues and enhancements. It would be nice to simplify the value size code. struct EnumToFlags(alias E) { import std.traits, std.conv, std.string, std.algorithm, std.array; static if (E.max < 8) alias vtype = ubyte; else static if (E.max < 16) alias vtype = ushort; else static if (E.max < 32) alias vtype = uint; else static if (E.max < 64) alias vtype = ulong; else static if (E.max < 128) alias vtype = ucent; else static assert("Cannot store more than 128 flags in an enum."); vtype value; template opDispatch(string Name) { enum opDispatch = 1 << __traits(getMember, E, Name); } auto opAssign(vtype e) { value = e; } bool opEquals(typeof(this) e) { if (e is this) return true; return (value == e.value); } auto opBinary(string op)(typeof(this) e) { auto x = typeof(this)(); mixin("x.value = this.value "~op~" e.value;"); return x; } auto opUnary(string op)() { auto x = typeof(this)(); mixin("x.value = "~op~"this.value;"); return x; } auto opCast() { return value; } auto toString() { string s; foreach(i, e; EnumMembers!E) { if (((1 << i) & value) == 1 << i) s ~= to!string(e) ~ " | "; } if (s.length > 0) s = s[0..$-3]; return s; } this(string from) { char uOp = ' '; char Op = '='; char nOp = '='; int previdx = 0; value = 0; for(int i = 0; i < from.length; i++) { nOp = from[i]; if (nOp == '!' || nOp == '~') { uOp = nOp; previdx = i + 1; continue; } if (nOp == '&' || nOp == '^' || nOp == '|' || nOp == '+' || nOp == '-' || i == from.length-1) { auto v = from[previdx..(i + ((i == from.length-1) ? 1 : 0))].strip(); vtype x = 0; foreach(j, e; EnumMembers!E) if (to!string(e) == v) x = cast(vtype)(1 << j); if (uOp == '!') x = !x; if (uOp == '~') x = ~x; switch(Op) { case '&': value = cast(vtype)(value & x); break; case '|': value = cast(vtype)(value | x); break; case '+': value = cast(vtype)(value + x); break; case '-': value = cast(vtype)(value - x); break; case '^': value = cast(vtype)(value ^ x); break; case '=': value = cast(vtype)(x); break; default: assert("Invalid EnumFlags operation `"~Op~"`."); } previdx = i + 1; Op = nOp; uOp = ' '; } } } alias value this; }; Possibly the to and from string stuff can be improved? Simply apply to a pre-existing enum.
Jun 29 2016
On 06/30/2016 04:39 AM, JS wrote:struct EnumToFlags(alias E) { import std.traits, std.conv, std.string, std.algorithm, std.array; static if (E.max < 8) alias vtype = ubyte;Convention says: Capitalize user-defined type names. So it should be "Vtype" or maybe "VType" instead of "vtype".else static if (E.max < 16) alias vtype = ushort; else static if (E.max < 32) alias vtype = uint; else static if (E.max < 64) alias vtype = ulong; else static if (E.max < 128) alias vtype = ucent;There is no ucent type. The keyword is reserved, but there is no actual type.else static assert("Cannot store more than 128 flags in an enum."); vtype value; template opDispatch(string Name) { enum opDispatch = 1 << __traits(getMember, E, Name);`1` needs to be `1L` here, or maybe `1UL`. Can't shift more than 31 bits otherwise, because `1` is an int. Same throughout. Note that you're shifting by the enum member's value here.} auto opAssign(vtype e) { value = e; }In my opinion, a void return type would be clearer here.bool opEquals(typeof(this) e) { if (e is this) return true; return (value == e.value); }opEquals should be const. The parentheses around the return value are pointless. Does this opEquals do anything different from the default one? As far as I see, `e is this` does the same as `value == e.value` and the default equality would do the same, too.auto opBinary(string op)(typeof(this) e) { auto x = typeof(this)(); mixin("x.value = this.value "~op~" e.value;"); return x; } auto opUnary(string op)() { auto x = typeof(this)(); mixin("x.value = "~op~"this.value;"); return x; } auto opCast() { return value; }When is this opCast going to be called?auto toString() { string s; foreach(i, e; EnumMembers!E) { if (((1 << i) & value) == 1 << i) s ~= to!string(e) ~ " | "; } if (s.length > 0) s = s[0..$-3]; return s; } this(string from) { char uOp = ' '; char Op = '=';Convention says: Don't capitalize variable names. So it should be "op" instead of "Op".char nOp = '='; int previdx = 0; value = 0; for(int i = 0; i < from.length; i++)There's a subtle problem here with i's type. from.length is a size_t, i.e. ulong in a 64 bit program. So it may be larger than int.max. When that happens, you've an infinite loop here, as i can never reach from.length. Solution: use size_t for i and previdx. Also, foreach is nicer: foreach (i; 0 .. from.length){ nOp = from[i]; if (nOp == '!' || nOp == '~') { uOp = nOp; previdx = i + 1; continue; } if (nOp == '&' || nOp == '^' || nOp == '|' || nOp == '+' || nOp == '-' || i == from.length-1) { auto v = from[previdx..(i + ((i == from.length-1) ? 1 : 0))].strip(); vtype x = 0; foreach(j, e; EnumMembers!E) if (to!string(e) == v) x = cast(vtype)(1 << j);Here you're shifting by the enum member's index. In opDispatch you shift by its value. That difference leads to this: enum E {a = 0, b = 2} alias F = EnumToFlags!E; assert(F("a") is F.a); /* passes */ assert(F("b") is F.b); /* fails */if (uOp == '!') x = !x; if (uOp == '~') x = ~x; switch(Op) { case '&': value = cast(vtype)(value & x); break; case '|': value = cast(vtype)(value | x); break; case '+': value = cast(vtype)(value + x); break; case '-': value = cast(vtype)(value - x); break; case '^': value = cast(vtype)(value ^ x); break; case '=': value = cast(vtype)(x); break; default: assert("Invalid EnumFlags operation `"~Op~"`."); } previdx = i + 1; Op = nOp; uOp = ' '; } } } alias value this; };No semicolon after struct declarations in D. You forgot to include some tests or a usage example. It's not obvious to me what EnumToFlags is supposed to accomplish or how to use it.Possibly the to and from string stuff can be improved? Simply apply to a pre-existing enum.I'm not sure what the purpose of all this is, but it seems a bit convoluted to me. Conversion from string seems pointless. In my opinion, this: alias F = EnumToFlags!E; auto f = F("foo | bar"); isn't better than this: alias F = EnumToFlags!E; auto f = F.foo | F.bar; For conversion to string I can't think of a use case other than debugging. And for debugging a free function would be ok, too, I think. Without the conversions from/to string, the struct just forwards to the value member, and one could just use the integer directly. Have you considered simply generating a new enum with power-of-two values? For example, like so: template Flags(E) if (is(E == enum)) { import std.format: format; import std.traits: EnumMembers; enum code = () { string result = "enum Flags {"; foreach (e; EnumMembers!E) result ~= format("%s = 1L << %dL,", e, e); return result ~ "}"; }(); mixin(code); } unittest { enum E { e0, e1, e2, e3, e4, e5, e6, e7, e8, e9, e10, e11, e12, e13, e14, e15, e16, e17, e18, e19, e20, e21, e22, e23, e24, e25, e26, e27, e28, e29, e30, e31, e32, e33, e34, e35, e36, e37, e38, e39, e40, e41, e42, e43, e44, e45, e46, e47, e48, e49, e50, e51, e52, e53, e54, e55, e56, e57, e58, e59, e60, e61, e62, e63 } with (Flags!E) { assert((e0 | e1) == 0b11); assert((e1 | e3) == 0b1010); assert((e0 | e63) == 0x80_00_00_00_00_00_00_01); assert((e0 | e9 | e18 | e27 | e36 | e45 | e54 | e63) == 0x80_40_20_10_08_04_02_01); assert(0b1010 & e1); assert(0b1010 & e3); assert(!(0b1010 & e0)); assert(!(0b1010 & e2)); assert(!(0b1010 & e4)); } }
Jun 30 2016
On Thursday, 30 June 2016 at 02:39:22 UTC, JS wrote:I created a type that makes working with flags much easier. Please review for issues and enhancements. It would be nice to simplify the value size code. [...]You can look at this, it's more or less the same concept: https://github.com/BBasile/iz/blob/master/import/iz/enumset.d#L226 Two things that important: - enum members are not always integer numbers. - enum used as flags is only fast when members values are consecutives. I've recently discovered that it can even be used to make UDAs, what a "yeah" ;)
Jun 30 2016