Rewrite bignum’s hex and oct overrides
authorFather Chrysostomos <sprout@cpan.org>
Thu, 4 Oct 2012 07:35:05 +0000 (00:35 -0700)
committerFather Chrysostomos <sprout@cpan.org>
Thu, 4 Oct 2012 16:37:58 +0000 (09:37 -0700)
As mentioned in <https://rt.cpan.org/Ticket/Display.html?id=79915>,
bigint.pm does not use any prototype when globally overriding hex.
This means that map { hex } ... will stop working in completely unre-
lated code if bigint happens to be loaded.  (Explicit $_ will con-
tinue to work.)

I thought it would be a simple matter of adding the right prototype
depending on perl version (and inferring $_), but the basic tests
I added failed for other reasons after I fixed the prototype and
$_ handling.

It turns out this whole thing is a mess, so I have basically reimple-
mented these two overrides.

What bigint, bignum and bigrat were doing was this: In import,
*CORE::GLOBAL::hex and ::oct are assigned functions that create
Math::BigInt objects if the pragma is in effect.  If import is passed
'hex' or 'oct', then the function assigned does not check the pragma
hints, but simply creates Math::BigInt objects regardless.

This means that ‘use bigrat’ stops hex() and oct() from creating
objects in ‘use bigint’ scopes, and vice versa.  In fact, whichever
pragma is loaded last wins.  Any scopes elsewhere in the program that
use the same pragma will have special hex() and oct() behaviour.  But
the other two lowercase big* pragmata will be disabled with regard to
hex and oct.

Having ‘use bigint 'hex'’ override hex globally makes no sense to me.
I have no qualms about changing it, as it was already broken.  Any
subsequent ‘use bigint;’ would turn off the global override.  So now
it exports hex or oct to the calling package, just like a normal mod-
ule.  You can now also call bigint::hex.

Also, in writing tests I found that oct("20") gives me 20.  Apparently
this was never tested properly.

I also found notes about ‘5.9.4 or later’ when the code checked
$] > 5.009004.  (Actually, in one place the code checked > 5.009003,
so I made it match, as we use the _ prototype now, which was intro-
duced in 5.9.5.)  One was in the docs, so I changed it to 5.10.0,
since  it is not helpful to mention dev versions.  The docs were also
wrong to imply that ‘no bigint’ would countermand ‘use bigint 'hex'’.

MANIFEST
dist/bignum/lib/bigint.pm
dist/bignum/lib/bignum.pm
dist/bignum/lib/bigrat.pm
dist/bignum/t/overrides.t [new file with mode: 0644]

index 6fab70f..5767392 100644 (file)
--- a/MANIFEST
+++ b/MANIFEST
@@ -3054,6 +3054,7 @@ dist/bignum/t/infnan.inc          See if bignum with inf/NaN works
 dist/bignum/t/option_a.t               See if bignum a => X works
 dist/bignum/t/option_l.t               See if bignum l => X works
 dist/bignum/t/option_p.t               See if bignum p => X works
+dist/bignum/t/overrides.t              See if global overrides behave
 dist/bignum/t/ratopt_a.t               See if bigrat a => X works
 dist/bignum/t/scope_f.t                        See if no bignum works
 dist/bignum/t/scope_i.t                        See if no bigint works
index 7f877ab..3120fdd 100644 (file)
@@ -4,7 +4,7 @@ use 5.006;
 $VERSION = '0.30';
 use Exporter;
 @ISA           = qw( Exporter );
-@EXPORT_OK     = qw( PI e bpi bexp );
+@EXPORT_OK     = qw( PI e bpi bexp hex oct );
 @EXPORT                = qw( inf NaN );
 
 use strict;
@@ -120,38 +120,67 @@ sub in_effect
 #############################################################################
 # the following two routines are for "use bigint qw/hex oct/;":
 
-sub _hex_global
+use constant LEXICAL => $] > 5.009004;
+
+{
+    my $proto = LEXICAL ? '_' : ';$';
+    eval '
+sub hex(' . $proto . ')' . <<'.';
   {
-  my $i = $_[0];
+  my $i = @_ ? $_[0] : $_;
   $i = '0x'.$i unless $i =~ /^0x/;
   Math::BigInt->new($i);
   }
-
-sub _oct_global
+.
+    eval '
+sub oct(' . $proto . ')' . <<'.';
   {
-  my $i = $_[0];
-  return Math::BigInt->from_oct($i) if $i =~ /^0[0-7]/;
+  my $i = @_ ? $_[0] : $_;
+  # oct() should never fall back to decimal
+  return Math::BigInt->from_oct($i) if $i =~ /^(?:0[0-9]|[1-9])/;
   Math::BigInt->new($i);
   }
+.
+}
 
 #############################################################################
 # the following two routines are for Perl 5.9.4 or later and are lexical
 
