The bignum module changes behavior of Math::BigInt::bdiv()

[p5pRT]

Migrated from rt.perl.org#70109 (status was 'open')

Searchable as RT70109$

[p5pRT]

From jl_post@hotmail.com

Created by jl_post@hotmail.com

The documentation in "perldoc Math​::BigInt" says that if $x is a Math​::BigInt object\, the code​:

  $x->bdiv($y);

will change the value of $x. That is exactly what I see when I run the following code​:

#!/usr/bin/perl

use strict; use warnings;

use Math​::BigInt; my $n = Math​::BigInt->new(6);

print "\$n before \$n->bdiv(2)​: $n\n"; # prints 6 $n->bdiv(2); print "\$n after \$n->bdiv(2)​: $n\n"; # should print 3

# use bignum;

__END__

The output I get (on both Linux and Strawberry Perl for Windows) is​:

$n before $n->bdiv(2)​: 6 $n after $n->bdiv(2)​: 3

Nothing wrong here. (This is exactly what I expect to see.)

However\, if I uncomment the "use bignum;" line (that last line of the script\, right before the __END__ tag) and re-run the code\, I get this output (on both Linux and Strawberry Perl for Windows)​:

$n before $n->bdiv(2)​: 6 $n after $n->bdiv(2)​: 6

Notice that now the second line says that $n is 6. (It should say 3.)

Evidently just the inclusion of the "bignum" module is causing the Math​::BigInt​::bdiv() method to behave differently (in that it is not modifying the calling object).

I haven't found any other methods that are affected (such as Math​::BigInt​::bmul())\, any other modules that are affected (such as Math​::BigFloat)\, or any other offending modules that cause this problem (such as "bigint").

So the bug is that simply declaring "use bignum;" causes the Math​::BigInt​::bdiv() method to not modify the calling object.

Perl Info

``` Flags: category=core severity=low Site configuration information for perl v5.8.4: Configured by Debian Project at Wed May 10 04:14:05 UTC 2006. Summary of my perl5 (revision 5 version 8 subversion 4) configuration: Platform: osname=linux, osvers=2.6.15.6, archname=i386-linux-thread-multi uname='linux ernie 2.6.15.6 #1 thu mar 16 13:11:55 est 2006 i686 gnulinux ' config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=i386-linux -Dprefix=/usr -Dprivlib=/usr/share/perl/5.8 -Darchlib=/usr/lib/perl/5.8 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.8.4 -Dsitearch=/usr/local/lib/perl/5.8.4 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Uusesfio -Uusenm -Duseshrplib -Dlibperl=libperl.so.5.8.4 -Dd_dosuid -des' hint=recommended, useposix=true, d_sigaction=define usethreads=define use5005threads=undef useithreads=define usemultiplicity=define useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBIAN -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2', cppflags='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBIAN -fno-strict-aliasing -I/usr/local/include' ccversion='', gccversion='3.3.5 (Debian 1:3.3.5-13)', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=4, prototype=define Linker and Libraries: ld='cc', ldflags =' -L/usr/local/lib' libpth=/usr/local/lib /lib /usr/lib libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt perllibs=-ldl -lm -lpthread -lc -lcrypt libc=/lib/libc-2.3.2.so, so=so, useshrplib=true, libperl=libperl.so.5.8.4 gnulibc_version='2.3.2' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib' Locally applied patches: @INC for perl v5.8.4: /etc/perl /usr/local/lib/perl/5.8.4 /usr/local/share/perl/5.8.4 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.8 /usr/share/perl/5.8 /usr/local/lib/site_perl . Environment for perl v5.8.4: HOME=/home/jromano LANG=en_US LANGUAGE=en_US:en_GB:en LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/local/bin:/bin:/usr/bin:/usr/X11R6/bin PERL_BADLANG (unset) SHELL=/bin/tcsh ```

[p5pRT]

From @pjf

On Thu Oct 29 10​:21​:18 2009\, jl_post@​hotmail.com wrote​:

Evidently just the inclusion of the "bignum" module is causing the Math​::BigInt​::bdiv() method to behave differently (in that it is not modifying the calling object).

Ouch! Nice catch\, that looks like a nasty bug. I can confirm it's still a problem under 5.10.0 and 5.10.1.

I'm marking this as something that needs attention in 5.12.0. Giving wrong results is bad.

Thanks again for the bug report\, it's very much appreciated!

  Paul

-- Paul Fenwick \pjf@​perltraining\.com\.au | http​://perltraining.com.au/ Director of Training | Ph​: +61 3 9354 6001 Perl Training Australia | Fax​: +61 3 9354 2681

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From @TJC

I confirmed this bug still exists in 5.11.1

[p5pRT]

From @iabyn

On Thu\, Oct 29\, 2009 at 10​:21​:18AM -0700\, jl_post@​hotmail.com (via RT) wrote​:

#!/usr/bin/perl

use strict; use warnings;

use Math​::BigInt; my $n = Math​::BigInt->new(6);

print "\$n before \$n->bdiv(2)​: $n\n"; # prints 6 $n->bdiv(2); print "\$n after \$n->bdiv(2)​: $n\n"; # should print 3

# use bignum;

[snip]

However\, if I uncomment the "use bignum;" line (that last line of the script\, right before the __END__ tag) and re-run the code\, I get this output (on both Linux and Strawberry Perl for Windows)​:

$n before $n->bdiv(2)​: 6 $n after $n->bdiv(2)​: 6

Notice that now the second line says that $n is 6. (It should say 3.)

Evidently just the inclusion of the "bignum" module is causing the Math​::BigInt​::bdiv() method to behave differently (in that it is not modifying the calling object).

It's actually a big in BigInt.pm or BigFloat.pm; the 'use bignum' just sets an internal flag\, as can be seen below​:

  use Math​::BigInt;   use Math​::BigFloat;

  my $fn = Math​::BigInt->new(6);   $Math​::BigInt​::upgrade ='Math​::BigFloat'; # side-effect of 'use bignum';   $fn->bdiv(2);

  print "result (should be 3)​: $fn\n";

Jesse\, given that Big* are CPAN modules and that this bug is present in 5.8.*\, I can't see that this bug should be a 5.12 showstopper.

Also\, does anyone know if there's a way to transfer this to the appropriate CPAN RT queue\, since its not a core bug? Or does one just ask the originator to re-report it?

-- That he said that that that that is is is debatable\, is debatable.

[p5pRT]

From @obra

On Wed Jan 20 14​:13​:43 2010\, davem wrote​:

Jesse\, given that Big* are CPAN modules and that this bug is present in 5.8.*\, I can't see that this bug should be a 5.12 showstopper.

It's not a regression\, but I was certainly happy to take PJF's assertion that it's nasty enough to want a fix at face value.

Bugs in dual-lifed modules can certainly be showstoppers if they're nasty enough. Doing my own pass through this list an hour ago\, I opted not to unlink as a showstopper yet\, mostly in the hope that the dwindling size of the list would sucker some poor soul into providing us/tels with a fix. I don't feel particularly strongly about that\, though. If you want to unlink it now\, I won't object.

Also\, does anyone know if there's a way to transfer this to the appropriate CPAN RT queue\, since its not a core bug? Or does one just ask the originator to re-report it?

There is not. We've been talking to perl.org about merging the two\, but we're not there yet.

[p5pRT]

From @nwc10

On Wed\, Jan 20\, 2010 at 10​:13​:10PM +0000\, Dave Mitchell wrote​:

Also\, does anyone know if there's a way to transfer this to the appropriate CPAN RT queue\, since its not a core bug? Or does one just ask the originator to re-report it?

I've been rather hoping that sd could be taught how to do this (specifically one way trips).

http​://syncwith.us/sd/

Nicholas Clark

[p5pRT]

From @xdg

On Wed\, Jan 20\, 2010 at 5​:19 PM\, Jesse via RT \perlbug\-followup@​perl\.org wrote​:

Also\, does anyone know if there's a way to transfer this to the appropriate CPAN RT queue\, since its not a core bug? Or does one just ask the originator to re-report it?

There is not. We've been talking to perl.org about merging the two\, but we're not there yet.

One could forward the original email to the right RT queue at rt.cpan.org. And then "reply" to the new ticket with a link to the rt.perl.org ticket for further discussion.

David

[p5pRT]

From @obra

Based on previous discussion and input from former pumpkings\, I'm removing this as a 5.12.0 blocker.

[jkeenan]

Back in October 2009 it was reported,

"Evidently just the inclusion of the "bignum" module is causing
the Math​::BigInt​::bdiv() method to behave differently (in that
it is not modifying the calling object)."

There was discussion as to the real cause of the problem.

I reported this problem upstream today at https://rt.cpan.org/Ticket/Display.html?id=150252. @pjacklam, can you take a look? Thanks.

[pjacklam]

The underlying cause here is upgrading and downgrading. Unlike the bigint, bigrat, and bigfloat pragmas, the bignum pragma enables upgrading and downgrading by default. The observed behaviour is how it always has been¹, and from what I understand, this is how the original author intended it to work.

Here is what happens: Division is an operation that might return a non-integer. So when upgrading is enabled in Math::BigInt, the division is handed over to the upgrade class. By default, Math::BigInt upgrades to Math::BigFloat. So it is Math::BigFloat that performs the division, using Math::BigFloat objects. When Math::BigFloat has performed a division, and downgrading is enabled, Math::BigFloat checks the result to see if it is an integer, and if it is, the result is passed to the downgrade class. By default, Math::BigFloat downgrades to Math::BigInt. So the final result is a Math::BigInt object. However, it will be a different object than the original invocand object.

The same thing happens with every Math::BigInt method that might return a non-integer, not just bdiv(). You get the same with bsqrt():

$x = Math::BigInt -> new(9); $x -> bsqrt();    # $x is still 9

I think the best solution is to improve the documentation and make it clear that this will happen, and why. The only way to make sure that the $x is never modified when upgrading and/or downgrading is enabled, is to re-bless $x from one class to another. This can be done, but from what I know of OOP, this is very bad practice. It would be possible to check whether the result can be represented exactly as an integer, and upgrade only when the result is not an integer. The downside of this, is that certain operations must be computed twice. But I am open for suggestions, corrections, arguments, … :)

¹ There were a few releases of bignum where I disabled upgrading and downgrading. This functionality was only partly implemented, according to the orginal author, and I had given up trying to understand how and when it was supposed to work. Eventually I got a grip of it and enabled it again.

[jkeenan]

The underlying cause here is upgrading and downgrading. Unlike the bigint, bigrat, and bigfloat pragmas, the bignum pragma enables upgrading and downgrading by default. The observed behaviour is how it always has been¹, and from what I understand, this is how the original author intended it to work.

Here is what happens: Division is an operation that might return a non-integer. So when upgrading is enabled in Math::BigInt, the division is handed over to the upgrade class. By default, Math::BigInt upgrades to Math::BigFloat. So it is Math::BigFloat that performs the division, using Math::BigFloat objects. When Math::BigFloat has performed a division, and downgrading is enabled, Math::BigFloat checks the result to see if it is an integer, and if it is, the result is passed to the downgrade class. By default, Math::BigFloat downgrades to Math::BigInt. So the final result is a Math::BigInt object. However, it will be a different object than the original invocand object.

The same thing happens with every Math::BigInt method that might return a non-integer, not just bdiv(). You get the same with bsqrt():

$x = Math::BigInt -> new(9); $x -> bsqrt();    # $x is still 9

@pjacklam, thank you for that explanation. I appreciate the time and effort you had to put into researching it.

I think the best solution is to improve the documentation and make it clear that this will happen, and why.

Yes, I think that some documentation about the existence of weird corner cases would be good.

[sisyphus]

The only way to make sure that the $x is modified when upgrading and/or downgrading is enabled, is to re-bless $x from one class to another. This can be done, but from what I know of OOP, this is very bad practice, ...

But this is exactly what already happens with:

D:\>perl -Mbignum -wle "$x = Math::BigInt->new(7); print ref($x); $x /= 2; print $x; print ref($x);"
Math::BigInt
3.5
Math::BigFloat

I personally think that's the correct behaviour, and I would expect that the exact same thing would happen with:

D:\>perl -Mbignum -wle "$x = Math::BigInt->new(7); print ref($x); $x->bdiv(2); print $x; print ref($x);"
Math::BigInt
7
Math::BigInt

But if we want to use bdiv() we instead need to do:

D:\>perl -Mbignum -wle "$x = Math::BigInt->new(7); print ref($x); $x = $x->bdiv(2); print $x; print ref($x);"
Math::BigInt
3.5
Math::BigFloat

To make it worse, these method calls will modify-in-place the object's value if there's no upgrading involved, but won't modify-in-place the value if upgrading is involved, UPDATE - 2 examples that demonstrate this:

D:\>perl -Mbignum -wle "$x = 7; $x->badd(2.5); print $x;"
7

D:\>perl -Mbignum -wle "$x = 7; $x->badd(2); print $x;"
9

This is all goes on silently, does not DWIM, and requires that the programmer be alert to these issues.

I know that ships have sailed and bridges have been burnt. I don't use bignum (and I haven't perused the docs,), but if I did use bignum I would hope that I use the overloaded operators rather than these troublesome method calls.

@pjacklam, if it's impractical to make any behavioural changes, then I agree that the bignum docs should prominently set out the perils of using the method calls.

Cheers, Rob

[pjacklam]

The only way to make sure that the $x is modified when upgrading and/or downgrading is enabled, is to re-bless $x from one class to another. This can be done, but from what I know of OOP, this is very bad practice, ...

But this is exactly what already happens with:

D:\>perl -Mbignum -wle "$x = Math::BigInt->new(7); print ref($x); $x /= 2; print $x; print ref($x);"
Math::BigInt
3.5
Math::BigFloat

You got me there. I didn’t expect that. It must be perl itself that takes care of this, because /= is implemented as

    '/=' => sub { scalar $_[0] -> bdiv($_[1]); },

When I look at the addresses, I see that they change:

$ perl -MScalar::Util=refaddr -Mbignum -wle \
    '$x = Math::BigInt->new(7); print refaddr($x); $x /= 2; print $x; print refaddr($x);'
42949688776
3.5
42949904040

I personally think that's the correct behaviour, and I would expect that the exact same thing would happen with:

D:\>perl -Mbignum -wle "$x = Math::BigInt->new(7); print ref($x); $x->bdiv(2); print $x; print ref($x);"
Math::BigInt
7
Math::BigInt

I agree. I think the fact that $x /= 2 and $x -> bdiv(2) give different results is counter-intuitive.

But if we want to use bdiv() we instead need to do:

D:\>perl -Mbignum -wle "$x = Math::BigInt->new(7); print ref($x); $x = $x->bdiv(2); print $x; print ref($x);"
Math::BigInt
3.5
Math::BigFloat

Right. The code is full of cases like $x = $x -> bdiv($y) because – in general – I don’t know whether $x is modified in-place or not. I agree that this is not ideal.

To make it worse, these method calls will modify-in-place the object's value if there's no upgrading involved, but won't modify-in-place the value if upgrading is involved, UPDATE - 2 examples that demonstrate this:

D:\>perl -Mbignum -wle "$x = 7; $x->badd(2.5); print $x;"
7

D:\>perl -Mbignum -wle "$x = 7; $x->badd(2); print $x;"
9

This is all goes on silently, does not DWIM, and requires that the programmer be alert to these issues.

Yes, I am aware of this. I believe the idea is that when you use bignum, it is to get transparent support for arbitrarily large numbers. In such cases, you don’t use method calls.

