BitFields:
https://github.com/rtcvb32/phobos/commit/729c96e2f20ddd0c05a1afb488030b5c4db3e012

new BitArray Overhead functions/templates:
https://github.com/rtcvb32/phobos/commit/55aaf82a39f3901f03f5f1ac26fd175c5b2cd167

BitArray:
https://github.com/rtcvb32/phobos/commit/568c0d1966208afe70cedcaddc3bb880dc3d481d

 I'd like opinions of the code I've submitted, and certain shortcomings that are present that appropriate changes would be needed to fix. Only one of these has been a problem during debugging and that's due to references. (All three patches must be present to work)

BitArray:
1) Slices use references (sometimes) to previous bitarray. Since it's a slice (like an array) this could be expected.
Solutions:
 a) Drop the union, make all functions @safe (compared to @trusted), and they are all ref/slices
 b) Leave as sometimes ref, sometimes not; Use .dup when you NEED to ensure different unique copies.
 c) Apply COW (Copy-On-Write) where a known slice type (canExpand is false) when a change would apply, it replaces the present slice into it's own unique array.
 d) All slices become new dup'ed allocated arrays (Most expensive of them all)

2) opCast disabled: Anyone using opCast their code will break, however depending on it's usage that may not be an issue. If anyone can recommend alternative opCast(s) that won't prevent normal use of casting, then they will be reimplemented.

3) dim - An artifact of the previous BitArray, and I think needs to be removed as it has limited use.

4) Certain operators (opCat, opCat_r) I can't replace with opBinary/opBinary_r.

5) Examples of additional use cases/construction that it currently fails and should work.

6) Added rawWrite (for primitives), usefulness testing and opinions.

7) opApply, toHash, opCmp, opEqual tested in various environments (Like associative arrays) for problems, along with any possible const issues.

BitFields:
 I've concentrated on adding default values. A set of functions previously present string functions 'myXXX' were present, why were they present to begin with? Was it a limitation of what was available at the time? Or was it to make compiling faster so it didn't rely on other library functions? Or were the string functions not available at the time? They might be removed once this is truly answered.

//currently should work
struct A
{
  mixin(bitfields!(
    uint, "x",    2,
    int,  "y",    3,
    uint, "z=1",  2,
    bool, "flag=true", 1));
}
A obj;
assert(obj.x == 0);
assert(obj.z == 1);    //default
assert(obj.flag == 1); //true/false should also work

On Saturday, July 28, 2012 22:41:48 Era Scarecrow wrote:
> 2) opCast disabled: Anyone using opCast their code will break, however depending on it's usage that may not be an issue. If anyone can recommend alternative opCast(s) that won't prevent normal use of casting, then they will be reimplemented.

I'm not particularly familar with BitArray, so I don't know what its opCast would convert to, but opCast will only convert to the types that you let it, so if you need to constrain what types it works with, then use template constraints to do that. If, on the other hand, opCast just doesn't make any sense at all, then just don't have it.

- Jonathan M Davis

On 29-Jul-12 00:41, Era Scarecrow wrote:
>
> BitFields:
> https://github.com/rtcvb32/phobos/commit/729c96e2f20ddd0c05a1afb488030b5c4db3e012
>
>
> new BitArray Overhead functions/templates:
> https://github.com/rtcvb32/phobos/commit/55aaf82a39f3901f03f5f1ac26fd175c5b2cd167
>
>
> BitArray:
> https://github.com/rtcvb32/phobos/commit/568c0d1966208afe70cedcaddc3bb880dc3d481d
>
>
>   I'd like opinions of the code I've submitted, and certain shortcomings
> that are present that appropriate changes would be needed to fix. Only
> one of these has been a problem during debugging and that's due to
> references. (All three patches must be present to work)
>

Great! But I strongly suggest to repost it in d.D newsgroup as it has
wider audience and is more appropriate for reviews.

> BitArray:
> 1) Slices use references (sometimes) to previous bitarray. Since it's a
> slice (like an array) this could be expected.
> Solutions:
>   a) Drop the union, make all functions @safe (compared to @trusted),
> and they are all ref/slices
>   b) Leave as sometimes ref, sometimes not; Use .dup when you NEED to
> ensure different unique copies.
>   c) Apply COW (Copy-On-Write) where a known slice type (canExpand is
> false) when a change would apply, it replaces the present slice into
> it's own unique array.
>   d) All slices become new dup'ed allocated arrays (Most expensive of
> them all)

My solution is make slices different type e.g. BitSlice.
It's always reference to the original BitArray. Array itself still can be either compact or not. All operations with slice go to array (and still check if they can use word-aligned speed up).

How does it sound?

>
> 2) opCast disabled: Anyone using opCast their code will break, however
> depending on it's usage that may not be an issue. If anyone can
> recommend alternative opCast(s) that won't prevent normal use of
> casting, then they will be reimplemented.
>
> 3) dim - An artifact of the previous BitArray, and I think needs to be
> removed as it has limited use.
>
> 4) Certain operators (opCat, opCat_r) I can't replace with
> opBinary/opBinary_r.
>

