www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Killing integer division problems?

reply burt <invalid_email_address cab.abc> writes:
Hello,

A while ago I spent an hour figuring out why some variable wasn't 
changing like it should, and the reason was something like the 
following (although in a much more complex expression):

```d
double y = 0.0;
// ...
y += 1 / b; // with b : int
// ...
```

I have run into this C remnant multiple times now and spent much 
too long trying to find out what happened, so to fix it, here's 
my proposal:

"Make integer division without cast illegal."

In the case above, the compiler would complain (or at least emit 
a warning) that there is integer division going on and that the 
result will be truncated.

If I want to keep the integer division behaviour, I would just 
have to change the code to:

```d
y += cast(int) (1 / b);
```

If I don't want to keep that behaviour, then the compiler has 
shown me the bug and I can fix it:

```d
y += 1.0 / b;
```

It doesn't silently change behaviour that used to be valid in C 
(it will emit a warning) and can be fixed trivially, so it 
shouldn't be an intrusive change (and might even catch bugs that 
weren't found yet).

Would such a change require a DIP? Could a -preview/-transition 
flag be added to warn me about integer divisions? What do you 
think?
Aug 04 2020
next sibling parent reply jmh530 <john.michael.hall gmail.com> writes:
On Tuesday, 4 August 2020 at 12:34:37 UTC, burt wrote:

 [snip]

 Would such a change require a DIP? Could a -preview/-transition 
 flag be added to warn me about integer divisions? What do you 
 think?


The problem is you have stuff like
```d
int x = 2;
int y = 4 / x;
```
and now you would be force someone to write a cast.

That's a big enough change that it would require a DIP. Though 
I've been hit by the same thing in the past, I would be a bit 
skeptical you could get it through since a lot of code depends on 
that.

You could also always use something like below instead of the 
operator overloading.
double divide(T, U)(T x, U y)
     if (isIntegral!T && isIntegral!U)
{
     return cast(double) / cast(double) y;
}
Aug 04 2020
parent burt <invalid_email_address cab.abc> writes:
On Tuesday, 4 August 2020 at 12:53:24 UTC, jmh530 wrote:

 On Tuesday, 4 August 2020 at 12:34:37 UTC, burt wrote:

 [snip]

 Would such a change require a DIP? Could a 
 -preview/-transition flag be added to warn me about integer 
 divisions? What do you think?


 The problem is you have stuff like
 ```d
 int x = 2;
 int y = 4 / x;
 ```
 and now you would be force someone to write a cast.


I guess I should have been more specific, I meant only when 
converting to `double`/`float`/`real`, because then you will 
implicitly lose precision. When converting the result to `int`s, 
you already expect the loss in precision of the division, so it's 
not a big deal.


 That's a big enough change that it would require a DIP. Though 
 I've been hit by the same thing in the past, I would be a bit 
 skeptical you could get it through since a lot of code depends 
 on that.


Perhaps an opt-in flag (-vintdiv or something, just like 
-vtemplates/-vgc) would be more appropriate. It would be a useful 
tool, and I would have it active all the time.


 You could also always use something like below instead of the 
 operator overloading.
 double divide(T, U)(T x, U y)
     if (isIntegral!T && isIntegral!U)
 {
     return cast(double) / cast(double) y;
 }


The problem isn't fixing the problem, it's finding it. I would 
have to replace all my divisions with this function, and that's 
not very practical.
Aug 04 2020
prev sibling next sibling parent reply WebFreak001 <d.forum webfreak.org> writes:
On Tuesday, 4 August 2020 at 12:34:37 UTC, burt wrote:

 [...]
 I have run into this C remnant multiple times now and spent 
 much too long trying to find out what happened, so to fix it, 
 here's my proposal:

 "Make integer division without cast illegal."

 In the case above, the compiler would complain (or at least 
 emit a warning) that there is integer division going on and 
 that the result will be truncated.

 [...]


I agree having this as warning would be useful, however it should 
not apply to:

int x = y / z;

but only to

float x = y / z; (and double)

