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));
}
}
