On Tuesday, 31 July 2012 at 16:16:00 UTC, Era Scarecrow wrote:
> On Tuesday, 31 July 2012 at 15:25:55 UTC, Andrej Mitrovic wrote:
>> On 7/31/12, monarch_dodra <monarchdodra@gmail.com> wrote:
>>> The bug is only when the field is EXACTLY 32 bits BTW. bitfields works quite nice with 33 or whatever. More details in the report.
>>
>> Yeah 32 or 64 bits, thanks for changing the title.
>
>  I wonder, is it really a bug? If you are going to have it fill a whole size it would fit anyways, why even put it in as a bitfield? You could just declare it separately.
>
>  I get the feeling it's not so much a bug as a design feature. If you really needed a full size not aligned with whole bytes (or padded appropriately) then I could understand, but still...
>
>
>  And I'm the one that changed the title name. Suddenly I'm reminded of south park (movie) and the buttfor.

No, it's a bug. There is no reason for it to fail (and it certainly isn't a feature).

Maybe the user wants to pack an "uint, ushort, ubyte, ubyte" together in a struct, but doesn't want the rest of that struct's members 1-aligned?

Maybe the user needs a 32 bit ulong? This way the ulong only takes 32 bits, but can still be implicitly passed to functions expecting ulongs.

Maybe the user generated the mixin using another template? For example, testing integers from 10 bits wide to 40 bits wide? Should he write a special case for 32?

...

Now I'm not saying it is big bug or anything, but it is something that should be supported... Or at least explicitly asserted until fixed...

On Tuesday, 31 July 2012 at 16:59:11 UTC, monarch_dodra wrote:
> No, it's a bug. There is no reason for it to fail (and it certainly isn't a feature).

 If I made two fields in a 64bit bitfield, each 32bits int's I'd like it to complain; If it's calculated from something else then finding the problem may be a little more difficult. But that's how my mind works, give you the tools you need to do whatever you want including shooting yourself in the foot (although you need to work harder to do that than C++).

> Maybe the user wants to pack an "uint, ushort, ubyte, ubyte" together in a struct, but doesn't want the rest of that struct's members 1-aligned?

 That would be the one reason that makes sense; Course can't you align sections at 1 byte and then by the default afterwards? Course now that I think about it, some systems don't have options to do byte alignments and require to access memory at 4/8 byte alignments. This makes sense needing to support it.

> Maybe the user needs a 32 bit ulong? This way the ulong only takes 32 bits, but can still be implicitly passed to functions expecting ulongs.

 I would think the bug only showed itself if you did int at 32bits and ulong at 64 bits, not ulong at 32bits.

> Maybe the user generated the mixin using another template? For example, testing integers from 10 bits wide to 40 bits wide? Should he write a special case for 32?

 Ummm..... Yeah I think I see what your saying; And you're probably right a special case would be less easily documented (and more a pain) when it shouldn't need a special case.

> Now I'm not saying it is big bug or anything, but it is something that should be supported... Or at least explicitly asserted until fixed...

On 07/31/2012 06:57 PM, Era Scarecrow wrote:
> On Tuesday, 31 July 2012 at 16:48:37 UTC, Andrej Mitrovic wrote:
>> On 7/31/12, Era Scarecrow <rtcvb32@yahoo.com> wrote:
>>> I wonder, is it really a bug? If you are going to have it fill a
>>> whole size it would fit anyways, why even put it in as a bitfield?
>>> You could just declare it separately.
>>
>> I don't really know, I'm looking at this from a point of wrapping C++.
>> I haven't used bitfields myself in my own code.
>
> I'd say it's not a bug since C/C++ is free to reorder the fields you'd
> need to tinker with it anyways; HOWEVER if you still need to be able to
> have it then who's to stop you from doing it?
>
> I think more likely a flag/version or some indicator that you didn't
> make a mistake, such as making them depreciated so it complains to you.
> Kinda like how you can't make assignments in if statements or do useless
> compares, it's an error and helps prevent issues that are quite
> obviously mistakes.

This is obviously a mistake in the bitfield implementation. What else
could be concluded from the error message:

std\bitmanip.d(76): Error: shift by 32 is outside the range 0..31

Requesting a 32 bit or 64 bit member on the other hand is not a
mistake, and it is not useless, therefore the analogy breaks down.

(Also IMO, the once-in-a-year wtf that is caused by accidentally
assigning in an if condition does not justify special casing assignment
expressions inside if conditions. Also, what is an useless compare?)

On Tuesday, 31 July 2012 at 17:17:43 UTC, Timon Gehr wrote:

> (Also IMO, the once-in-a-year wtf that is caused by accidentally assigning in an if condition does not justify special casing assignment expressions inside if conditions. Also, what is an useless compare?)

 I've noticed in my experience, DMD gives you an error if you do a statement that has no effect; IE:

 1 + 2; //statement has no effect
 a == b;//ditto

On Tuesday, 31 July 2012 at 17:17:25 UTC, Era Scarecrow wrote:
> On Tuesday, 31 July 2012 at 16:59:11 UTC, monarch_dodra wrote:
>> Maybe the user needs a 32 bit ulong? This way the ulong only takes 32 bits, but can still be implicitly passed to functions expecting ulongs.
>
>  I would think the bug only showed itself if you did int at 32bits and ulong at 64 bits, not ulong at 32bits.