In my mind I would like if this was implemented something along 
the lines "if this expression has an integer division in it and 
is implicitly converted to float/double, emit a warning", if it 
isn't implicitly converted to float/double it should not emit a 
warning, as casts are quite ugly and may introduce bugs later on 
if types change.

Also alternative to casting, which would be safer:

float x = int(y / z);

This is already valid syntax and is basically just like an 
implicit assignment, just explicitly written out. This way you 
can't accidentally change y or z to float/double without 
triggering a compiler error.

The rule with this would be: an implicit conversion to int is 
treated as clearing any integer division flag inside an 
expression.


Though I haven't really contributed to dmd, so I don't know how 
feasible this actually is :)
Aug 04 2020
parent reply burt <invalid_email_address cab.abc> writes:
On Tuesday, 4 August 2020 at 12:54:42 UTC, WebFreak001 wrote:

 On Tuesday, 4 August 2020 at 12:34:37 UTC, burt wrote:

 [...]


 I agree having this as warning would be useful, however it 
 should not apply to:

 int x = y / z;

 but only to

 float x = y / z; (and double)


Yes exactly, I might not have been clear about this but that is 
what I intended.


 In my mind I would like if this was implemented something along 
 the lines "if this expression has an integer division in it and 
 is implicitly converted to float/double, emit a warning", if it 
 isn't implicitly converted to float/double it should not emit a 
 warning, as casts are quite ugly and may introduce bugs later 
 on if types change.

 Also alternative to casting, which would be safer:

 float x = int(y / z);


Sure.


 This is already valid syntax and is basically just like an 
 implicit assignment, just explicitly written out. This way you 
 can't accidentally change y or z to float/double without 
 triggering a compiler error.

 [...]

 Though I haven't really contributed to dmd, so I don't know how 
 feasible this actually is :)


Me neither... that's why I was asking around if it's feasible.
Aug 04 2020
parent Timon Gehr <timon.gehr gmx.ch> writes:
On 04.08.20 15:13, burt wrote:

 On Tuesday, 4 August 2020 at 12:54:42 UTC, WebFreak001 wrote:

 On Tuesday, 4 August 2020 at 12:34:37 UTC, burt wrote:

 [...]


 I agree having this as warning would be useful, however it should not 
 apply to:

 int x = y / z;

 but only to

 float x = y / z; (and double)


 
 Yes exactly, I might not have been clear about this but that is what I 
 intended.
 

 In my mind I would like if this was implemented something along the 
 lines "if this expression has an integer division in it and is 
 implicitly converted to float/double, emit a warning", if it isn't 
 implicitly converted to float/double it should not emit a warning, as 
 casts are quite ugly and may introduce bugs later on if types change.

 Also alternative to casting, which would be safer:

 float x = int(y / z);


 
 Sure.
 

 This is already valid syntax and is basically just like an implicit 
 assignment, just explicitly written out. This way you can't 
 accidentally change y or z to float/double without triggering a 
 compiler error.

 [...]

 Though I haven't really contributed to dmd, so I don't know how 
 feasible this actually is :)


 
 Me neither... that's why I was asking around if it's feasible.


Sure. For someone not very familiar with the DMD code base, the way to 
make it happen is to find some error message mentioning things that are 
related to the patch you want to make (like implicit casting) and grep 
for them in the DMD source code. Then read the code around them until 
you understand how to patch DMD so it emits your warning.
Aug 04 2020
prev sibling parent reply bachmeier <no spam.net> writes:
On Tuesday, 4 August 2020 at 12:34:37 UTC, burt wrote:

 Hello,

 A while ago I spent an hour figuring out why some variable 
 wasn't changing like it should, and the reason was something 
 like the following (although in a much more complex expression):

 ```d
 double y = 0.0;
 // ...
 y += 1 / b; // with b : int
 // ...
 ```

 I have run into this C remnant multiple times now and spent 
 much too long trying to find out what happened, so to fix it, 
 here's my proposal:

 "Make integer division without cast illegal."

 In the case above, the compiler would complain (or at least 
 emit a warning) that there is integer division going on and 
 that the result will be truncated.

 If I want to keep the integer division behaviour, I would just 
 have to change the code to:

 ```d
 y += cast(int) (1 / b);
 ```