-sub _hex
+my ($prev_oct, $prev_hex, $overridden);
+
+if (LEXICAL) { eval <<'.' }
+sub _hex(_)
   {
-  return CORE::hex($_[0]) unless in_effect(1);
+  my $hh = (caller 0)[10];
+  return $prev_hex ? &$prev_hex($_[0]) : CORE::hex($_[0])
+    unless $$hh{bigint}||$$hh{bignum}||$$hh{bigrat};
   my $i = $_[0];
   $i = '0x'.$i unless $i =~ /^0x/;
   Math::BigInt->new($i);
   }
 
-sub _oct
+sub _oct(_)
   {
-  return CORE::oct($_[0]) unless in_effect(1);
+  my $hh = (caller 0)[10];
+  return $prev_oct ? &$prev_oct($_[0]) : CORE::oct($_[0])
+    unless $$hh{bigint}||$$hh{bignum}||$$hh{bigrat};
   my $i = $_[0];
-  return Math::BigInt->from_oct($i) if $i =~ /^0[0-7]/;
+  # oct() should never fall back to decimal
+  return Math::BigInt->from_oct($i) if $i =~ /^(?:0[0-9]|[1-9])/;
   Math::BigInt->new($i);
   }
+.
+
+sub _override
+  {
+  return if $overridden;
+  $prev_oct = *CORE::GLOBAL::oct{CODE};
+  $prev_hex = *CORE::GLOBAL::hex{CODE};
+  no warnings 'redefine';
+  *CORE::GLOBAL::oct = \&_oct;
+  *CORE::GLOBAL::hex = \&_hex;
+  $overridden++;
+  }
 
 sub import 
   {
@@ -159,12 +188,10 @@ sub import
 
   $^H{bigint} = 1;                                     # we are in effect
 
-  my ($hex,$oct);
   # for newer Perls always override hex() and oct() with a lexical version:
-  if ($] > 5.009004)
+  if (LEXICAL)
     {
-    $oct = \&_oct;
-    $hex = \&_hex;
+    _override();
     }
   # some defaults
   my $lib = ''; my $lib_kind = 'try';
@@ -205,17 +232,7 @@ sub import
       $trace = 1;
       splice @a, $j, 1; $j --;
       }
-    elsif ($_[$i] eq 'hex')
-      {
-      splice @a, $j, 1; $j --;
-      $hex = \&_hex_global;
-      }
-    elsif ($_[$i] eq 'oct')
-      {
-      splice @a, $j, 1; $j --;
-      $oct = \&_oct_global;
-      }
-    elsif ($_[$i] !~ /^(PI|e|bpi|bexp)\z/)
+    elsif ($_[$i] !~ /^(PI|e|bpi|bexp|hex|oct)\z/)
       {
       die ("unknown option $_[$i]");
       }
@@ -271,11 +288,6 @@ sub import
     {
     $self->export_to_level(1,$self,@a);           # export inf and NaN, e and PI
     }
-  {
-    no warnings 'redefine';
-    *CORE::GLOBAL::oct = $oct if $oct;
-    *CORE::GLOBAL::hex = $hex if $hex;
-  }
   }
 
 sub inf () { Math::BigInt::binf(); }
@@ -302,14 +314,14 @@ bigint - Transparent BigInteger support for Perl
   print 2 ** 512,"\n";                 # really is what you think it is
   print inf + 42,"\n";                 # inf
   print NaN * 7,"\n";                  # NaN
-  print hex("0x1234567890123490"),"\n";        # Perl v5.9.4 or later
+  print hex("0x1234567890123490"),"\n";        # Perl v5.10.0 or later
 
   {
     no bigint;
     print 2 ** 256,"\n";               # a normal Perl scalar now
   }
 
-  # Note that this will be global:
+  # Import into current package:
   use bigint qw/hex oct/;
   print hex("0x1234567890123490"),"\n";
   print oct("01234567890123490"),"\n";
@@ -397,14 +409,16 @@ Math::BigInt.
 =item hex
 
 Override the built-in hex() method with a version that can handle big
-integers. Note that under Perl v5.9.4 or ealier, this will be global
-and cannot be disabled with "no bigint;".
+integers. This overrides it by exporting it to the current package. Under
+Perl v5.10.0 and higher, this is not so necessary, as hex() is lexically
+overridden in the current scope whenever the bigint pragma is active.
 
 =item oct
 
 Override the built-in oct() method with a version that can handle big
