WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#28559 closed enhancement (wontfix)

Unit tests: ABSINT() returns bad result for large or long numbers

Reported by: pbearne Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.5
Component: General Keywords:
Focuses: Cc:

Description

Hi All

See the attached unit tests.

The ABSINT() function will return unexpected numbers with some edge cases.

This ticket is here as much as anything to document them as mySQL will have imploded before this hit most of these tests are working

Here are some quick examples (see unit test patch for more)

we round up not down if we have lots of .9+16

$this->assertEquals( absint( 4.999999999999999 ), 4, '4.999999999999999' );
$this->assertEquals( absint( 4.9999999999999999 ), 5, '4.9999999999999999' ); 
//64 bit
$this->assertEquals( absint( 49999999999999999999 ), 5340232221128654848, '49999999999999999999' );
// 32 bit
$this->assertEquals( absint( 4999999999.9999999999 ), 705032704, '4999999999.9999999999' );
$this->assertEquals( absint( PHP_INT_MAX +1 ), 2147483648, 'PHP_INT_MAX+1-32bit' );
$this->assertEquals( absint( PHP_INT_MAX +11 ), 2147483638, 'PHP_INT_MAX+11-32bit' );
$this->assertEquals( absint( PHP_INT_MAX +99 ), 2147483550, 'PHP_INT_MAX+99-32bit' );
$this->assertEquals( absint( PHP_INT_MAX +999 ), 2147482650, 'PHP_INT_MAX+999-32bit' );

Attachments (4)

absint.php (14.7 KB) - added by pbearne 7 years ago.
ABSINT() unit tests test/functions/absint.php
absint.2.php (14.7 KB) - added by pbearne 7 years ago.
ABSINT() unit tests test/functions/absint.php
absint.php.patch (13.7 KB) - added by pbearne 7 years ago.
Patch for the Unit Test
absint.php.2.patch (13.8 KB) - added by pbearne 7 years ago.
patch with 64bit update

Download all attachments as: .zip

Change History (12)

@pbearne
7 years ago

ABSINT() unit tests test/functions/absint.php

@pbearne
7 years ago

ABSINT() unit tests test/functions/absint.php

This ticket was mentioned in IRC in #wordpress-dev by pbearne. View the logs.


7 years ago

@pbearne
7 years ago

Patch for the Unit Test

#3 @pbearne
7 years ago

  • Keywords has-patch added

@pbearne
7 years ago

patch with 64bit update

#4 @nacin
7 years ago

I have absolutely zero desire to work around these issues.

absint() is simply a wrapper for (int) and abs(). What about all of the times we're already using (int) instead? What about all of the times we should be using (int) instead? I struggle to see us wanting to do anything along these lines. Do any other PHP projects decide to not trust int casts? Should we just abandon PHP at this point? Or abandon 32-bit systems? Or, hell, 64-bit systems?

Cool bug, and an interesting mess, but wontfix.

#5 @pbearne
7 years ago

I agree with the wontfix :-) there is no clean way to fix it.

But I would ask the the patch is added to the test so we have coverage and to form part of the documentation / examples of the problem.

Paul

#6 @helen
7 years ago

  • Summary changed from ABSINT() returns bad result for large or long numbers to Unit tests: ABSINT() returns bad result for large or long numbers
  • Type changed from defect (bug) to enhancement
  • Version changed from trunk to 2.5

#7 @pbearne
7 years ago

Some throughts

In PHP 5.6 "GMP objects now support operator overloading and casting to scalar types."
http://php.net/manual/en/migration56.new-features.php

Which means that we can do this -

function absint( $maybeint ) {
	if( 5.6 <= PHP_VERSION ){
		return gmp_abs( gmp_invert( $maybeint ) );
	}
	return abs( intval( $maybeint ) );
}

Not added as patch as I haven't got a PHP 5.6 install to test.

Will this fly?

Or we can get creative like this

function absint( $maybeint ) {
	$absint      = abs( intval( $maybeint ) );

	if( function_exists( 'gmp_abs' ) ) {
		$gmp_absint = gmp_abs( gmp_invert( $maybeint ) );

		if ( $absint != $gmp_absint ) {
			return $gmp_absint;
		}
	}

	return $absint;

This will return a gmp object if the number is over maxint and work's in pre 5.6

Testing etc. needed :-)

#8 @aaroncampbell
6 years ago

  • Keywords has-patch removed
  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Using gmp_intval doesn't actually fix this (I don't think we'd want to try to use it even if it did, but it doesn't):

var_dump( (int) 4.999999999999999 ); // 4
var_dump( (int) 4.9999999999999999 ); // 5
var_dump( gmp_intval( 4.999999999999999 ) ); // 4
var_dump( gmp_intval( 4.9999999999999999 ) ); // 5

Having said that, it also seems to me like unit tests for abs() or casting to an int belong in PHP's unit tests not ours.

Note: See TracTickets for help on using tickets.