Skip Menu |

This queue is for tickets about the Math-BigInt CPAN distribution.

Report information
The Basics
Id: 25144
Status: open
Worked: 10 min
Priority: 0/
Queue: Math-BigInt

People
Owner: TELS [...] cpan.org
Requestors: mschwern [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 1.79
Fixed in: (no value)



Subject: [PATCH] Math::BigFloat->new considers any reference a BigInt.
If you pass in a reference to Math::BigFloat->new() it assumes its a BigInt object. This assumption breaks overloaded objects. $bigfloat = Math::BigFloat->new($overloaded_number); It thinks its a BigInt and tries to call as_number() on it. The attached patch tests for and fixes this.
Subject: new_overloaded.patch
Auto-merging (0, 27642) /local/Math-BigInt to /vendor/Math-BigInt (base /vendor/Math-BigInt:27640). A t/new_overloaded.t U lib/Math/BigFloat.pm ==== Patch <-> level 1 Source: 9c88509d-e914-0410-b01c-b9530614cbfe:/local/Math-BigInt:27642 Target: 9c88509d-e914-0410-b01c-b9530614cbfe:/vendor/Math-BigInt:27640 Log: r27641@windhund: schwern | 2007-02-25 12:16:14 -0800 Local copy r27642@windhund: schwern | 2007-02-25 12:30:13 -0800 Math::BigFloat->new would consider any reference to be a BigInt. This broke overloaded non-BigInts. === t/new_overloaded.t ================================================================== --- t/new_overloaded.t (revision 27640) +++ t/new_overloaded.t (patch - level 1) @@ -0,0 +1,32 @@ +#!/usr/bin/perl -w + +# Math::BigFloat->new had a bug where it would assume any object is a +# BigInt which broke overloaded non-BigInts. + +use Test::More tests => 4; + + +package Overloaded::Num; + +use overload '0+' => sub { ${$_[0]} }, + fallback => 1; +sub new { + my($class, $num) = @_; + return bless \$num, $class; +} + + +package main; + +use Math::BigFloat; + +my $overloaded_num = Overloaded::Num->new(2.23); +is $overloaded_num, 2.23; + +my $bigfloat = Math::BigFloat->new($overloaded_num); +is $bigfloat, 2.23, 'BigFloat->new accepts overloaded numbers'; + +my $bigint = Math::BigInt->new(Overloaded::Num->new(3)); +is $bigint, 3, 'BigInt->new accepts overloaded numbers'; + +is( Math::BigFloat->new($bigint), 3, 'BigFloat from BigInt' ); === lib/Math/BigFloat.pm ================================================================== --- lib/Math/BigFloat.pm (revision 27640) +++ lib/Math/BigFloat.pm (patch - level 1) @@ -127,7 +127,7 @@ my $self = {}; bless $self, $class; # shortcut for bigints and its subclasses - if ((ref($wanted)) && (ref($wanted) ne $class)) + if ((ref($wanted)) && eval { $wanted->isa("Math::BigInt") }) { $self->{_m} = $wanted->as_number()->{value}; # get us a bigint copy $self->{_e} = $MBI->_zero(); ==== BEGIN SVK PATCH BLOCK ==== Version: svk v2.0.0 (darwin) eJyNVN9v21QULhLiwbzxS0IMcdk82oq6ude/GjssC6hN2q7tKtGNhw1Z19fHjZljB/9oqOpOJE5W xAMvSLzuiRf+Gf6U/QscO20ZotUqJcqN7znfOd/x951uctDqsKLdpoXMaPHt4we2vc8z0b/L9EI2 C/CCLE5kowjhCEJZK8L4UNaLiA8Ab9M4T0R1yHhyCFl1CMQzyNpthnDNOdxGDXEBW6O6PIujVLZq eCdLAGRWmJ21oqNVH0dmVpEC3tSwTgJHQRrEEbahrpm6iiEYzzA/HkLkJHGcXV6pVTYtRBin4FTw FaRRxasyUqoTvCABgT0d49OsRrrIrgP1KwKReOD+J7Ru0zgv9Doiwx46ZgWkyc2Ce57jByFU48ga EYyc+AiSMOYeeKtVcdWR1UJh1+Opc7z6lg+H4bGTwU+ZB2HGqxqOI+tNC0e9t7CwMBPjDyZbPz/v Pv1ra/iiO35nM56tj2/9vjP2f8Hfcn18sP7FeGMMf+D3t/XJJ5Pu5P3ywfj2tDf+au+js+34xf54 ZUP+dX/y42S9/Lg3CafrZHLw4fjhJOmVn26W97ulWu7dOtuNzrbL5mS37E565XePyrcn2+Vg1pu+ tTntOdP3ysflVncajbem2cZns96T2Z3PG3maNNwgagyRP1FGknSH7PKsb9vfBIddnEimtHE+pM89 wombH5JRHxIgQUZGcR7iwzTNB0B4dExi9wecDwlSwhEF87cijOoHok/cJH4G5N8hkyiOlHRVkvIU yAGkmW3vxgib4TEl99pEb0mSNOTiGT8E8tC29/JBiyzSLxfT3CUnRD6RnSf0+1NyuiIRn4ehi6Gs JZ0MjpdkEWJTK0SO8sEyuUc6TiuBLE8i4oaQpuTpCmlJpwMeRFhjcExkBwPJvSV1VdWWW1KAqW5w 6OOTFbLIhYBhhhRdSNLFINqKtOUgylaQ5dIy8ZN4sEgwqaO/rvC5tFBpmlpwaHJhUN/QVQq6Yfpi zXWFblLWpK4JZqWjWuDGFQJvVgJvVO+jdsAbVa5fopkXPqx7UeklUOPiva4OB3XVN4Iac9BrpG5W JJsuaDrzhWVYwvQ0ChQsy6SaMHSfctWUTTb3wuzV/dmrnXfvLrz0F/5cmP399cvncMTD6o2OeJSB p7SDlC/dvpQgSuj2Mjm9cr51acuydOYC+J6laz6n1KKM44hNBpbZ9KBeD6pqFCSpeLLOKIi8fh55 NiGp6I8AdVEQldI1haqKahCm2sy0mU4U2qRUIjuxwAZFPDyW5hDqTSA0ajPtAuIqQ83dI3DjBh4k tX8S8NFakUAbxMRFT517aJWQg36QSle7aB6DXkKmSrutFsh2vuMP6kVt24+iAFNSHt7FBagXw0pO WoH7G//keeBVW7ARVjRrhZwjXghD1szCEs2mQS1PAYvpCtUZVVzKhOJahkZNpgvXh/ayWlxbwCoa RxB5cXJFBXrTCpWKbhJn/5+MXVP5B0vbWqE= ==== END SVK PATCH BLOCK ====
On Sun Feb 25 15:33:14 2007, MSCHWERN wrote: Show quoted text
> If you pass in a reference to Math::BigFloat->new() it assumes its a > BigInt object. This assumption breaks overloaded objects. > > $bigfloat = Math::BigFloat->new($overloaded_number); > > It thinks its a BigInt and tries to call as_number() on it. > > The attached patch tests for and fixes this.
Thanx, just one question: + if ((ref($wanted)) && eval { $wanted->isa("Math::BigInt") }) Would it be okay to write this as: + if ((ref($wanted)) && UNIVERSAL::isa($wanted,"Math::BigInt")) I don't like to introduce needless evals. Best wishes, Tels
I just released Math::BigInt 1.80 to CPAN, which hopefully fixes this issue. Thank you for your report, and sorry for the long delay.
From: MSCHWERN [...] cpan.org
On Thu Mar 01 16:39:45 2007, TELS wrote: Show quoted text
> Thanx, just one question: > > + if ((ref($wanted)) && eval { $wanted->isa("Math::BigInt") }) > > Would it be okay to write this as: > > + if ((ref($wanted)) && UNIVERSAL::isa($wanted,"Math::BigInt")) > > I don't like to introduce needless evals.
Sorry, I didn't see this message because of the RT email outages. No, its not ok. $wanted may override isa() and UNIVERSAL::isa() as a function does not honor this. Also eval BLOCK is not the same as eval STRING and does not have nearly the overhead. Its not worth worrying about. There are legit reasons to override isa(). For example, seemless delegation to a Math:BigFloat object. package Thing; sub new { my($class, $number) = @_; my $self = bless {}, $class; $self->{bigfloat} = Math::BigFloat->new($number); return $self; } sub AUTOLOAD { ...blah blah autoload stuff... return $self->{bigfloat}->$method(@args); } sub isa { my($class, $isa) = @_; return 1 if $class->{bigfloat}->isa($isa); return $class->SUPER::isa($isa); } sub can { my($class, $method) = @_; return 1 if $class->{bigfloat}->can($method); return $class->SUPER::can($method); }
RT-Send-CC: MSCHWERN [...] cpan.org
On Fri Apr 13 11:59:28 2007, MSCHWERN wrote: Show quoted text
> No, its not ok. $wanted may override isa() and UNIVERSAL::isa() as a > function does not honor this. Also eval BLOCK is not the same as eval > STRING and does not have nearly the overhead. Its not worth worrying
about. Show quoted text
> > There are legit reasons to override isa(). For example, seemless > delegation to a Math:BigFloat object.
Is this still an issue? There are still references to UNIVERSAL:: calls in BigFloat.
Subject: Re: [rt.cpan.org #25144] [PATCH] Math::BigFloat->new considers any reference a BigInt.
Date: Mon, 09 Jul 2012 17:51:23 -0700
To: bug-Math-BigInt [...] rt.cpan.org
From: Michael G Schwern <schwern [...] pobox.com>
On 2012.7.9 10:19 AM, Brendan Byrd via RT wrote: Show quoted text
>> There are legit reasons to override isa(). For example, seemless >> delegation to a Math:BigFloat object.
> > Is this still an issue? There are still references to UNIVERSAL:: calls > in BigFloat.
Yes, it's still an issue in the sense that calling UNIVERSAL::isa or can breaks encapsulation. The first two calls will fail of $wanted overrides can() or isa(). Math::BigFloat's own isa() override does not honor its parent's possible isa() override. The isa() override in the subclass of MBF in t/upgradef.t also does not honor MBF's isa() override and happens to work by knowing the details of MBF's isa() and repeating them. This is the first example of recursive irony I've encountered. A patch is attached. I don't know why Tels didn't want to use eval blocks, probably a misplaced micro-optimization concern. -- Alligator sandwich, and make it snappy!

Message body is not shown because sender requested not to inline it.