-integers. Note that under Perl v5.9.4 or ealier, this will be global
-and cannot be disabled with "no bigint;".
+integers. This overrides it by exporting it to the current package. Under
+Perl v5.10.0 and higher, this is not so necessary, as oct() is lexically
+overridden in the current scope whenever the bigint pragma is active.
 
 =item l, lib, try or only
 
index 55489b9..32d0675 100644 (file)
@@ -4,7 +4,7 @@ use 5.006;
 $VERSION = '0.30';
 use Exporter;
 @ISA           = qw( bigint );
-@EXPORT_OK     = qw( PI e bexp bpi ); 
+@EXPORT_OK     = qw( PI e bexp bpi hex oct ); 
 @EXPORT        = qw( inf NaN ); 
 
 use strict;
@@ -17,6 +17,8 @@ BEGIN
   {
   *inf = \&bigint::inf;
   *NaN = \&bigint::NaN;
+  *hex = \&bigint::hex;
+  *oct = \&bigint::oct;
   }
 
 # These are all alike, and thus faked by AUTOLOAD
@@ -68,23 +70,6 @@ sub in_effect
   }
 
 #############################################################################
-# the following two routines are for Perl 5.9.4 or later and are lexical
-
-sub _hex
-  {
-  return CORE::hex($_[0]) unless in_effect(1);
-  my $i = $_[0];
-  $i = '0x'.$i unless $i =~ /^0x/;
-  Math::BigInt->new($i);
-  }
-
-sub _oct
-  {
-  return CORE::oct($_[0]) unless in_effect(1);
-  my $i = $_[0];
-  return Math::BigInt->from_oct($i) if $i =~ /^0[0-7]/;
-  Math::BigInt->new($i);
-  }
 
 sub import 
   {
@@ -92,13 +77,10 @@ sub import
 
   $^H{bignum} = 1;                                     # we are in effect
 
-  my ($hex,$oct);
-
   # for newer Perls override hex() and oct() with a lexical version:
-  if ($] > 5.009003)
+  if ($] > 5.009004)
     {
-    $hex = \&_hex;
-    $oct = \&_oct;
+    bigint::_override();
     }
 
   # some defaults
@@ -156,17 +138,7 @@ sub import
       $trace = 1;
       splice @a, $j, 1; $j --;
       }
-    elsif ($_[$i] eq 'hex')
-      {
-      splice @a, $j, 1; $j --;
-      $hex = \&bigint::_hex_global;
-      }
-    elsif ($_[$i] eq 'oct')
-      {
-      splice @a, $j, 1; $j --;
-      $oct = \&bigint::_oct_global;
-      }
-    elsif ($_[$i] !~ /^(PI|e|bexp|bpi)\z/)
+    elsif ($_[$i] !~ /^(PI|e|bexp|bpi|hex|oct)\z/)
       {
       die ("unknown option $_[$i]");
       }
@@ -233,11 +205,6 @@ sub import
     {
     $self->export_to_level(1,$self,@a);           # export inf and NaN
     }
-  {
-    no warnings 'redefine';
-    *CORE::GLOBAL::oct = $oct if $oct;
-    *CORE::GLOBAL::hex = $hex if $hex;
-  }
   }
 
 sub PI () { Math::BigFloat->new('3.141592653589793238462643383279502884197'); }
@@ -267,7 +234,7 @@ bignum - Transparent BigNumber support for Perl
     print 2 ** 256,"\n";               # a normal Perl scalar now
   }
 
-  # for older Perls, note that this will be global:
+  # for older Perls, import into current package:
   use bignum qw/hex oct/;
   print hex("0x1234567890123490"),"\n";
   print oct("01234567890123490"),"\n";
@@ -420,14 +387,16 @@ This will be hopefully fixed soon ;)
 =item hex
 
 Override the built-in hex() method with a version that can handle big
-integers. Note that under Perl older than v5.9.4, this will be global
-and cannot be disabled with "no bigint;".
+numbers. This overrides it by exporting it to the current package. Under
+Perl v5.10.0 and higher, this is not so necessary, as hex() is lexically
+overridden in the current scope whenever the bignum pragma is active.
 
 =item oct
 
 Override the built-in oct() method with a version that can handle big