opCat isn't it just operator "~" and "~=" ? You can use opOpAssign for "~=" IRC.

> 5) Examples of additional use cases/construction that it currently fails
> and should work.
>
> 6) Added rawWrite (for primitives), usefulness testing and opinions.
>
> 7) opApply, toHash, opCmp, opEqual tested in various environments (Like
> associative arrays) for problems, along with any possible const issues.
>
> BitFields:
>   I've concentrated on adding default values. A set of functions
> previously present string functions 'myXXX' were present, why were they
> present to begin with?

I suspect to!string wasn't CTFEable. Regardless you can catch a word from Andrei Alexandrescu on the main D newsgroup who (I think) came up with it in the first place.


-- 
Dmitry Olshansky

On Sunday, July 29, 2012 00:58:59 Dmitry Olshansky wrote:
> My solution is make slices different type e.g. BitSlice.
> It's always reference to the original BitArray. Array itself still can
> be either compact or not. All operations with slice go to array (and
> still check if they can use word-aligned speed up).
> 
> How does it sound?

I would point out that while hasSlicing doesn't currently check for it, if opSlice doesn't return a type which is assignable to the original range, it won't work in a lot of Phobos functions. I keep meaning to bring that up for discussion in the main newsgroup. I'd argue that hasSlicing really be changed from

    enum bool hasSlicing = !isNarrowString!R && is(typeof(
    (inout int _dummy=0)
    {
        R r = void;
        auto s = r[1 .. 2];
        static assert(isInputRange!(typeof(s)));
    }));

to something like

    enum bool hasSlicing = !isNarrowString!R && is(typeof(
    (inout int _dummy=0)
    {
        R r = void;
        auto s = r[1 .. 2];
        static assert(isInputRange!(typeof(s)));
        r = s;
    }));

- Jonathan M Davis

On 29-Jul-12 01:07, Jonathan M Davis wrote:
> On Sunday, July 29, 2012 00:58:59 Dmitry Olshansky wrote:
>> My solution is make slices different type e.g. BitSlice.
>> It's always reference to the original BitArray. Array itself still can
>> be either compact or not. All operations with slice go to array (and
>> still check if they can use word-aligned speed up).
>>
>> How does it sound?
>
> I would point out that while hasSlicing doesn't currently check for it, if
> opSlice doesn't return a type which is assignable to the original range, it
> won't work in a lot of Phobos functions. I keep meaning to bring that up for
> discussion in the main newsgroup.

BitArray isn't and shouldn't be a range. It's a container.
So BitSlice is a range over BitArray. What am I missing?



-- 
Dmitry Olshansky

On Saturday, 28 July 2012 at 20:59:15 UTC, Dmitry Olshansky wrote:
> Great! But I strongly suggest to repost it in d.D newsgroup as it has wider audience and is more appropriate for reviews.

 I was thinking of not bothering Andrei or Walter while i fixed other issues on it before bringing more major ones forward (which is why i brought it to D.learn), but re-posting it there is easy enough

> My solution is make slices different type e.g. BitSlice. It's always reference to the original BitArray. Array itself still can be either compact or not. All operations with slice go to array (and still check if they can use word-aligned speed up).
>
> How does it sound?

 Someone else also suggested making the slice different (Meaning a new Bitarray always is at max compact size). Since we need at least a single flag to determine if the array is compact or array reference, there would be a larger loss which the current offset/maxOffset handle quite nicely filling in that space. Making BitSlice separate suggests BitArray would be the bare minimum (canUseBulk, length, and index operations) while bitSlice would handle all others. But if you do that, either BitSlice would have to be inherently convertible to BitArray if you want to pass it to a function that expects a BitArray. I'm hesitant to do it, but if there's a strong enough argument for it I wouldn't refuse it.

> opCat isn't it just operator "~" and "~=" ? You can use opOpAssign for "~=" IRC.

 Yes, I think opAssign is used for ~= already, but doing the following requires opCat and opCat_r.

 BitArray a; //filled with something
 auto x = true ~ a;
 auto y = a ~ true;

 I just can't get it to accept it (for whatever reason). Compiler bug?

> I suspect to!string wasn't CTFEable. Regardless you can catch a word from Andrei Alexandrescu on the main D newsgroup who (I think) came up with it in the first place.

 If we can use the std.string functions I'll replace them all :) (As I was just following the theme since they were present).

 I'll move this to the D group and see what we get for this.

On Saturday, 28 July 2012 at 21:07:31 UTC, Jonathan M Davis wrote:
> I would point out that while hasSlicing doesn't currently check for it, if opSlice doesn't return a type which is assignable to the original range, it won't work in a lot of Phobos functions. I keep meaning to bring that up for discussion in the main newsgroup. I'd argue that hasSlicing really be changed

 Which is one of the main reasons I am hesitant to have a separate range struct (of BitSlice)... Hmmm maybe we should keep it here for a little while, and talk it over a bit more.

On Sunday, July 29, 2012 01:19:49 Dmitry Olshansky wrote:
> On 29-Jul-12 01:07, Jonathan M Davis wrote:
> > On Sunday, July 29, 2012 00:58:59 Dmitry Olshansky wrote:
> >> My solution is make slices different type e.g. BitSlice.
> >> It's always reference to the original BitArray. Array itself still can
> >> be either compact or not. All operations with slice go to array (and
> >> still check if they can use word-aligned speed up).
> >> 
> >> How does it sound?
> > 
> > I would point out that while hasSlicing doesn't currently check for it, if
> > opSlice doesn't return a type which is assignable to the original range,
> > it
> > won't work in a lot of Phobos functions. I keep meaning to bring that up
> > for discussion in the main newsgroup.
> 
> BitArray isn't and shouldn't be a range. It's a container. So BitSlice is a range over BitArray. What am I missing?

Probably nothing. I'm not all that familiar with BitArray. If it's not a range, it's not a problem. I assumed that it was and was just pointing out a potential problem based on that assumption.

- Jonathan M Davis

On 29-Jul-12 01:20, Era Scarecrow wrote:
> On Saturday, 28 July 2012 at 20:59:15 UTC, Dmitry Olshansky wrote:
>> Great! But I strongly suggest to repost it in d.D newsgroup as it has
>> wider audience and is more appropriate for reviews.
>
>   I was thinking of not bothering Andrei or Walter while i fixed other
> issues on it before bringing more major ones forward (which is why i
> brought it to D.learn), but re-posting it there is easy enough
>
>> My solution is make slices different type e.g. BitSlice. It's always
>> reference to the original BitArray. Array itself still can be either
>> compact or not. All operations with slice go to array (and still check
>> if they can use word-aligned speed up).
>>
>> How does it sound?
>
>   Someone else also suggested making the slice different (Meaning a new
> Bitarray always is at max compact size).

Obviously that was me (blackwhale) ;)

Since we need at least a single
> flag to determine if the array is compact or array reference, there
> would be a larger loss which the current offset/maxOffset handle quite
> nicely filling in that space.

Not sure I followed you - what with offset/maxOffset ? You mean less space available for compact version?

 Making BitSlice separate suggests BitArray
> would be the bare minimum (canUseBulk, length, and index operations)
> while bitSlice would handle all others. But if you do that, either
> BitSlice would have to be inherently convertible to BitArray if you want
> to pass it to a function that expects a BitArray.

Take a look at std.container from what I see there Array does have a separate range type.

I'm hesitant to do it,
> but if there's a strong enough argument for it I wouldn't refuse it.
>
>> opCat isn't it just operator "~" and "~=" ? You can use opOpAssign for
>> "~=" IRC.
>
>   Yes, I think opAssign is used for ~= already, but doing the following
> requires opCat and opCat_r.
>
>   BitArray a; //filled with something
>   auto x = true ~ a;
>   auto y = a ~ true;
>
>   I just can't get it to accept it (for whatever reason). Compiler bug?
>
Hm opBinaryRight with if (op == "~") should work.

>> I suspect to!string wasn't CTFEable. Regardless you can catch a word
>> from Andrei Alexandrescu on the main D newsgroup who (I think) came up
>> with it in the first place.
>
>   If we can use the std.string functions I'll replace them all :) (As I
> was just following the theme since they were present).
>
>   I'll move this to the D group and see what we get for this.


-- 
Dmitry Olshansky

On Saturday, July 28, 2012 23:23:30 Era Scarecrow wrote:
> On Saturday, 28 July 2012 at 21:07:31 UTC, Jonathan M Davis wrote:
> > I would point out that while hasSlicing doesn't currently check for it, if opSlice doesn't return a type which is assignable to the original range, it won't work in a lot of Phobos functions. I keep meaning to bring that up for discussion in the main newsgroup. I'd argue that hasSlicing really be changed
> 
>   Which is one of the main reasons I am hesitant to have a
> separate range struct (of BitSlice)... Hmmm maybe we should keep
> it here for a little while, and talk it over a bit more.

Well, it seems that my comment was somewhat misplaced in that BitArray isn't a range but a container. My comment was directed at opSlice on ranges. Container types should return whatever range type is appropriate. That will usually be a struct of some variety. I'd have to study BitArray and your changes to determine what the appropriate type was here, but it sounds like you were talking about duping the contents with opSlice, which would be highly irregular. Normally, ranges over containers are light wrappers which can iterate through and potentially alter the elements in the container, and duping up an array as the range type doesn't work like that. But depending on what BitArray is doing exactly, it might be appropriate. I don't know.

- Jonathan M Davis