This would be *really* confusing. y is a double in your example, 
and the user would be forced to cast to an int in order to enable 
an implicit cast (or promotion, or whatever language you want to 
use) to a double?

I hate the current default behavior. It's not safe but it is 
consistent with what a C programmer will expect. You can't have 
these yielding different values:

```
x = 7/3;
y = 7/3;
```

If x is an int and y is a double, you'd get different results, 
and that would be awful. What's needed is something explicit like 
this:

y = double(7/3);
x = 7/3;
y = 7/3; // Error because of implicit cast

There is precedent for this in the change to foreach. This code

import std;
void main()
{
     foreach(int ii, val; [1.1, 2.2, 3.3]) {
         writeln(ii);
         writeln(val);
     }
}

returns

onlineapp.d(4): Deprecation: foreach: loop index implicitly 
converted from size_t to int
0
1.1
1
2.2
2
3.3

You have to do the casting inside the loop instead.
Aug 04 2020
parent reply burt <invalid_email_address cab.abc> writes:
On Tuesday, 4 August 2020 at 15:25:50 UTC, bachmeier wrote:

 On Tuesday, 4 August 2020 at 12:34:37 UTC, burt wrote:

 [...]


 This would be *really* confusing. y is a double in your 
 example, and the user would be forced to cast to an int in 
 order to enable an implicit cast (or promotion, or whatever 
 language you want to use) to a double?

 I hate the current default behavior. It's not safe but it is 
 consistent with what a C programmer will expect. You can't have 
 these yielding different values:

 ```
 x = 7/3;
 y = 7/3;
 ```


That is not what I was proposing (please reread). I am just 
saying that the bottom one (assuming x is int and y is double) 
should issue a warning, unless it is explicitly told that integer 
division is intended.


 If x is an int and y is a double, you'd get different results, 
 and that would be awful. What's needed is something explicit 
 like this:

 y = double(7/3);
 x = 7/3;
 y = 7/3; // Error because of implicit cast


This is the same as what I was proposing, except for the syntax 
to explicitly annotate integer division: in your example, you use 
a cast to double (to show you're casting from integer to 
division), in my example, you use a cast to int (to show you're 
truncating). Open for discussion, but the same idea.


 [...]
Aug 04 2020
parent bachmeier <no spam.net> writes:
On Tuesday, 4 August 2020 at 15:43:19 UTC, burt wrote:

 On Tuesday, 4 August 2020 at 15:25:50 UTC, bachmeier wrote:



 That is not what I was proposing (please reread). I am just 
 saying that the bottom one (assuming x is int and y is double) 
 should issue a warning, unless it is explicitly told that 
 integer division is intended.


It may not have been clear from my reply, but I was only saying 
whatever is done, if anything, it has to preserve the constraint 
that the same division return the same result. That part wasn't a 
direct response to your proposal.


 If x is an int and y is a double, you'd get different results, 
 and that would be awful. What's needed is something explicit 
 like this:

 y = double(7/3);
 x = 7/3;
 y = 7/3; // Error because of implicit cast


 This is the same as what I was proposing, except for the syntax 
 to explicitly annotate integer division: in your example, you 
 use a cast to double (to show you're casting from integer to 
 division), in my example, you use a cast to int (to show you're 
 truncating). Open for discussion, but the same idea.


It's kind of the same. There are two issues. One is the integer 
division that is missed. The other is the silent 
casting/promotion of an integer to a double. My opinion is that 
it's rarely the case that this code does what you want:

double y = 7/3;

I just don't think that's reasonable. If you do want integer 
division *and* you want to store it in a double, you should do 
something like

double y = trunc(7.0/3);

or

int x = 7/3;
double y = x;

I see I had an error above. This

y = double(7/3);

should have been

y = double(7)/3;

The former is not clear, while the latter forces the result to be 
a double. Your proposal of

y += cast(int) (1 / b);

does tell the viewer there's something going on, but the code is 
still not doing what you want. OCaml has its own operator and 
requires all integers to be explicitly promoted to float prior to 
doing the division. That might be overkill, but I prefer it to 
D's current bug-inducing behavior.
Aug 04 2020