Front page | perl.perl5.porters |
Postings from November 2003
Re: [5.8.12] Proposal for changing UNIVERSAL semantics
Thread Previous
|
Thread Next
From:
Tim Bunce
Date:
November 4, 2003 07:26
Subject:
Re: [5.8.12] Proposal for changing UNIVERSAL semantics
Message ID:
20031104152616.GA96036@dansat.data-plan.com
On Tue, Nov 04, 2003 at 02:34:36AM -0800, Michael G Schwern wrote:
> 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().
Overridden yes, but busted is a different matter:
=item C<can>
$is_implemented = $h->can($method_name);
Returns true if $method_name is implemented by the driver or a
default method is provided by the DBI.
It returns false where a driver hasn't implemented a method and the
default method is provided by the DBI is just an empty stub.
(That last sentance isn't in 1.38)
That's why $drh->can('disconnect_all') returned false above.
Also, when it returns true it returns a ref to the DBI dispatch
method and not to the underlying driver method. That's so
$code = $h->can($method_name); $code->(...); won't bypass the
important functionality provided by the dispatcher.
Note that the DBI does not alter UNIVERSAL::can, it only provides
a can() method for DBI handles.
> 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.
Since when is it a hack? There may be a problem in the implementation,
which I'll gladly fix if indentified, but there's no hack here.
> Here's t/01basic.t run this time with -MAttribute::Handlers
>
> 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.
I'm not convinced. Here's the code for can() that's embedded into the DBI
dispatcher:
if (*meth_name == 'c' && strEQ(meth_name,"can")) {
char *can_meth = SvPV(st1,lna);
SV *dbi_msv = Nullsv;
SV *imp_msv; /* handle implementors method (GV or CV) */
here it looks up the $h->can($can_meth) method just as it does when
it's going to call it:
if ( (imp_msv = (SV*)gv_fetchmethod(DBIc_IMP_STASH(imp_xxh), can_meth)) ) {
if found it then looks up the same method name using the original handle
using gv_fetchmethod_autoload() just as UNIVERSAL::can does in universal.c:
/* return DBI's CV, not the implementors CV (else we'd bypass dispatch) */
/* and anyway, we may have hit a private method not part of the DBI */
GV *gv = gv_fetchmethod_autoload(SvSTASH(SvRV(orig_h)), can_meth, FALSE);
if (gv && isGV(gv))
dbi_msv = (SV*)GvCV(gv);
}
if (dbi_msv) {
ST(0) = sv_2mortal(newRV(dbi_msv));
XSRETURN(1);
}
else not found so return nothing:
}
XSRETURN(0);
Returning an empty list instead of undef difers from UNIVERSAL::can so I'll
change that for the next release. But I doubt that's the issue here.
> 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.
I'm not sure it's relevent but I'll remind people here, in case
it's been forgotten, that AUTOLOAD and methods do not mix well unless
all the method have been pre-declared. That's why modules like AutoLoader
go to such lengths to generate and load declarations for all the subs
to be autoloaded.
> > * 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.
Before I can "fix" the DBI someone needs to explain what it's doing
wrong and how that's causing the problem reported.
Tim.
Thread Previous
|
Thread Next