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:
- round.patch [download] added by mdejong on 2005-10-28 11:22:55. [details]