No, the bug shows itself if the first field is 32 bits, regardless of (ulong included).

I would add though that requesting a field in bits that is bigger than the type of the field should not work (IMO). EG:
--------
struct A
{
    mixin(bitfields!(
          ushort, "a", 24,
          uint,    "",  8
        )
    );
}
--------
I don't see any way how that could make sense...
But it *is* legal in C and C++...
But it does generates warnings...

I think it should static assert in D.

On Tuesday, 31 July 2012 at 17:21:00 UTC, Era Scarecrow wrote:
> On Tuesday, 31 July 2012 at 17:17:43 UTC, Timon Gehr wrote:
>
>> (Also IMO, the once-in-a-year wtf that is caused by accidentally assigning in an if condition does not justify special casing assignment expressions inside if conditions. Also, what is an useless compare?)
>
>  I've noticed in my experience, DMD gives you an error if you do a statement that has no effect; IE:
>
>  1 + 2; //statement has no effect
>  a == b;//ditto

That's part of the standard: Statements that have no effect are illegal. This is a good think, IMO. I've seen MANY C++ bugs that could have been saved by that.

Regarding the assignment in if. I think it is a good thing. D sides with safety. If you *really* want to test assign, you can always comma operator it.

--------
void main()
{
  int a = 0, b = 5;
  while(a = --b, a) //YES, I *DO* want to assign!
  {
    write(a);
  }
};
--------I'm sure the compiler will optimize away what it needs.

On Tuesday, 31 July 2012 at 17:34:33 UTC, monarch_dodra wrote:
> No, the bug shows itself if the first field is 32 bits, regardless of (ulong included).
>
> I would add though that requesting a field in bits that is bigger than the type of the field should not work (IMO). EG:
> --------
> struct A
> {
>     mixin(bitfields!(
>           ushort, "a", 24,
>           uint,    "",  8
>         )
>     );
> }
> --------
> I don't see any way how that could make sense...
> But it *is* legal in C and C++...
> But it does generates warnings...

 Maybe so ushort has extra padding for expansion at some later date when they change it to uint?? could put an assert in, but if it doesn't break code...

> I think it should static assert in D.

 Glancing over the issue, the [0..31] is a compiler error based on bit shifting (not bitfields itself); if the storage type is ulong then it shouldn't matter if the first one is a 32bit size or not. Unless... Nah, couldn't be... I'll look it over later to be sure.

> That's part of the standard: Statements that have no effect are illegal. This is a good think, IMO. I've seen MANY C++ bugs that could have been saved by that.
>
> Regarding the assignment in if. I think it is a good thing. D sides with safety. If you *really* want to test assign, you can always comma operator it.

On Tuesday, 31 July 2012 at 17:17:43 UTC, Timon Gehr wrote:

> This is obviously a mistake in the bitfield implementation. What else could be concluded from the error message:
>
> std\bitmanip.d(76): Error: shift by 32 is outside the range 0..31
>
> Requesting a 32 bit or 64 bit member on the other hand is not a mistake, and it is not useless, therefore the analogy breaks down.

 Well curiously it was easier to fix than I thought (a line for a static if, and a modification of the masking)... Was there any other bugs that come to mind? Anything of consequence?

On Tuesday, 31 July 2012 at 17:17:43 UTC, Timon Gehr wrote:

> This is obviously a mistake in the bitfield implementation. What else could be concluded from the error message:
>
> std\bitmanip.d(76): Error: shift by 32 is outside the range 0..31
>
> Requesting a 32 bit or 64 bit member on the other hand is not a mistake, and it is not useless, therefore the analogy breaks down.

 Well curiously it was easier to fix than I thought (a line for a static if, and a modification of the masking)... Was there any other bugs that come to mind? Anything of consequence?

On 07/31/2012 09:15 AM, Era Scarecrow wrote:
> On Tuesday, 31 July 2012 at 15:25:55 UTC, Andrej Mitrovic wrote:
>> On 7/31/12, monarch_dodra <monarchdodra@gmail.com> wrote:
>>> The bug is only when the field is EXACTLY 32 bits BTW. bitfields
>>> works quite nice with 33 or whatever. More details in the report.
>>
>> Yeah 32 or 64 bits, thanks for changing the title.
>
> I wonder, is it really a bug? If you are going to have it fill a whole
> size it would fit anyways, why even put it in as a bitfield? You could
> just declare it separately.

It can happen in templated code where the width of the first field may be a template parameter. I wouldn't want to 'static if (width == 32)'.

But thanks for fixing the bug already! :)

Ali

On 31-Jul-12 22:21, Era Scarecrow wrote:
> On Tuesday, 31 July 2012 at 17:17:43 UTC, Timon Gehr wrote:
>
>> This is obviously a mistake in the bitfield implementation. What else
>> could be concluded from the error message:
>>
>> std\bitmanip.d(76): Error: shift by 32 is outside the range 0..31
>>
>> Requesting a 32 bit or 64 bit member on the other hand is not a
>> mistake, and it is not useless, therefore the analogy breaks down.
>
>   Well curiously it was easier to fix than I thought (a line for a
> static if, and a modification of the masking)... Was there any other
> bugs that come to mind? Anything of consequence?

Great to see things moving. Could you please do a separate pull for bitfields it should get merged easier and it seems like a small but important bugfix.

-- 
Dmitry Olshansky