wduquette / molt

Embeddable TCL Interpreter for Rust applications
BSD 3-Clause "New" or "Revised" License
103 stars 12 forks source link

Integer Division is incorrect #27

Closed wduquette closed 5 years ago

wduquette commented 5 years ago

The expr command has an error. The expression M/N should always return 0 where M and N are integers and M < abs(N). This works correctly if N is positive, but if N is negative the result is -1 if M is positive, and 1 if M is negative.

% expr {1/-2}
-1
% expr {-1/-2}
1

The magnitude of the divisor doesn't matter, so long as abs(M) < abs(N).

wduquette commented 5 years ago

In fact, this appears to be a general problem with the divisor is negative:

% expr {11/-10}
-2
% expr {-11/-10}
2
wduquette commented 5 years ago

The TCL 7.6 expr code contains a complex little algorithm to give consistent remainders with negative divisors across C compilers. This may still be an issue in Rust across platforms; but the translated code does the wrong thing, and inspecting it I can't see how the C code ever did the right thing. I've replaced it with a naive algorithm, and it seems to work. I'll need to check remainders separately.

wduquette commented 5 years ago

Whoops! I notice that TCL 8.6 gives the same answers as TCL 7.6, which implies that the answers are correct if counter intuitive. ????

Reopening until I understand what's going on.

wduquette commented 5 years ago

Here's a table of results I've compiled.

Operation TCL 7.6 TCL 8.6 Molt Rust
12 / 10 1 1 1 1
12 % 10 2 2 2 2
-12 / 10 -2 -2 -1 -1
-12 % 10 8 8 -2 -2
12 / -10 -2 -2 -2 -1
12 % -10 -8 -8 -8 2
-12 / -10 1 1 2 1
-12 % -10 -2 -2 8 -2

From this, we see:

The question now is, which behavior is preferable for Molt? Consistency with the host language (appropriate for an extension language) or consistency with long-established TCL practice?

wduquette commented 5 years ago

Per Kevin Kenny:

Kevin insists that the / and % must be defined consistently, i.e., that the following equation holds for integer a and b, where b is not zero:

b*(a/b) + (a%b) == a