www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Pop Quiz what is wrong with this code?

reply Danni Coy <danni.coy gmail.com> writes:
...
foreach (y, ref row; offsetMap)
{
   auto run = 0;
    auto nPixels = 0;
    foreach (x; 0..size.x)
    {
         immutable SDL_FPair offset  = { center.x - x, y - center.y };
...

and why does it violate the principle of making the right thing the
easy thing to do?
Jun 20
parent reply rikki cattermole <rikki cattermole.co.nz> writes:
On 21/06/2020 3:08 AM, Danni Coy wrote:
 ...
 foreach (y, ref row; offsetMap)
 {
     auto run = 0;
      auto nPixels = 0;
      foreach (x; 0..size.x)
      {
           immutable SDL_FPair offset  = { center.x - x, y - center.y };
 ...
 
 and why does it violate the principle of making the right thing the
 easy thing to do?
 
oo oo, is it that you didn't want x and y to be of type size_t? And why are you initializing run and nPixels manually?
Jun 20
parent reply Danni Coy <danni.coy gmail.com> writes:
On Sun, Jun 21, 2020 at 1:15 AM rikki cattermole via Digitalmars-d
<digitalmars-d puremagic.com> wrote:
 On 21/06/2020 3:08 AM, Danni Coy wrote:
 ...
 foreach (y, ref row; offsetMap)
 {
     auto run = 0;
      auto nPixels = 0;
      foreach (x; 0..size.x)
      {
           immutable SDL_FPair offset  = { center.x - x, y - center.y };
 ...

 and why does it violate the principle of making the right thing the
 easy thing to do?
oo oo, is it that you didn't want x and y to be of type size_t?
more specifically I didn't want them to be unsigned I am adapting the code from C++ code, coming from other Algol syntax languages. That one is a real gotcha. I tried explicitly making x and y ints and I got a depreciation warning.
 And why are you initializing run and nPixels manually?
the value there so the compiler can figure out the type (auto)
Jun 20
next sibling parent reply rikki cattermole <rikki cattermole.co.nz> writes:
On 21/06/2020 4:00 AM, Danni Coy wrote:
 On Sun, Jun 21, 2020 at 1:15 AM rikki cattermole via Digitalmars-d
 <digitalmars-d puremagic.com> wrote:
 On 21/06/2020 3:08 AM, Danni Coy wrote:
 ...
 foreach (y, ref row; offsetMap)
 {
      auto run = 0;
       auto nPixels = 0;
       foreach (x; 0..size.x)
       {
            immutable SDL_FPair offset  = { center.x - x, y - center.y };
 ...

 and why does it violate the principle of making the right thing the
 easy thing to do?
oo oo, is it that you didn't want x and y to be of type size_t?
more specifically I didn't want them to be unsigned I am adapting the code from C++ code, coming from other Algol syntax languages. That one is a real gotcha. I tried explicitly making x and y ints and I got a depreciation warning.
You are performing an operation on memory, size_t is indeed what you wanted. It is an offset into the address space past another offset (the pointer). You cannot fit the entire address space into an int, but a size_t value is guaranteed to be able to do this. Hence size_t is the correct data type for indexes over a slice.
 And why are you initializing run and nPixels manually?
the value there so the compiler can figure out the type (auto)
You don't need to do this. All D types are default initialized to a value. int in this case will default initialize to 0, which is what you are doing explicitly.
Jun 20
parent Danni Coy <danni.coy gmail.com> writes:
On Sun, Jun 21, 2020 at 2:25 AM rikki cattermole via Digitalmars-d
<digitalmars-d puremagic.com> wrote:
 On 21/06/2020 4:00 AM, Danni Coy wrote:
 On Sun, Jun 21, 2020 at 1:15 AM rikki cattermole via Digitalmars-d
 <digitalmars-d puremagic.com> wrote:
 On 21/06/2020 3:08 AM, Danni Coy wrote:
 ...
 foreach (y, ref row; offsetMap)
 {
      auto run = 0;
       auto nPixels = 0;
       foreach (x; 0..size.x)
       {
            immutable SDL_FPair offset  = { center.x - x, y - center.y };
 ...

 and why does it violate the principle of making the right thing the
 easy thing to do?
oo oo, is it that you didn't want x and y to be of type size_t?
more specifically I didn't want them to be unsigned I am adapting the code from C++ code, coming from other Algol syntax languages. That one is a real gotcha. I tried explicitly making x and y ints and I got a depreciation warning.
You are performing an operation on memory, size_t is indeed what you wanted. It is an offset into the address space past another offset (the pointer).
I am working with slices rather than with pointers directly. In this case the maximum size of x or y I could possibly imagine being useful is less than 64,000 realistic sizes for what I am doing are in the 10s or 100s. This might not be the part I am most worried about.
 All D types are default initialized to a value.

 int in this case will default initialize to 0, which is what you are
 doing explicitly.
I can do int nPixels; or auto nPixels = 0;
Jun 20
prev sibling parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 6/20/20 12:00 PM, Danni Coy wrote:

 I tried explicitly making x and y ints and I got a depreciation warning.
foreach(ptrdiff_t y, ref row; offsetMap) is that what you wanted? ptrdiff_t is the signed version of size_t. The complaint is not that you are converting from unsigned to signed, but that you are converting from 64-bit to 32-bit. -Steve
Jun 20
parent reply Avrina <avrina12309412342 gmail.com> writes:
On Saturday, 20 June 2020 at 18:15:27 UTC, Steven Schveighoffer 
wrote:
 On 6/20/20 12:00 PM, Danni Coy wrote:

 I tried explicitly making x and y ints and I got a 
 depreciation warning.
foreach(ptrdiff_t y, ref row; offsetMap) is that what you wanted? ptrdiff_t is the signed version of size_t. The complaint is not that you are converting from unsigned to signed, but that you are converting from 64-bit to 32-bit. -Steve
Why isn't that deprecated as well? implicitly converting from ulong to long is an error as much as ulong to uint.
Jun 20
next sibling parent reply Danni Coy <danni.coy gmail.com> writes:
On Sun, Jun 21, 2020 at 9:40 AM Avrina via Digitalmars-d
<digitalmars-d puremagic.com> wrote:
 On Saturday, 20 June 2020 at 18:15:27 UTC, Steven Schveighoffer
 wrote:
 On 6/20/20 12:00 PM, Danni Coy wrote:

 I tried explicitly making x and y ints and I got a
 depreciation warning.
foreach(ptrdiff_t y, ref row; offsetMap) is that what you wanted? ptrdiff_t is the signed version of size_t. The complaint is not that you are converting from unsigned to signed, but that you are converting from 64-bit to 32-bit. -Steve
Why isn't that deprecated as well? implicitly converting from ulong to long is an error as much as ulong to uint.
It turns out this line is the bigger problem. immutable SDL_FPair offset = { center.x - x, y - center.y }; FPair is a struct SDL_FPair { float x; float y; ... if those values were ints. dmd will not allow a downcast from ulong to int (the compiler would have caught the issue for me) but it will allow casting ulong to float which is why It took me a couple of hours with a debugger trying to find the bug. So this turns out to be more of a corner case than I thought. offset is a pair of floats because it's about to used in a bunch of angle calculations and I wanted to minimise the number of casts.
Jun 20
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 6/20/20 11:22 PM, Danni Coy wrote:
 On Sun, Jun 21, 2020 at 9:40 AM Avrina via Digitalmars-d
 <digitalmars-d puremagic.com> wrote:
 On Saturday, 20 June 2020 at 18:15:27 UTC, Steven Schveighoffer
 wrote:
 On 6/20/20 12:00 PM, Danni Coy wrote:

 I tried explicitly making x and y ints and I got a
 depreciation warning.
foreach(ptrdiff_t y, ref row; offsetMap) is that what you wanted? ptrdiff_t is the signed version of size_t. The complaint is not that you are converting from unsigned to signed, but that you are converting from 64-bit to 32-bit.
 Why isn't that deprecated as well? implicitly converting from
 ulong to long is an error as much as ulong to uint.
It turns out this line is the bigger problem. immutable SDL_FPair offset = { center.x - x, y - center.y }; FPair is a struct SDL_FPair { float x; float y; ... if those values were ints. dmd will not allow a downcast from ulong to int (the compiler would have caught the issue for me) but it will allow casting ulong to float which is why It took me a couple of hours with a debugger trying to find the bug.
So this entire conversation is incorrect -- there is no promotion to unsigned happening. What is the bug with size_t implicitly casting to float? I would think it would work. -Steve
Jun 22
parent reply WebFreak001 <d.forum webfreak.org> writes:
On Monday, 22 June 2020 at 12:42:26 UTC, Steven Schveighoffer 
wrote:
 On 6/20/20 11:22 PM, Danni Coy wrote:
 On Sun, Jun 21, 2020 at 9:40 AM Avrina via Digitalmars-d
 <digitalmars-d puremagic.com> wrote:
 On Saturday, 20 June 2020 at 18:15:27 UTC, Steven 
 Schveighoffer
 wrote:
 On 6/20/20 12:00 PM, Danni Coy wrote:

 I tried explicitly making x and y ints and I got a
 depreciation warning.
foreach(ptrdiff_t y, ref row; offsetMap) is that what you wanted? ptrdiff_t is the signed version of size_t. The complaint is not that you are converting from unsigned to signed, but that you are converting from 64-bit to 32-bit.
 Why isn't that deprecated as well? implicitly converting from
 ulong to long is an error as much as ulong to uint.
It turns out this line is the bigger problem. immutable SDL_FPair offset = { center.x - x, y - center.y }; FPair is a struct SDL_FPair { float x; float y; ... if those values were ints. dmd will not allow a downcast from ulong to int (the compiler would have caught the issue for me) but it will allow casting ulong to float which is why It took me a couple of hours with a debugger trying to find the bug.
So this entire conversation is incorrect -- there is no promotion to unsigned happening. What is the bug with size_t implicitly casting to float? I would think it would work. -Steve
I think the issue is that it's converting the ulongs to float, which are actually negative and not positive: center.x - x and y - center.y might become negative and because it's a ulong it will be a huge number, then converting to float it will be a huge float number instead of some normal negative number. so: size_t centerX = 1; size_t x = 3; float f = centerX - x; // <- this is not -2, this is 1.84467e+19 without any warnings or deprecations printed Meanwhile an implicit cast to int is disallowed (error) but it would actually contain the correct result and nobody would have ever found it.
Jun 22
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 6/22/20 9:02 AM, WebFreak001 wrote:
 On Monday, 22 June 2020 at 12:42:26 UTC, Steven Schveighoffer wrote:
 On 6/20/20 11:22 PM, Danni Coy wrote:
 On Sun, Jun 21, 2020 at 9:40 AM Avrina via Digitalmars-d
 <digitalmars-d puremagic.com> wrote:
 On Saturday, 20 June 2020 at 18:15:27 UTC, Steven Schveighoffer
 wrote:
 On 6/20/20 12:00 PM, Danni Coy wrote:

 I tried explicitly making x and y ints and I got a
 depreciation warning.
foreach(ptrdiff_t y, ref row; offsetMap) is that what you wanted? ptrdiff_t is the signed version of size_t. The complaint is not that you are converting from unsigned to signed, but that you are converting from 64-bit to 32-bit.
 Why isn't that deprecated as well? implicitly converting from
 ulong to long is an error as much as ulong to uint.
It turns out this line is the bigger problem.   immutable SDL_FPair offset  = { center.x - x, y - center.y }; FPair is a struct SDL_FPair {      float x;      float y; ... if those values were ints. dmd will not allow a downcast from ulong to int (the compiler would have caught the issue for me) but it will allow casting ulong to float which is why It took me a couple of hours with a debugger trying to find the bug.
So this entire conversation is incorrect -- there is no promotion to unsigned happening. What is the bug with size_t implicitly casting to float? I would think it would work.
I think the issue is that it's converting the ulongs to float, which are actually negative and not positive: center.x - x and y - center.y might become negative and because it's a ulong it will be a huge number, then converting to float it will be a huge float number instead of some normal negative number. so: size_t centerX = 1; size_t x = 3; float f = centerX - x; // <- this is not -2, this is 1.84467e+19 without any warnings or deprecations printed Meanwhile an implicit cast to int is disallowed (error) but it would actually contain the correct result and nobody would have ever found it.
But the OP rejected the idea of using ptrdiff_t (or did he?), which would solve that problem. It's really hard to tell what the problem is without more definition or explanation. -Steve
Jun 22
parent reply Kagamin <spam here.lot> writes:
On Monday, 22 June 2020 at 13:24:32 UTC, Steven Schveighoffer 
wrote:
 But the OP rejected the idea of using ptrdiff_t (or did he?), 
 which would solve that problem.

 It's really hard to tell what the problem is without more 
 definition or explanation.
OP is precise: On Saturday, 20 June 2020 at 16:00:40 UTC, Danni Coy wrote:
 more specifically I didn't want them to be unsigned
The discussion was just derailed.
Jun 22
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 6/22/20 2:19 PM, Kagamin wrote:
 On Monday, 22 June 2020 at 13:24:32 UTC, Steven Schveighoffer wrote:
 But the OP rejected the idea of using ptrdiff_t (or did he?), which 
 would solve that problem.

 It's really hard to tell what the problem is without more definition 
 or explanation.
OP is precise: On Saturday, 20 June 2020 at 16:00:40 UTC, Danni Coy wrote:
 more specifically I didn't want them to be unsigned
The discussion was just derailed.
That is exactly what declaring the variables to be ptrdiff_t would do. I guess he didn't specifically reject that answer, but proceeded to say something else is the "bigger problem". If using ptrdiff_t doesn't work, then I would have to have more information about why it doesn't work. -Steve
Jun 22
parent reply Kagamin <spam here.lot> writes:
On Monday, 22 June 2020 at 18:37:41 UTC, Steven Schveighoffer 
wrote:
 I guess he didn't specifically reject that answer, but 
 proceeded to say something else is the "bigger problem".
The problem is that nonnumbers are converted to float, but they were intended to be compatible only with signed integers, such conversion doesn't change the bit pattern and most of the time the last bit is useless anyway.
Jun 27
parent Kagamin <spam here.lot> writes:
And processor doesn't even support conversion between unsigned 
integers and floating point numbers.
Jun 27
prev sibling next sibling parent reply Patrick Schluter <Patrick.Schluter bbox.fr> writes:
On Saturday, 20 June 2020 at 23:36:25 UTC, Avrina wrote:
 On Saturday, 20 June 2020 at 18:15:27 UTC, Steven Schveighoffer 
 wrote:
 On 6/20/20 12:00 PM, Danni Coy wrote:

 I tried explicitly making x and y ints and I got a 
 depreciation warning.
foreach(ptrdiff_t y, ref row; offsetMap) is that what you wanted? ptrdiff_t is the signed version of size_t. The complaint is not that you are converting from unsigned to signed, but that you are converting from 64-bit to 32-bit. -Steve
Why isn't that deprecated as well? implicitly converting from ulong to long is an error as much as ulong to uint.
No. conversion of ulong to uint loses 32 times more data than conversion to long does.
Jun 21
parent reply Avrina <avrina12309412342 gmail.com> writes:
On Sunday, 21 June 2020 at 10:12:22 UTC, Patrick Schluter wrote:
 On Saturday, 20 June 2020 at 23:36:25 UTC, Avrina wrote:
 On Saturday, 20 June 2020 at 18:15:27 UTC, Steven 
 Schveighoffer wrote:
 On 6/20/20 12:00 PM, Danni Coy wrote:

 I tried explicitly making x and y ints and I got a 
 depreciation warning.
foreach(ptrdiff_t y, ref row; offsetMap) is that what you wanted? ptrdiff_t is the signed version of size_t. The complaint is not that you are converting from unsigned to signed, but that you are converting from 64-bit to 32-bit. -Steve
Why isn't that deprecated as well? implicitly converting from ulong to long is an error as much as ulong to uint.
No. conversion of ulong to uint loses 32 times more data than conversion to long does.
Lol, it's not linear friend. You lose around 18446744069414584319 values from ulong to uint, you lose 9223372032559808512 from ulong to long. Which is about 2 times less data. Which makes sense as it is basically 2^64 - 2^63, so of course it would only be x2 -- not x32.
Jun 21
next sibling parent Avrina <avrina12309412342 gmail.com> writes:
On Sunday, 21 June 2020 at 13:17:54 UTC, Avrina wrote:
 On Sunday, 21 June 2020 at 10:12:22 UTC, Patrick Schluter wrote:
 On Saturday, 20 June 2020 at 23:36:25 UTC, Avrina wrote:
 On Saturday, 20 June 2020 at 18:15:27 UTC, Steven 
 Schveighoffer wrote:
 On 6/20/20 12:00 PM, Danni Coy wrote:

 I tried explicitly making x and y ints and I got a 
 depreciation warning.
foreach(ptrdiff_t y, ref row; offsetMap) is that what you wanted? ptrdiff_t is the signed version of size_t. The complaint is not that you are converting from unsigned to signed, but that you are converting from 64-bit to 32-bit. -Steve
Why isn't that deprecated as well? implicitly converting from ulong to long is an error as much as ulong to uint.
No. conversion of ulong to uint loses 32 times more data than conversion to long does.
Lol, it's not linear friend. You lose around 18446744069414584319 values from ulong to uint, you lose 9223372032559808512 from ulong to long. Which is about 2 times less data. Which makes sense as it is basically 2^64 - 2^63, so of course it would only be x2 -- not x32.
Not that it should matter how much, it's a problem either way.
Jun 21
prev sibling parent reply Patrick Schluter <Patrick.Schluter bbox.fr> writes:
On Sunday, 21 June 2020 at 13:17:54 UTC, Avrina wrote:
 On Sunday, 21 June 2020 at 10:12:22 UTC, Patrick Schluter wrote:
 On Saturday, 20 June 2020 at 23:36:25 UTC, Avrina wrote:
 On Saturday, 20 June 2020 at 18:15:27 UTC, Steven 
 Schveighoffer wrote:
 On 6/20/20 12:00 PM, Danni Coy wrote:

 I tried explicitly making x and y ints and I got a 
 depreciation warning.
foreach(ptrdiff_t y, ref row; offsetMap) is that what you wanted? ptrdiff_t is the signed version of size_t. The complaint is not that you are converting from unsigned to signed, but that you are converting from 64-bit to 32-bit. -Steve
Why isn't that deprecated as well? implicitly converting from ulong to long is an error as much as ulong to uint.
No. conversion of ulong to uint loses 32 times more data than conversion to long does.
Lol, it's not linear friend.
losing 1 bit vs losing 32 bits. The exponentiation is irrelevant.
 You lose around 18446744069414584319 values from ulong to uint, 
 you lose 9223372032559808512 from ulong to long. Which is about 
 2 times less data. Which makes sense as it is basically 2^64 - 
 2^63, so of course it would only be x2 -- not x32.
 Not that it should matter how much, it's a problem either way.
But a much less severe one. Overflowing a 32 bit index on a 64 bit machine with the data sizes that exist today (files of more than 4GiB are not rare, zips, videos and databases). 9 223 372 032 559 808 512 is still 32768x bigger than 281 474 976 710 656, the biggest addressable range on x86_64.
Jun 21
parent Avrina <avrina12309412342 gmail.com> writes:
On Sunday, 21 June 2020 at 19:33:47 UTC, Patrick Schluter wrote:
 On Sunday, 21 June 2020 at 13:17:54 UTC, Avrina wrote:
 On Sunday, 21 June 2020 at 10:12:22 UTC, Patrick Schluter 
 wrote:
 On Saturday, 20 June 2020 at 23:36:25 UTC, Avrina wrote:
 On Saturday, 20 June 2020 at 18:15:27 UTC, Steven 
 Schveighoffer wrote:
 On 6/20/20 12:00 PM, Danni Coy wrote:

 I tried explicitly making x and y ints and I got a 
 depreciation warning.
foreach(ptrdiff_t y, ref row; offsetMap) is that what you wanted? ptrdiff_t is the signed version of size_t. The complaint is not that you are converting from unsigned to signed, but that you are converting from 64-bit to 32-bit. -Steve
Why isn't that deprecated as well? implicitly converting from ulong to long is an error as much as ulong to uint.
No. conversion of ulong to uint loses 32 times more data than conversion to long does.
Lol, it's not linear friend.
losing 1 bit vs losing 32 bits. The exponentiation is irrelevant.
It is when that 1 bit is worth 9223372032559808512
 You lose around 18446744069414584319 values from ulong to 
 uint, you lose 9223372032559808512 from ulong to long. Which 
 is about 2 times less data. Which makes sense as it is 
 basically 2^64 - 2^63, so of course it would only be x2 -- not 
 x32.
 Not that it should matter how much, it's a problem either way.
But a much less severe one. Overflowing a 32 bit index on a 64 bit machine with the data sizes that exist today (files of more than 4GiB are not rare, zips, videos and databases). 9 223 372 032 559 808 512 is still 32768x bigger than 281 474 976 710 656, the biggest addressable range on x86_64.
That's an index to an array, not a file. The File api uses ulong (as it should) and doesn't use size_t. If you are using memory mapped files, that isn't portable and you are going to have issues with performance. Otherwise a 32-bit index is sufficient, you aren't ever going to have an array that big that uses up that much memory in the system. So again, doesn't matter. There's a reason no one has complained about this "bug" for 20 years, and the only reason it was "discovered" was because it was inconsistent with foreach_reverse.
Jun 21
prev sibling parent Steven Schveighoffer <schveiguy gmail.com> writes:
On 6/20/20 7:36 PM, Avrina wrote:
 On Saturday, 20 June 2020 at 18:15:27 UTC, Steven Schveighoffer wrote:
 On 6/20/20 12:00 PM, Danni Coy wrote:

 I tried explicitly making x and y ints and I got a depreciation warning.
foreach(ptrdiff_t y, ref row; offsetMap) is that what you wanted? ptrdiff_t is the signed version of size_t. The complaint is not that you are converting from unsigned to signed, but that you are converting from 64-bit to 32-bit.
Why isn't that deprecated as well? implicitly converting from ulong to long is an error as much as ulong to uint.
It's not an error as you don't lose information. In this case, you are losing nothing, as there are no systems with exabytes of RAM (i.e. an array size will never be big enough to overflow a long). -Steve
Jun 22