Tcl Source Code

View Ticket
Login
Ticket UUID: 1340260
Title: Simplify round() implementation in expr cmd
Type: Patch Version: None
Submitter: mdejong Created on: 2005-10-28 04:22:54
Subsystem: 45. Parsing and Eval Assigned To: dgp
Priority: 5 Medium Severity:
Status: Closed Last Modified: 2005-11-02 02:13:36
Resolution: Rejected Closed By: dgp
    Closed on: 2005-11-01 15:18:28
Description:
I was just reviewing the round() implementation in Tcl 8.4
vs Tcl 8.5. The round() implementation seems to be
more complex than it needs to be. The attached patch
simplifies the function so that it works more like the
8.4 version. I also added tests cases around the min int
and max int cases. As far and I can tell, the results of
no tests are changed by this patch. Kevin, could you
review this patch before I apply it?
User Comments: kennykb added on 2005-11-02 02:13:36:
Logged In: YES 
user_id=99768

Don is correct - because of the double rounding step.

Converting a wide to a double has to round to 53 significant
bits, so converting 0x7fffffffffffffff rounds up:

% expr 0x7fffffffffffffff
9223372036854775807
% expr entier(double(0x7fffffffffffffff))
9223372036854775808

Rounding the double then yields an integer that overflows
when reconverted to 64-bits, so winds up being negative
rather than positive.

dgp added on 2005-11-02 02:10:15:
Logged In: YES 
user_id=80530

expr {double(.)} converts to a
double-precision floating point
value, and suffers the loss of
precision compared with a
64-bit integer that implies.

In these examples, the nearest
double value to 0x7fffffffffffffff
is 1.0 * 2**63, or 9223372036854775808.0

Convert that back to integer using
entier(.) to get 9223372036854775808

Convert to integer and truncate to 
wide integer range using wide(.), and
get the results you report.

Pehaps "odd" at first glance, but
I think correct results.

mdejong added on 2005-11-02 01:55:18:
Logged In: YES 
user_id=90858

Here is another test case that I find odd.

% expr {wide(0x7fffffffffffffff)}
9223372036854775807

% expr {wide(double(0x7fffffffffffffff))}
-9223372036854775808

In the round() example you have below, should the result
actually be 9223372036854775807?

dgp added on 2005-11-01 22:18:27:
Logged In: YES 
user_id=80530


Here's a test case:

test expr-46.13 {round() handling of long/bignum boundary} {
    expr {round(double(0x7fffffffffffffff))}
} 9223372036854775808

With the contributed patch, on L64 systems:

==== expr-46.13 round() handling of long/bignum boundary FAILED
==== Contents of test case:

    expr {round(double(0x7fffffffffffffff))}

---- Result was:
-9223372036854775808
---- Result should have been (exact matching):
9223372036854775808
==== expr-46.13 FAILED

On the HEAD, the test passes.

Committing that to the test suite...

dgp added on 2005-11-01 21:55:14:
Logged In: YES 
user_id=80530


The problem is with systems with
64-bit longs.  Since IEEE doubles
have only 53 bits of precision, the
calculation

    (double) LONG_MAX - 1.0

for example, can't be performed.
That is, the subtraction is lost
below the precision threshold.

Doing the operations as longs
doesn't suffer this problem.

kennykb added on 2005-10-31 22:28:37:
Logged In: YES 
user_id=99768

Don, I don't see anything obvious wrong with Mo's approach,
but I haven't had time to really go over things with a
fine-toothed
comb.  It looks as if he has the corner cases accounted for.
Could you give it a second look?

mdejong added on 2005-10-28 11:22:56:

File Added - 154087: round.patch

Attachments: