Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bignum >= 0.51 breaks BigInt >> #21370

Open
daniel-pfeiffer opened this issue Aug 13, 2023 · 13 comments
Open

bignum >= 0.51 breaks BigInt >> #21370

daniel-pfeiffer opened this issue Aug 13, 2023 · 13 comments

Comments

@daniel-pfeiffer
Copy link

This is a bug report for perl from [email protected],
generated with the help of perlbug 1.42 running under perl 5.36.0.


Devuan Daedalus (testing, based on Debian Bookworm 12), comes with two installed Perl versions. Both break Math::BigInt badly, turning an integer right shift one into a floating division by two. While they both seem to be using their Perl's respective version of bignum, they seem to have a bleeding edge Math::BigInt, slightly ahead even of Perl 5.38:

Perl 5.32.1 bignum 0.51

$ /usr/bin/perl5.32-x86_64-linux-gnu -Mbignum -e 'use 5.32.1;
    say for $bignum::VERSION, $Math::BigInt::VERSION, Math::BigInt->new(3) >> 1, 3 >> 1'
0.51
1.999838
1.5
1.5

$ /usr/bin/perl5.32-x86_64-linux-gnu -M'bignum upgrade => undef' -e 'use 5.32.1;
    say for Math::BigInt->new(3) >> 1, 3 >> 1'
1
1

$ /usr/bin/perl5.32-x86_64-linux-gnu -MMath::BigInt -e 'use 5.32.1;
    say for $Math::BigInt::VERSION, Math::BigInt->new(3) >> 1'
1.999838
1

Perl 5.36.0 bignum 0.65

$ perl -Mbignum -e 'use 5.36.0;
    say for $bignum::VERSION, $Math::BigInt::VERSION, Math::BigInt->new(3) >> 1, 3 >> 1'
0.65
1.999838
1.5
1.5

$ perl -M'bignum upgrade => undef' -e 'use 5.36.0;
    say for Math::BigInt->new(3) >> 1, 3 >> 1'
1
1

$ perl -MMath::BigInt -e 'use 5.36.0;
    say for $Math::BigInt::VERSION, Math::BigInt->new(3) >> 1'
1.999838
1


Flags:
category=library
severity=medium
module=bignum

Site configuration information for perl 5.36.0:

Configured by Debian at Sun Jan 8 21:28:47 UTC 2023.

Summary of my perl5 (revision 5 version 36 subversion 0) configuration:

Platform:
osname=linux
osvers=4.19.0
archname=x86_64-linux-gnu-thread-multi
uname='linux localhost 4.19.0 #1 smp debian 4.19.0 x86_64 gnulinux '
config_args='-Dmksymlinks -Dusethreads -Duselargefiles -Dcc=x86_64-linux-gnu-gcc -Dcpp=x86_64-linux-gnu-cpp -Dld=x86_64-linux-gnu-gcc -Dccflags=-DDEBIAN -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -ffile-prefix-map=/dummy/build/dir=. -fstack-protector-strong -Wformat -Werror=format-security -Dldflags= -Wl,-z,relro -Dlddlflags=-shared -Wl,-z,relro -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.36 -Darchlib=/usr/lib/x86_64-linux-gnu/perl/5.36 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/x86_64-linux-gnu/perl5/5.36 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.36.0 -Dsitearch=/usr/local/lib/x86_64-linux-gnu/perl/5.36.0 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Duse64bitint -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -Ui_libutil -Ui_xlocale -Uversiononly -DDEBUGGING=-g -Doptimize=-O2 -dEs -Duseshrplib -Dlibperl=libperl.so.5.36.0'
hint=recommended
useposix=true
d_sigaction=define
useithreads=define
usemultiplicity=define
use64bitint=define
use64bitall=define
uselongdouble=undef
usemymalloc=n
default_inc_excludes_dot=define
Compiler:
cc='x86_64-linux-gnu-gcc'
ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
optimize='-O2 -g'
cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include'
ccversion=''
gccversion='12.2.0'
gccosandvers=''
intsize=4
longsize=8
ptrsize=8
doublesize=8
byteorder=12345678
doublekind=3
d_longlong=define
longlongsize=8
d_longdbl=define
longdblsize=16
longdblkind=3
ivtype='long'
ivsize=8
nvtype='double'
nvsize=8
Off_t='off_t'
lseeksize=8
alignbytes=8
prototype=define
Linker and Libraries:
ld='x86_64-linux-gnu-gcc'
ldflags =' -fstack-protector-strong -L/usr/local/lib'
libpth=/usr/local/lib /usr/lib/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib
libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
perllibs=-ldl -lm -lpthread -lc -lcrypt
libc=/lib/x86_64-linux-gnu/libc.so.6
so=so
useshrplib=true
libperl=libperl.so.5.36
gnulibc_version='2.36'
Dynamic Linking:
dlsrc=dl_dlopen.xs
dlext=so
d_dlsymun=undef
ccdlflags='-Wl,-E'
cccdlflags='-fPIC'
lddlflags='-shared -L/usr/local/lib -fstack-protector-strong'

Locally applied patches:
DEBPKG:debian/cpan_definstalldirs - Provide a sensible INSTALLDIRS default for modules installed from CPAN.
DEBPKG:debian/db_file_ver - https://bugs.debian.org/340047 Remove overly restrictive DB_File version check.
DEBPKG:debian/doc_info - Replace generic man(1) instructions with Debian-specific information.
DEBPKG:debian/enc2xs_inc - https://bugs.debian.org/290336 Tweak enc2xs to follow symlinks and ignore missing @inc directories.
DEBPKG:debian/errno_ver - https://bugs.debian.org/343351 Remove Errno version check due to upgrade problems with long-running processes.
DEBPKG:debian/libperl_embed_doc - https://bugs.debian.org/186778 Note that libperl-dev package is required for embedded linking
DEBPKG:fixes/respect_umask - Respect umask during installation
DEBPKG:debian/writable_site_dirs - Set umask approproately for site install directories
DEBPKG:debian/extutils_set_libperl_path - EU:MM: set location of libperl.a under /usr/lib
DEBPKG:debian/no_packlist_perllocal - Don't install .packlist or perllocal.pod for perl or vendor
DEBPKG:debian/fakeroot - Postpone LD_LIBRARY_PATH evaluation to the binary targets.
DEBPKG:debian/instmodsh_doc - Debian policy doesn't install .packlist files for core or vendor.
DEBPKG:debian/ld_run_path - Remove standard libs from LD_RUN_PATH as per Debian policy.
DEBPKG:debian/libnet_config_path - Set location of libnet.cfg to /etc/perl/Net as /usr may not be writable.
DEBPKG:debian/perlivp - https://bugs.debian.org/510895 Make perlivp skip include directories in /usr/local
DEBPKG:debian/squelch-locale-warnings - https://bugs.debian.org/508764 Squelch locale warnings in Debian package maintainer scripts
DEBPKG:debian/patchlevel - https://bugs.debian.org/567489 List packaged patches for 5.36.0-7 in patchlevel.h
DEBPKG:fixes/document_makemaker_ccflags - https://bugs.debian.org/628522 [rt.cpan.org #68613] Document that CCFLAGS should include $Config{ccflags}
DEBPKG:debian/find_html2text - https://bugs.debian.org/640479 Configure CPAN::Distribution with correct name of html2text
DEBPKG:debian/perl5db-x-terminal-emulator.patch - https://bugs.debian.org/668490 Invoke x-terminal-emulator rather than xterm in perl5db.pl
DEBPKG:debian/cpan-missing-site-dirs - https://bugs.debian.org/688842 Fix CPAN::FirstTime defaults with nonexisting site dirs if a parent is writable
DEBPKG:fixes/memoize_storable_nstore - [rt.cpan.org #77790] https://bugs.debian.org/587650 Memoize::Storable: respect 'nstore' option not respected
DEBPKG:debian/makemaker-pasthru - https://bugs.debian.org/758471 Pass LD settings through to subdirectories
DEBPKG:debian/makemaker-manext - https://bugs.debian.org/247370 Make EU::MakeMaker honour MANnEXT settings in generated manpage headers
DEBPKG:debian/kfreebsd-softupdates - https://bugs.debian.org/796798 Work around Debian Bug#796798
DEBPKG:fixes/memoize-pod - [rt.cpan.org #89441] Fix POD errors in Memoize
DEBPKG:debian/hurd-softupdates - https://bugs.debian.org/822735 Fix t/op/stat.t failures on hurd
DEBPKG:fixes/math_complex_doc_great_circle - https://bugs.debian.org/697567 [rt.cpan.org #114104] Math::Trig: clarify definition of great_circle_midpoint
DEBPKG:fixes/math_complex_doc_see_also - https://bugs.debian.org/697568 [rt.cpan.org #114105] Math::Trig: add missing SEE ALSO
DEBPKG:fixes/math_complex_doc_angle_units - https://bugs.debian.org/731505 [rt.cpan.org #114106] Math::Trig: document angle units
DEBPKG:fixes/cpan_web_link - https://bugs.debian.org/367291 CPAN: Add link to main CPAN web site
DEBPKG:debian/hppa_op_optimize_workaround - https://bugs.debian.org/838613 Temporarily lower the optimization of op.c on hppa due to gcc-6 problems
DEBPKG:debian/installman-utf8 - https://bugs.debian.org/840211 Generate man pages with UTF-8 characters
DEBPKG:debian/hppa_opmini_optimize_workaround - https://bugs.debian.org/869122 Lower the optimization level of opmini.c on hppa
DEBPKG:debian/sh4_op_optimize_workaround - https://bugs.debian.org/869373 Also lower the optimization level of op.c and opmini.c on sh4
DEBPKG:debian/perldoc-pager - https://bugs.debian.org/870340 [rt.cpan.org #120229] Fix perldoc terminal escapes when sensible-pager is less
DEBPKG:debian/prune_libs - https://bugs.debian.org/128355 Prune the list of libraries wanted to what we actually need.
DEBPKG:debian/mod_paths - Tweak @inc ordering for Debian
DEBPKG:debian/deprecate-with-apt - https://bugs.debian.org/747628 Point users to Debian packages of deprecated core modules
DEBPKG:debian/disable-stack-check - https://bugs.debian.org/902779 [GH #16607] Disable debugperl stack extension checks for binary compatibility with perl
DEBPKG:debian/perlbug-editor - https://bugs.debian.org/922609 Use "editor" as the default perlbug editor, as per Debian policy
DEBPKG:debian/eu-mm-perl-base - https://bugs.debian.org/962138 Suppress an ExtUtils::MakeMaker warning about our non-default @inc
DEBPKG:fixes/io_socket_ip_ipv6 - Disable getaddrinfo(3) AI_ADDRCONFIG for localhost and IPv4 numeric addresses
DEBPKG:debian/usrmerge-lib64 - https://bugs.debian.org/914128 Configure / libpth.U: Do not adjust glibpth when /usr/lib64 is present.
DEBPKG:debian/usrmerge-realpath - https://bugs.debian.org/914128 Configure / libpth.U: use realpath --no-symlinks on Debian
DEBPKG:debian/configure-regen - https://bugs.debian.org/762638 Regenerate Configure et al. after probe unit changes
DEBPKG:fixes/x32-io-msg-skip - https://bugs.debian.org/922609 Skip io/msg.t on x32 due to broken System V message queues
DEBPKG:debian/hurd-eumm-workaround - https://bugs.debian.org/1018289 Work around a MakeMaker regression breaking GNU/Hurd hint files
DEBPKG:fixes/json-pp-warnings - https://bugs.debian.org/1019757 Call unimport first to silence warnings
DEBPKG:fixes/readline-stream-errors - [80c1f1e] [GH #6799] https://bugs.debian.org/1016369 only clear the stream error state in readline() for glob()
DEBPKG:fixes/readline-stream-errors-test - [0b60216] [GH #6799] https://bugs.debian.org/1016369 test that <> doesn't clear the stream error state
DEBPKG:fixes/lto-test-fix - [69b4fa3] [GH #20518] https://bugs.debian.org/1015579 skip checking categorization of libperl symbols for LTO builds


@inc for perl 5.36.0:
/etc/perl
/usr/local/lib/x86_64-linux-gnu/perl/5.36.0
/usr/local/share/perl/5.36.0
/usr/lib/x86_64-linux-gnu/perl5/5.36
/usr/share/perl5
/usr/lib/x86_64-linux-gnu/perl-base
/usr/lib/x86_64-linux-gnu/perl/5.36
/usr/share/perl/5.36
/usr/local/lib/site_perl


Environment for perl 5.36.0:
HOME=/home/pfeiffer
LANG=eo.utf8
LANGUAGE (unset)
LC_ALL=eo.utf8
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/home/pfeiffer/.cargo/bin:/home/pfeiffer/bin:/home/software/eclipse:/usr/local/sbin:/home/software/jdk/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/bin:/bin
PERL_BADLANG (unset)
SHELL=/bin/bash

@hvds
Copy link
Contributor

hvds commented Aug 13, 2023

This sounds like it may be connected to earlier ticket #19539; see also related danaj/Math-Prime-Util#61 and bignum#141772.

The comment from Math::BigInt author @pjacklam here may be of particular relevance.

@jkeenan
Copy link
Contributor

jkeenan commented Aug 13, 2023

Consider these two programs:

$ cat gh-21370-BigFloat.pl 
#!/usr/bin/env perl
use 5.14.0;
use warnings;

#use bignum;
use Math::BigInt;

say "perl VERSION:\t\t", $^V;
#say "bignum VERSION:\t\t", $bignum::VERSION;
say "Math::BigInt VERSION:\t", $Math::BigInt::VERSION;
say '';
say Math::BigInt->new(3) >> 1, 3 >> 1;
say Math::BigInt->new(3) >> 1, '|', 3 >> 1;

and

$ cat gh-21370-bignum.pl
#!/usr/bin/env perl
use 5.14.0;
use warnings;

use bignum;
use Math::BigInt;

say "perl VERSION:\t\t", $^V;
say "bignum VERSION:\t\t", $bignum::VERSION;
say "Math::BigInt VERSION:\t", $Math::BigInt::VERSION;
say '';
say Math::BigInt->new(3) >> 1, 3 >> 1;
say Math::BigInt->new(3) >> 1, '|', 3 >> 1;

... which differ only in that the second has use bignum. Now let's run them in the two most recent production releases of perl.

perl-5.36.0

$ perlbrew use perl-5.36.0
$ perl gh-21370-BigFloat.pl 
perl VERSION:		v5.36.0
Math::BigInt VERSION:	1.999830

11
1|1

$ perl gh-21370-bignum.pl
perl VERSION:		v5.36.0
bignum VERSION:		0.65
Math::BigInt VERSION:	1.999830

11
1|1

perl-5.38.0

$ perlbrew use perl-5.38.0
$ perl gh-21370-BigFloat.pl 
perl VERSION:		v5.38.0
Math::BigInt VERSION:	1.999837

11
1|1

$ perl gh-21370-bignum.pl
perl VERSION:		v5.38.0
bignum VERSION:		0.66
Math::BigInt VERSION:	1.999837

1.51.5
1.5|1.5

@daniel-pfeiffer, does this accurately represent the problem you are experiencing?

@pjacklam
Copy link
Contributor

I have looked at this, and unfortunately it is not clear to me what the problem is. I would be grateful for an example showing what is expected vs. how it is working.

When I took over maintenance, the bignum pragma, as well as the upgrading/downgrading functionality in Math::BigInt and Math::BigFloat, were poorly documented and had an inconsistent behaviour. I was uncertain about how it was supposed to be working. Unfortunately, there was some back and forth until I was able to make it work as I believe it was intended by the original author. Anyway, here is how it works (or is supposed to work) now:

The bignum pragma converts every numerical literal into a Math::BigInt or Math::BigFloat, depending on whether the literal represents an integer (or Inf or NaN) or non-integer, respectively.

The bignum pragma enables upgrading and downgrading, so any function or operand giving a non-integer result, returns a Math::BigFloat. This applies also when the operands are Math::BigInt objects. An example of this is

$ perl -Mbignum -wle 'print 3 / 2'
1.5

And vice versa: Any function or operand giving an integer (or Inf or NaN) returns a Math::BigInt. This applies also when the operands are Math::BigFloat objects. An example of this is

$ perl -Mbignum -wle 'print 1.5 + 1.5'
3

Earlier versions of bignum/Math::BigInt/Math::BigFloat were inconsistent: Some functions and operators would upgrade whereas others wouldn't, even in equivalent cases. Some functions would upgrade only in certain cases, but not in other, similar cases. Ditto for downgrading. The whole thing seemed rather arbitrary. At least it should be consistent now.

@sisyphus
Copy link
Contributor

sisyphus commented Aug 14, 2023

The bignum pragma enables upgrading and downgrading, so any function or operand giving a non-integer result, returns a Math::BigFloat. This applies also when the operands are Math::BigInt objects. An example of this is
$ perl -Mbignum -wle 'print 3 / 2'
1.5

There are probably sound arguments that support that behaviour, but I think @daniel-pfeiffer is complaining about the following behaviour:

>perl -Mbignum -le "print 7 >> 1;"
3.5

"7" is an integer, and so is 7 >> 1, so there's a reasonable expectation (IMO) that the result should be (BigInt) 3, not (BigFloat) 3.5.

EDIT: I'm running bignum-0.66 and Math-BigInt-1.999839 on perl-5.38.0.

Cheers,
Rob

@pjacklam
Copy link
Contributor

There are probably sound arguments that support that behaviour, but I think @daniel-pfeiffer is complaining about the following behaviour:

>perl -Mbignum -le "print 7 >> 1;"
3.5

"7" is an integer, and so is 7 >> 1, so there's a reasonable expectation (IMO) that the result should be (BigInt) 3, not (BigFloat) 3.5.

Ah, I see. The simple solution is to use bigint:

$ perl -Mbigint -le "print 7 >> 1;"
3

When using bignum, 7 >> 1 only returns 3.5 because bignum upgrades the operands from BigInts to BigFloats. So I guess the essential questions here are:

Should >> upgrade? Is >> an operator that is inherently limited to integers (i.e., like the binary |, &, ^, and unary ~)?

The >> operator simply calls brsft(). When brsft() was discussed some years ago, my impression was that the majority wanted brsft() to do integer right shift regardless of the class of the operands. So I modified brsft() accordingly. At that point 7 >> 1 returned 3, even when the operands were BigFloats. As a result, other people were furious, arguing that brsft() is essentially just a division by a power of 2, which is not at all limited to integers. So I changed it back. These changes to brsft() directly affected >>.

I see good arguments both for letting >> always treat its operands as integers, and for letting >> be a "true" division.

EDIT: I'm running bignum-0.66 and Math-BigInt-1.999839 on perl-5.38.0.

From (and including) Math-BigInt-1.999832, 7 >> 1 gives 3.5 with all version of bignum back to (and including) the ancient bignum-0.05.

@daniel-pfeiffer
Copy link
Author

daniel-pfeiffer commented Aug 14, 2023

I stumbled across this when regenerating the pl website. For the examples with a ▶ play button (that pops up the output) I mostly just pluck the code from .pod and run it. Last time I generated the page months ago (not sure which bignum/BigInt/perl version Devuan had back then) this worked.

This weekend the 3rd example for Collatz was suddenly failing – with to_base refusing to format a float with a riddle message “the value to convert must be a finite, non-negative integer.” So I explored and found I can turn off upgrading (hours wasted…)

Now I find shifting floats an amusing edge case, I had never even considered. But shifting ints should never ever upgrade! That totally breaks the semantic of shifting, with bignum taking a nanny attitude of “what you really want is division.” Well no thank you, I have a / on my keyboard, if that were what I really want.

I wasn't aware of bigint, sounds like a good workaround. It doesn't document to_base – anybody aware which minimum Perl version introduced that? (I have 5.18.2 without, and 5.28.1with – so it must have been introduced between these.) Edit: it seems that Devuan has muddled include pathes, partially specific to the Perl version, but Math is shared from /usr/share/perl5, so it doesn't reflect upstream versions.

@sisyphus
Copy link
Contributor

Ah, I see. The simple solution is to use bigint

I was thinking a better solution would be to have the overloaded >> operator perform an actual right shift if ref($num) eq 'Math::BigInt' and do a division only if ref($num) eq 'Math::BigFloat'.
That way you can still use bignum && have the >> operator DWIM for both BigInts and BigFloats.
(Of course, I have no idea how difficult that might be to implement, though it does sound pretty easy ;-)

But then .... maybe that's not such a good idea.
As $num jumps back and forth between BigInt and BigFloat (as can happen) people will probably go nuts trying to keep track of whether the >> operators are doing right shifts or divide-by-twos.

Cheers,
Rob

@pjacklam
Copy link
Contributor

I was thinking a better solution would be to have the overloaded >> operator perform an actual right shift if ref($num) eq 'Math::BigInt' and do a division only if ref($num) eq 'Math::BigFloat'. That way you can still use bignum && have the >> operator DWIM for both BigInts and BigFloats.

I fear that will be confusing. The Perl documentation (perldoc perlop) says

Binary ">>" returns the value of its left argument shifted right by the
number of bits specified by the right argument. Arguments should be
integers. (See also "Integer Arithmetic".)

so I think the best solution is to let << and >> do bitwise left and right shift, respectively, regardless of the class of the operands. An operand that is a Math::BigFloat or Math::BigRat will be truncated to an integer before the right/left shift is performed. This is consistent with the Perl documentation and the idea behind bignum which is, I believe, to add transparent support for arbitrarily large numbers.

The methods blsft() and brsft() should remain as they are and essentially be shorthands for multiplication and division, respectively. So $x -> brsft($y, $b) will remain equivalent to $x -> bdiv($b -> bpow($y)) and $x -> blsft($y, $b) will remain equivalent to $x -> bmul($b -> bpow($y)). Both blsft() and brsft() will upgrade and downgrade as necessary.

Currently, << is implemented by calling blsft() directly, and >> is implemented by calling brsft() directly, but his has to change.

Does this sound like a good idea?

@sisyphus
Copy link
Contributor

sisyphus commented Aug 19, 2023 via email

@pjacklam
Copy link
Contributor

I think the best solution is to let << and >> do bitwise left and right shift, respectively, regardless of the class of the operands. An operand that is a Math::BigFloat or Math::BigRat will be truncated to an integer before the right/left shift is performed. This is consistent with the Perl documentation and the idea behind bignum which is, I believe, to add transparent support for big numbers.

IIUC, this would mean that, under bignum the condition $n >> $s == int($n) >> $s is always true. That sounds fine to me.

Actually, since both operands might be non-integers, it is the condition $n >> $s == int($n) >> int($s) that will always be true. This is just like core Perl, where bitwise operators truncate their operands to integers.

Was that something you had tried to implement previously, only for it to be met with resistance ?

I wanted to create the functionality we are discussing here, i.e., that >> truncates its operands to integers, just like core Perl. Now, since >> is implemented simply by calling brsft(), what I did was to modify brsft() so it gave the functionality I wanted. This was a mistake. What I should have done, was to create a new subroutine (or method) for >> and let brsft() be untouched.

The methods blsft() and brsft() should remain as they are and essentially be shorthands for multiplication and division, respectively. So $x -> brsft($y, $b) will remain equivalent to $x -> bdiv($b -> bpow($y)) and $x -> blsft($y, $b) will remain equivalent to $x -> bmul($b -> bpow($y)). Both blsft() and brsft() will upgrade and downgrade as necessary. Currently, << is implemented by calling blsft() directly, and >> is implemented by calling brsft() directly, but his has to change.

I probably haven't quite got my head around that. For a Math::BigInt $x: 1) will $x >> $s still be equivalent to $x->brsft($s) ? && 2) will $x << $s still be equivalent to $x->blsft($s) ? `

My idea is to let >> and << be bitwise shift operators. They will truncate any non-integer operand to integer and never upgrade. This is how core Perl does it, so it will be in accordance with the idea that bignum introduces transparent support for big numbers.

So when bigint is in effect, $x >> $s will be equivalent to $x -> brsft($s) and $x << $s will be equivalent to $x -> blsft($s). However, when bignum is in effect, brsft() and blsft() might upgrade, as they do now, but << and >> will remain unchanged and behave like << and >> in core Perl: They will truncate their operands to integers and never upgrade.

@sisyphus
Copy link
Contributor

sisyphus commented Aug 20, 2023

This is how core Perl does it, so it will be in accordance with the idea that bignum introduces transparent support for big numbers.

Yep - I can't think of anything better than following perl's lead on this.

I might even change Math::MPFR's overloading of << and >> to do the same. (I doubt anyone will notice ;-)
Could you bump this thread when these changes have been implemented and made available for testing and/or installation ?
That way I shouldn't miss it.

Cheers,
Rob

@pjacklam
Copy link
Contributor

Could you bump this thread when these changes have been implemented and made available for testing and/or installation?
That way I shouldn't miss it.

If I remember, I will post it here, when I have something working.

While I am at it, I will also fix some very old bugs in blsft() and brsft(). For example, the following should return Inf, not NaN:

$ perl -MMath::BigInt -wle 'print Math::BigInt -> blsft(1, "Inf")'
NaN

and the following should return 0, not NaN:

$ perl -MMath::BigInt -wle 'print Math::BigInt -> brsft(1, "Inf")'
NaN

@hvds
Copy link
Contributor

hvds commented Aug 20, 2023

Was that something you had tried to implement previously, only for it to be met with resistance ?

I wanted to create the functionality we are discussing here, i.e., that >> truncates its operands to integers, just like core Perl. Now, since >> is implemented simply by calling brsft(), what I did was to modify brsft() so it gave the functionality I wanted. This was a mistake. What I should have done, was to create a new subroutine (or method) for >> and let brsft() be untouched.

Ah, I hadn't understood this detail of the history. If the pushback was indeed primarily because of changes to brsft() rather than changes to >>, then your proposed solution sounds ideal to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants