Re: EnumToFlags

2016-06-30 Thread Basile B. via Digitalmars-d-learn

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


Re: EnumToFlags

2016-06-30 Thread ag0aep6g via Digitalmars-d-learn

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 fr