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 2020
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 2020
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 2020
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 2020
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 2020
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 2020
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 2020
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 2020
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 2020
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 2020
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 2020
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 2020
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 2020
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 2020
parent Kagamin <spam here.lot> writes:
And processor doesn't even support conversion between unsigned 
integers and floating point numbers.
Jun 27 2020
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 2020
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 2020
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 2020
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 2020
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 2020
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 2020