Front page | perl.perl5.porters |
Postings from November 2003
Re: [5.12.0] Proposal for changing UNIVERSAL semantics
Thread Previous
|
Thread Next
From:
Michael G Schwern
Date:
November 4, 2003 05:14
Subject:
Re: [5.12.0] Proposal for changing UNIVERSAL semantics
Message ID:
20031104131316.GC13911@localhost.comcast.net
On Tue, Nov 04, 2003 at 04:10:58AM -0800, Michael Jacob wrote:
> Ok, here you're right, I was oversimplifying. DBI will break when used with any DBD that does not implement all possible methods when there is an UNIVERSAL::AUTOLOAD present.
>
> That's because DBI does somthing like:
>
> if ($dbd->can($method) or $dbd->can('AUTOLOAD') {
> $dbd->$method();
>
> And that's just what I said above---an unexpected result from isa().
There's nothing in DBI that appears to do this. The only place DBI uses
can() in normal code is DBI::PurePerl and even then it doesn't try an
AUTOLOAD fallback (though it mentions in the comments that it might).
Even the XS code doesn't appear to mention AUTOLOAD.
Either way, UNIVERSAL::AUTOLOAD is Fraught With Peril and implementers
should touch it At Their Own Risk. Changing can() to try to hide the
existence of a UNIVERSAL::AUTOLOAD is just putting a band-aid on the problem.
Its all too easy to screw it up in lots of interesting ways that have
nothing to do with can().
I'm looking at DBI 1.38. I don't know what version you're looking at.
> >> 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
>
> I think that should be "if an object can execute a given method".
>
> That's exactly the problem. It is not that object, that can() do something. It is a code written by another person for another reason for use with another kinf of objects that can do something other when called. This is almost never the thing I want to know.
>
> When I call can(), I want to know if the object or one of its base classes implement some method. And when someone pushed a method with the same name into UNIVERSAL he did almost surely did not want to add the functionality to that object I'm testing.
This is directly contrary to how OO inheritence works in Perl.
Anytime you inherit from a class in Perl you inherit *everything*.
This is a consequence of not having real method privacy, but that's a
Perl design decision. We deal. UNIVERSAL is simply a special case of this.
You can just as easily accidentally pick up an AUTOLOAD from Exporter
as from UNIVERSAL. Should can() stop looking at Exporter?
You seem to consider that anything which is inherited from UNIVERSAL is a
mistake. While its true that its all too easy for a lot of accidental
crap to get into UNIVERSAL if you're sloppy, this is clearly not its
intention. Again, I point to the three modules on CPAN which put
public methods into UNIVERSAL. UNIVERSAL::require, UNIVERSAL::exports and
UNIVERSAL::moniker.
> If one wants to add e.g. AUTOLOAD functionality to some class, he would surely not install UNIVERSAL::AUTOLOAD but subclass that class. (And if he doesn't we can tell him he's coding some kind of dangerous rubish. <smile/>)
There's three reasons people use UNIVERSAL::AUTOLOAD.
1) They need universal autoloading.
2) They think they universal autoloading.
3) They don't understand autoloading.
#1 implies a legit use of UNIVERSAL and shouldn't be papered over. 2 and 3
are poor design decisions and should be fixed. Unfortunately, we can't
programmaticly tell the difference between them.
> >breaks the well documented behavior of can() by making UNIVERSAL an
> >exception. can() might report the method isn't there, but its there.
>
> UNIVERSAL is an exception already. We'd just remove it.
UNIVERSAL is an exception *only that it sits at the top of the class
hierarchy* otherwise its a perfectly normal class. Methods in it should
be picked up by can() same as any other.
> >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.
>
> They can be called, right. But, if you want to call a method that comes from UNIVERSAL there is no need to test with can()---you already know it's there. And about the signs, you can always add "||UNIVERSAL->can()" if you really want to know.
No, the whole point is I *don't* want to know a method comes from UNIVERSAL.
I want to know the capabilities of a class *without caring where it comes
from*. It violates encapsulation to have methods inherited from a super
class, UNIVERSAL or not, be a special case.
The whole point of OO is that subclasses act like their parents. UNIVERSAL
is the parent of everything. Again, you might not like it but that's the
way it is.
> >It adds an extra step to nearly every use of can().
> >
> > $class->foo if $class->can('foo') or UNIVERSAL->can('foo');
>
> No, it removes the extra step that is missing from nearly every use of can()
>
> $class->foo if $class->can('foo') and $class->can('foo') ne UNIVERSAL->can('foo');
Again, you have this idea that anything inherited from UNIVERSAL is garbage.
This is wrong.
> >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.
>
> The problem with AUTOLOAD is, that it is one of the special methods Perl will automatically call. So if you want to code "autouse", you have no other way to do it. If you want to add specific error messages about attributes, you have no other way to do it. There will be conflicts aout this limited resource, we see it now, and we now debate about changing UNIVERSAL.pm anyway, do why not solve this problem now, too?
Heh. "Well, we've got the hood open to change the air filter, why don't we
just convert to hydrogen fuel cells while we're at it?"
I'm more than convinced that getting a transparent UNIVERSAL::AUTOLOAD
correct will involve lots and lots and lots of time and sweat and probably
wind up doing more trouble than good. No, this is definately not the
sort thing you just do while you're passing by and its definately not the
sort of thing you drop directly into the core.
Take your lumps on CPAN first.
> >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.
>
> There may be better ways, maybe "use UNIVERSAL AUTOLOAD => \&myHandler;" or "use UNIVERSAL::AUTOLOAD handler => \&myHandler;" instead of the array, that's true. But it really should be a solution in the core so all module authors pick up that save way to do it. Else it's just another nearly unused module like that slurp thing last week...
Now you're saying that if you put something on CPAN it'll be unused? Grand!
Placing something in the core doesn't guarantee anyone will use it either.
Let's see... File::stat, Time::Localtime, SelectSaver, FileCache. Great
ideas, but for whatever reason nobody uses them.
Always remember this: Once something is in the core *it cannot be removed*.
> Let's express it this way: UNIVERSAL::AUTOLOAD is a limited resource inside core Perl. So a solution for coordinated use should exist in the core, too.
Prove the concept on CPAN first. There's no need to special case your
solution by putting it into the core. Find the users of UNIVERSAL::AUTOLOAD
and submit them patches for your new module. I'm sure they'll be
more than happy that their modules don't conflict with anyone else.
Problem solved. No core dependency. No waiting for 5.10. No dead module
hanging around in the core should the concept not work out.
> >> 5.) Implement a smart UNIVERSAL::import in UNIVERSAL.pm
>
> >This issue has been fixed in bleadperl.
>
> And taken out of 2.8.2, I followed the list a while. But I've also seen that the simple fix breaks some other code, even if it's just some tests. So I proposed a more compatible way to do it.
You propose to break the documented behavior of can() and say its
more compatible than fixing an old bug? Come now.
Let's keep in mind that the UNIVERSAL::import() change actually got some use
and shook out some bugs. It all looked very safe and sensible at the time and
didn't break any documented behavior. Your clever and oh so tiny breakage
of can() might not look so compatible after running against real world code
for a week.
> >The severity of these problems is very low...
>
> Yes, the severity is low, but the level of knowledge to fix/workaround a simple "use Attribte::Handlers; use DBI; DBI->connect('mysql... => KAWUMM" is rather high.
Fortunately, only one person has to have that knowledge and its fixed for
everyone: The person who fixes Attribute::Handlers and/or DBI.
> >I didn't hear anything about Net::SSH.
>
> It used use() parameters in a cosmetic way for it's submodules and was no Exporter. Broke on UNIVERSAL::import...
I don't see how that could be, it inherits from Exporter and has no
custom import. Are you sure you mean Net::SSH and not something else?
> >> * removing UNIVERSAL::import breaking BerkeleyDB, ...
> >
> >This was a bug in a test, not the module itself. It would only effect
>
> I know, but I'd guess there is at least one module in the wild, that will really break. (Ok, fix is very easy, but as said above, the knowledge to fix..)
Again, only one person needs that knowledge.
> >> * existance of UNIVERSAL::AUTOLOAD breaking XML::SAX, ...
> >
> >Haven't heard about this one, but I'm not surprised.
>
> elsif (defined $callbacks->{'Handler'} and $method = $callbacks->{'Handler'}->can('comment') ) {
> ...
> elsif (defined $callbacks->{'Handler'} and $callbacks->{'Handler'}->can('AUTOLOAD') ) {
>
> Have a thousand Perl programmers search for bugs in this two lines, I'd guess the only ones that would find it are those who read about UNIVERSAL on p5porters these days.
Maybe you have a different version of CPAN than me, but I cannot find
any code like that in XML::SAX 0.12. I think you mean XML::SAX::Base.
I can tell you right now what's wrong with the logic in XML::SAX::Base.
Forget UNIVERSAL::AUTOLOAD, the idea that "its got an AUTOLOAD so it must
be able to do what I want" isn't gonna fly. You don't know what an AUTOLOAD
might do or what its there for. *Especially* in a base class like
XML::SAX::Base where you don't know who's going to extend it or how.
Its currently doing something like this:
if( $obj->can('some_method') ) {
$obj->some_method;
}
elsif( $obj->can('AUTOLOAD') ) {
# great, we have an AUTOLOADER, it must do what we need
$obj->some_method;
}
it should really be doing this:
if( $obj->can('some_method') ) {
$obj->some_method;
}
elsif( $obj->can('AUTOLOAD') ) {
# hmm, its got an AUTOLOADer, lets see if it does what I need
eval { $obj->some_method };
if( $@ ) {
if( $@ =~ /^Can't locate object method "some_method"/ ) {
# guess it doesn't have it, move on.
}
else { # we died for some other reason
die $@;
}
}
else {
# great, we have that method
}
}
That's a problem inherent in *all* AUTOLOADers.
--
Michael G Schwern schwern@pobox.com http://www.pobox.com/~schwern/
You'll do.
Thread Previous
|
Thread Next