Tcl Source Code

View Ticket
Login
Ticket UUID: 85ce4bf928cd9a40064aa5b36e7033184dd5379a
Title: [binary format R Inf] stores FLT_MAX
Type: Bug Version: 8.6.3
Submitter: anonymous Created on: 2015-02-02 08:21:54
Subsystem: 48. Number Handling Assigned To: kbk
Priority: 5 Medium Severity: Minor
Status: Open Last Modified: 2016-11-10 13:01:19
Resolution: Remind Closed By: nobody
    Closed on:
Description:
% binary scan [binary format R Inf] R x
% set x
3.4028234663852886e+38

This is FLT_MAX - see the bit pattern described near the bottom of http://wiki.tcl.tk/41088

The issue seems to be the test at http://core.tcl.tk/tcl/artifact/fee105a45aaf25a3?ln=1915-1917

By my reading, the condition should become:

    if (!isinf(dvalue) && (fabs(dvalue) > (double)FLT_MAX))

In the chat, kbk adds:

If the float64 is less than FLT_MAX + 1/2 ulp, then it's not a 'true' overflow, and it should [binary format] to FLT_MAX. Greater than that, and it should be Inf.

Exact FLT_MAX + 1/2 ulp should round to FLT_MAX (round to even).

I have implemented a fix in http://chiselapp.com/user/aspect/repository/tcl/timeline?r=aspect-binary-float-inf

The first checkin http://chiselapp.com/user/aspect/repository/tcl/ci/f4d7334db9 corrects behaviour for Inf/-Inf only, and adds some tests.

Following checkins follow kbk's advice (I hope!) to correct out-of-range rounding behaviour, but with some questionable aspects:

  * c99 reliance for INFINITY
  * FLT_MAX+1/2ulp is calculated with an expression I don't entirely trust
  * changes results (!!) of some existing tests which expect rounding to FLT_MAX

The included tests should be complete, but those in the 3rd checkin don't conform to conventions of their neighbours.

Another way to create FLT_MAX+1/2ulp for comparison would be as follows, though I can't vouch for its portability.

    /* smallest double that should round to +Inf */
    double flt_min_inf {} {
        uint64_t bits = 0x47effffff0000001;
        return *(double*)&bits;
    }

    /* largest double that should round to FLT_MAX */
    double flt_max_dbl {} {
        uint64_t bits = 0x47effffff0000000;
        return *(double*)&bits;
    }

Let me know if there's anything I can do to make this patch easier to consume.
User Comments: aspect added on 2016-11-10 13:01:19:
As another data point, the same is not true of doubles:

% binary scan [binary format q Inf] q x
1
% set x
Inf

dkf added on 2015-05-15 22:17:16:

Imported as branch bug-85ce4bf928 (see [21866fab39])


anonymous (claiming to be aspect) added on 2015-02-20 23:02:20:
fossil bundle attached to this ticket.

dgp added on 2015-02-04 18:18:08:
The patch changes the expected result of
existing tests in the test suite, so I'd
prefer to have a review from kbk.

dgp added on 2015-02-04 18:14:59:
The patch changes the expected result of
existing tests in the test suite, so I'd
prefer to have a review from kbk.

dgp added on 2015-02-04 18:10:28:
The patch changes the expected result of
existing tests in the test suite, so I'd
prefer to have a review from kbk.

dgp added on 2015-02-04 18:03:16:
The patch changes the expected result of
existing tests in the test suite, so I'd
prefer to have a review from kbk.

dgp added on 2015-02-04 15:43:29:
The patch changes the expected result of
existing tests in the test suite, so I'd
prefer to have a review from kbk.

dgp added on 2015-02-04 15:21:35:
The patch changes the expected result of
existing tests in the test suite, so I'd
prefer to have a review from kbk.

dgp added on 2015-02-04 15:15:50:
The patch changes the expected result of
existing tests in the test suite, so I'd
prefer to have a review from kbk.

dgp added on 2015-02-04 15:12:30:
The patch changes the expected result of
existing tests in the test suite, so I'd
prefer to have a review from kbk.

Attachments: