Re: [5.8.12] Proposal for changing UNIVERSAL semantics

Front page | perl.perl5.porters | Postings from November 2003

Re: [5.8.12] Proposal for changing UNIVERSAL semantics

Thread Previous | Thread Next

From:

Michael G Schwern

Date:

November 4, 2003 02:35

Subject:

Re: [5.8.12] Proposal for changing UNIVERSAL semantics

Message ID:

20031104103435.GA12786@localhost.comcast.net

I think you're attacking this problem from the wrong end.

On Mon, Nov 03, 2003 at 12:37:49PM -0800, Michael Jacob wrote:
> 1.) Change can()'s semantics.
> 
> can() returns unexpected reults if any module changed something in the UNIVERSAL:: package. This can make other modules fail, we have seen this with DBI(!)... So:

It would seem DBI breaks because it did something to can().

DBI::disconnect_all(/usr/local/src/CPAN/DBI-1.38/blib/lib/DBI.pm:652):
652:        while ( my ($name, $drh) = each %DBI::installed_drh ) {
  DB<2> n
DBI::disconnect_all(/usr/local/src/CPAN/DBI-1.38/blib/lib/DBI.pm:653):
653:            $drh->disconnect_all() if ref $drh;
  DB<2> x UNIVERSAL::can($drh, "disconnect_all")
0  CODE(0x514e94)
   -> &CODE(0x514e94) in ???
  DB<3> x $drh->can('disconnect_all')
  empty array
  DB<4> x $drh->can('can')
0  CODE(0x5142b4)
   -> &DBI::dr::can in 0
  DB<5> b DBI::dr::can
Subroutine DBI::dr::can not found.

DBI appears to have overriden and busted can().  Running DBI with UNIVERSAL.pm
doesn't cause any problems, so I think adding Attribute::Handlers into the 
mix exposed some problem with DBI's can() hacks.

Here's t/01basic.t run this time with -MAttribute::Handlers

ok 43 at line 112
DBI::disconnect_all(/usr/local/src/CPAN/DBI-1.38/blib/lib/DBI.pm:652):
652:        while ( my ($name, $drh) = each %DBI::installed_drh ) {
  DB<2> n
DBI::disconnect_all(/usr/local/src/CPAN/DBI-1.38/blib/lib/DBI.pm:653):
653:            $drh->disconnect_all() if ref $drh;
  DB<2> x $drh->can('can')
0  CODE(0x5417f0)
   -> &CODE(0x5417f0) in ???
  DB<3> x $drh->can('disconnect_all')
0  CODE(0x541820)
   -> &DBI::dr::disconnect_all in 0
  DB<4> 

DBI.xs is doing something really funky and intercepting can() method calls.
It also appears to have changed the expected behavior of can().  It also 
appears to have its own method dispatcher.  At this point I'm likely to
say that Attribute::Handlers' screwing with @UNIVERSAL::ISA simply exposed a 
bug in DBI. 

> Starting with Perl 5.8.12

Oh, good, we'll probably never make it to 5.8.12. ;)

> can() shall not search the package UNIVERSAL for inherited methods unless UNIVERSAL is explicitly included in the @ISA of the class tested against.

Wait a second, the results are *not* unexpected.  can() tells us if a method
can execute a given method and what that method will be.  This change
breaks the well documented behavior of can() by making UNIVERSAL an
exception.  can() might report the method isn't there, but its there.

This just papering over the problem of begin cavalier with UNIVERSAL 
(as Attribute::Handlers is) and overriding UNIVERSAL methods can() (as DBI 
is).  The namespace is still polluted, the methods are still there and can
be called.  All you're doing is taking down the signs.

> I think, this change should not break any well written module. Testing for any method that's in UNIVERSAL is just useless, we already know before the test that it must be there.  Any module that want's to know if there is some additional method in UNIVERSAL:: can just test against UNIVERSAL->can() directly.

No, this breaks encapsulation and defeats the point of can().  I shouldn't 
have to know if Foo implements a method or if Foo inherits it from UNIVERSAL 
or any other module. 

It adds an extra step to nearly every use of can().

	$class->foo if $class->can('foo') or UNIVERSAL->can('foo');

> 2.) Implement a smart UNIVERSAL::AUTOLOAD in UNIVERSAL.pm

Someone once said, when you've got a problem and you try to solve it with
AUTOLOAD you now have two problems.

Also, I would not want to see UNIVERSAL.pm expanded at all.  There's
already too many functions polluting the global namespace as it is which
is what caused all these problems in the first place.  Adding more,
and an AUTOLOAD no less!!, is likely to simply make things worse.

However, what you propose addresses a the valid problem of coordinating
AUTOLOAD overrides.  But its likely to be a really sticky thing to get just
and thus wrong for the core.  You should put this up on CPAN as 
UNIVERSAL::AUTOLOAD.

> 3.) Implement a smart UNIVERSAL::DESTROY in UNIVERSAL.pm
> 
> See UNIVERSAL::AUTOLOAD and ignore the return value things.

Same comments as UNIVERSAL::AUTOLOAD.

> 5.) Implement a smart UNIVERSAL::import in UNIVERSAL.pm
> 
> To stop doing damage to other modules we should also include a smart import():
> 
> UNIVERSAL::import will work as Exporter::import if call on UNIVERSAL itself It will issue a warning and behave like Exporter::import if the packing it works for includes a @EXPORT array. It will do nothing otherwise.
> 
> This way we will break neither the modules that rely on being an Exporter if use()ing UNIVERSAL, nor the modules that rely on not having any import() method by default.

Nobody should be relying on UNIVERSAL::import.  If they are, its a bug.
Fortunately, we're still allowed to fix minor bugs without considering bug
compatibility.

This issue has been fixed in bleadperl.

> Summary:
> 
> This may look like a lot of big changes, but it's not. This is a fix for many small problems and, it also extends UNIVERSAL's usefulness by removing the limitation of just one module being able to override UNIVERSAL::AUTOLOAD.
> 
> With this fix, even if using only the new UNIVERSAL.pm with an old Perl, we can fix all UNIVERSAL related problems reported the last month:

The severity of these problems is very low...

>  * useing UNIVERSAL breaking DBI, autouse, Net::SSH, ...

DBI is doing Dark Magick with can() and the method dispatcher which appears to
be not 100% correct.  Plus Attribute::Handlers is plowing *way* too much 
stuff into the UNIVERSAL namespace.  Both will be fixed and the problem 
should disappear.

autouse is making assumptions about Exporter usage.  Its assumptions were
challenged. :)  Since the new versions of both autouse and UNIVERSAL will be 
released simultaneously in 5.10, this will cause no problems.

I didn't hear anything about Net::SSH.

>  * removing UNIVERSAL::import breaking BerkeleyDB, ...

This was a bug in a test, not the module itself.  It would only effect
those installing that version with 5.10 (ie. the first stable version of
Perl to have the UNIVERSAL::import fix).  Its since been fixed and a new
version put on CPAN it will not effect anyone when 5.10 comes out.

>  * existance of UNIVERSAL::AUTOLOAD breaking XML::SAX, ...

Haven't heard about this one, but I'm not surprised.

-- 
Michael G Schwern        schwern@pobox.com  http://www.pobox.com/~schwern/
Don't be a lover, girl, I'd like to shamelessly hump your inner child.

Thread Previous | Thread Next