@pjacklam, if it's impractical to make any behavioural changes, then I agree that the bignum docs should prominently set out the perils of using the method calls.

I have been thinking about this, and I believe it is not as difficult to implement as I originally though. The question is how many other modules will break if I make this change. :-)

[sisyphus]

I believe the idea is that when you use bignum, it is to get transparent support for arbitrarily large numbers. In such cases, you don’t use method calls.

Yes, I expect that's the usual practice. Otherwise there would surely have been more complaints about this issue than just this one report from 14 years ago.

I have been thinking about this, and I believe it is not as difficult to implement as I originally thought. The question is how many other modules will break if I make this change. :-)

I actually think not many, if any, breakages will occur. (Easy for me to say, I know ;-) It should be a very rare case that someone does $y = $x->bdiv(2) and relies on $x not being modified. If they are relying on $x not being modified, then they need to be certain that the bdiv(2) call initiates an upgrade or downgrade of the result.

But code like $x = $x->bdiv(2) or $x = $x->bsqrt() would not be broken. (It's just that those 2 pieces of code are catching the return value, when there would no longer be any need to do that.)

(BTW, I've been allowing cross-class overloading between my Math::GMPz, Math::GMPq and Math::MPFR modules for a while now - for reasons of "fun" rather than "usefulness". Lately, I've enhanced these operations to allow "upgrading", but I don't do any "downgrading". It's all done in the XS code - so perhaps of little relevance to the task facing you. And I don't really know whether my implementation of some of the "upgrading" overloaded operations leak memory ....)

Cheers, Rob

[jkeenan]

@sisyphus, @pjacklam, were there any changes made upstream in response to the discussion in this ticket? Are there any changes we need to make in blead?

[sisyphus]

I'm not sure if this is closable or not. I'll go along with @pjacklam's call on that.

[pjacklam]

Alas, this has not been fixed yet.

[sisyphus]

One thing that has only recently penetrated my skull is that the overloaded "op=" operators ('+/=', '/=', ...., '&=', '^=', etc.) are NOT in-place operations. They take the value that is returned to them. (Over the years I had invested some time and effort in getting them to work in-place .... always without success ;-) If you replace the overloading of Math::BigInt's '+=' operator:

'/='    =>      sub { scalar $_[0] -> bdiv($_[1]); },
with
'/='    =>      sub { scalar $_[0] -> bdiv($_[1]); 42},

then every time you do $mbi += $x (where $mbi is a Math::BigInt object), you'll find that $mbi changes to an IV with a value of 42.

OTOH, the overloading of the '++' and '--' operators is truly done in place. If you replace the overloading of Math::BigInt's '++' operator:

  '++'    =>      sub { $_[0] -> binc() },
with
  '++'    =>      sub { $_[0] -> binc(); return 42 },

then you'll find that the '++' overloading still works fine, because the return value is ignored and discarded.

I now believe that the ancients, in their wisdom, deliberately designed the "op=" operators that way because they recognized that there were occasions when something like $mbi += $x should result in $mbi changing from a Math::BigInt object to a different type of object (eg Math::BigRat or Math::BigFloat). This opens the door to having $mbi += $mbf provide the exact same result as $mbf += $mbi, with it all being handled inside the overload '+=' sub, and without any additional interference.

Of course, implementing that change would probably result in a complaint from some idiot that prefers to work with a broken commutative law of addition. (Maybe best to wait until perl 7 for that type of change .... which means that we won't ever have to do anything about it.)

Similarly, there seems to be an assumption that (eg with overloading of addition) $mbi + $mbf must always return a Math::BigInt object and $mbf + $mbi must always return a Math::BigFloat object - which then also frequently breaks the commutative law of addition. Again, I've not seen such a thing mandated in any perl documentation - and if it is mandated somewhere then I would argue that documentation should be removed. To me, it makes sense to structure the overloading subs such that any overloaded operation involving a Math::BigFloat object always returns a Math::BigFloat object - even when it's Math::BigInt's or Math::BigRat's overload sub that has been called.