Thread overview
Killing integer division problems?
Aug 04, 2020
burt
Aug 04, 2020
jmh530
Aug 04, 2020
burt
Aug 04, 2020
WebFreak001
Aug 04, 2020
burt
Aug 04, 2020
Timon Gehr
Aug 04, 2020
bachmeier
Aug 04, 2020
burt
Aug 04, 2020
bachmeier
August 04, 2020
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?

August 04, 2020
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;
}


August 04, 2020
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 :)
August 04, 2020
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.

August 04, 2020
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.
August 04, 2020
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.
August 04, 2020
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.

> [...]

August 04, 2020
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.
August 04, 2020
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.