-integers. Note that under Perl older than v5.9.4, this will be global
-and cannot be disabled with "no bigint;".
+numbers. This overrides it by exporting it to the current package. Under
+Perl v5.10.0 and higher, this is not so necessary, as oct() is lexically
+overridden in the current scope whenever the bigint pragma is active.
 
 =item v or version
 
index f3e9fa0..1333011 100644 (file)
@@ -4,7 +4,7 @@ use 5.006;
 $VERSION = '0.30';
 require Exporter;
 @ISA           = qw( bigint );
-@EXPORT_OK     = qw( PI e bpi bexp );
+@EXPORT_OK     = qw( PI e bpi bexp hex oct );
 @EXPORT                = qw( inf NaN );
 
 use strict;
@@ -17,6 +17,8 @@ BEGIN
   {
   *inf = \&bigint::inf;
   *NaN = \&bigint::NaN;
+  *hex = \&bigint::hex;
+  *oct = \&bigint::oct;
   }
 
 # These are all alike, and thus faked by AUTOLOAD
@@ -69,23 +71,6 @@ sub in_effect
   }
 
 #############################################################################
-# the following two routines are for Perl 5.9.4 or later and are lexical
-
-sub _hex
-  {
-  return CORE::hex($_[0]) unless in_effect(1);
-  my $i = $_[0];
-  $i = '0x'.$i unless $i =~ /^0x/;
-  Math::BigInt->new($i);
-  }
-
-sub _oct
-  {
-  return CORE::oct($_[0]) unless in_effect(1);
-  my $i = $_[0];
-  return Math::BigInt->from_oct($i) if $i =~ /^0[0-7]/;
-  Math::BigInt->new($i);
-  }
 
 sub import 
   {
@@ -95,12 +80,10 @@ sub import
 
   $^H{bigrat} = 1;                                     # we are in effect
 
-  my ($hex,$oct);
   # for newer Perls always override hex() and oct() with a lexical version:
   if ($] > 5.009004)
     {
-    $oct = \&_oct;
-    $hex = \&_hex;
+    bigint::_override();
     }
   # some defaults
   my $lib = ''; my $lib_kind = 'try'; my $upgrade = 'Math::BigFloat';
@@ -148,17 +131,7 @@ sub import
       $trace = 1;
       splice @a, $j, 1; $j --;
       }
-    elsif ($_[$i] eq 'hex')
-      {
-      splice @a, $j, 1; $j --;
-      $hex = \&bigint::_hex_global;
-      }
-    elsif ($_[$i] eq 'oct')
-      {
-      splice @a, $j, 1; $j --;
-      $oct = \&bigint::_oct_global;
-      }
-    elsif ($_[$i] !~ /^(PI|e|bpi|bexp)\z/)
+    elsif ($_[$i] !~ /^(PI|e|bpi|bexp|hex|oct)\z/)
       {
       die ("unknown option $_[$i]");
       }
@@ -219,11 +192,6 @@ sub import
     {
     $self->export_to_level(1,$self,@a);           # export inf and NaN
     }
-  {
-    no warnings 'redefine';
-    *CORE::GLOBAL::oct = $oct if $oct;
-    *CORE::GLOBAL::hex = $hex if $hex;
-  }
   }
 
 sub PI () { Math::BigFloat->new('3.141592653589793238462643383279502884197'); }
@@ -257,7 +225,7 @@ bigrat - Transparent BigNumber/BigRational support for Perl
     print 1/3,"\n";                    # 0.33333...
   }
 
-  # Note that this will make hex() and oct() be globally overridden:
+  # Import into current package:
   use bigrat qw/hex oct/;
   print hex("0x1234567890123490"),"\n";
   print oct("01234567890123490"),"\n";
@@ -486,14 +454,16 @@ This will be hopefully fixed soon ;)
 =item hex
 
 Override the built-in hex() method with a version that can handle big
-integers. Note that under Perl v5.9.4 or ealier, this will be global
-and cannot be disabled with "no bigint;".
+numbers. This overrides it by exporting it to the current package. Under
+Perl v5.10.0 and higher, this is not so necessary, as hex() is lexically
+overridden in the current scope whenever the bigrat pragma is active.
 
 =item oct
 
 Override the built-in oct() method with a version that can handle big
-integers. Note that under Perl v5.9.4 or earlier, this will be global
-and cannot be disabled with "no bigint;".
+numbers. This overrides it by exporting it to the current package. Under
+Perl v5.10.0 and higher, this is not so necessary, as oct() is lexically
+overridden in the current scope whenever the bigrat pragma is active.
 
 =item v or version
 
diff --git a/dist/bignum/t/overrides.t b/dist/bignum/t/overrides.t
new file mode 100644 (file)
index 0000000..d807228
--- /dev/null
@@ -0,0 +1,100 @@
+#!perl -w
+
+# Test behaviour of hex and oct overrides in detail, and also how the three
+# modules interact.
+
+use Test::More tests => 35;
+
+# For testing that existing CORE::GLOBAL overrides are not clobbered
+BEGIN
+  {
+  if ($] > 5.009004)
+    {
+    no warnings 'syntax';
+    *CORE::GLOBAL::hex = sub(_) { ++$hex_called; CORE::hex(@_?$_[0]:$_) };
+    *CORE::GLOBAL::oct = sub(_) { ++$oct_called; CORE::oct(@_?$_[0]:$_) };
+    }
+  else
+    {
+    *CORE::GLOBAL::hex = sub(;$) { ++$hex_called; CORE::hex(@_?$_[0]:$_) };
+    *CORE::GLOBAL::oct = sub(;$) { ++$oct_called; CORE::oct(@_?$_[0]:$_) };
+    }
+  }
+
+{
+  use bigint;
+  $_ = "20";
+  is hex, "32", 'bigint hex override without arguments infers $_';
+  is oct, "16", 'bigint oct override without arguments infers $_';
+  @_ = 1..20;
+  is hex(@_), "32", 'bigint hex override provides scalar context';
+  is oct(@_), "16", 'bigint oct override provides scalar context';
+  is ref hex(1), 'Math::BigInt',
+    'bigint hex() works when bignum and bigrat are loaded';
+  is ref oct(1), 'Math::BigInt',
+    'bigint oct() works when bignum and bigrat are loaded';
+}
+{
+  use bignum;
+  $_ = "20";
+  is hex, "32", 'bignum hex override without arguments infers $_';
+  is oct, "16", 'bignum oct override without arguments infers $_';
+  @_ = 1..20;
+  is hex(@_), "32", 'bignum hex override provides scalar context';
+  is oct(@_), "16", 'bignum oct override provides scalar context';
+  is ref hex(1), 'Math::BigInt',
+    'bignum hex() works when bigint and bigrat are loaded';
+  is ref oct(1), 'Math::BigInt',
+    'bignum oct() works when bigint and bigrat are loaded';
+}
+{
+  use bigrat;
+  $_ = "20";
+  is hex, "32", 'bigrat hex override without arguments infers $_';
+  is oct, "16", 'bigrat oct override without arguments infers $_';
+  @_ = 1..20;
+  is hex(@_), "32", 'bigrat hex override provides scalar context';
+  is oct(@_), "16", 'bigrat oct override provides scalar context';
+  is ref hex(1), 'Math::BigInt',
+    'bigrat hex() works when bignum and bigint are loaded';
+  is ref oct(1), 'Math::BigInt',
+    'bigrat oct() works when bignum and bigint are loaded';
+}
+
+$hex_called = 0;
+() = hex 0;
+is $hex_called, 1, 'existing hex overrides are called';
+$oct_called = 0;
+() = oct 0;
+is $oct_called, 1, 'existing oct overrides are called';
+
+{
+  package _importer;
+  {
+    use bigint 'hex', 'oct';
+    ::is \&hex, \&bigint::hex, 'exported hex function';
+    ::is \&oct, \&bigint::oct, 'exported oct function';
+  }
+  ::ok ref hex(), 'exported hex function returns ref outside pragma scope';
+  ::ok ref oct(), 'exported oct function returns ref outside pragma scope';
+  ::is oct("20"), "16", 'exported oct function works with "decimal"';
+    # (used to return 20 because it thought it was decimal)
+}
+{
+  package _importer2;
+  use bignum 'hex', 'oct';
+  ::is \&hex, \&bignum::hex, 'bignum exports hex';
+  ::is \&oct, \&bignum::oct, 'bignum exports oct';
+  ::is \&hex, \&bigint::hex, 'bignum exports same hex as bigint';
+  ::is \&oct, \&bigint::oct, 'bignum exports same oct as bigint';
+}
+{
+  package _importer3;
+  use bigrat 'hex', 'oct';
+  ::is \&hex, \&bigrat::hex, 'bigrat exports hex';
+  ::is \&oct, \&bigrat::oct, 'bigrat exports oct';
+  ::is \&hex, \&bigint::hex, 'bigrat exports same hex as bigint';
+  ::is \&oct, \&bigint::oct, 'bigrat exports same oct as bigint';
+}
+is ref hex 0, "", 'hex export is not global';
+is ref oct 0, "", 'oct export is not global';