develooper Front page | perl.perl5.porters | Postings from April 2007

Re: [Andreas J. Koenig] C3 MRO breaks Params::Util and Graph

Thread Previous | Thread Next
From:
Brandon Black
Date:
April 23, 2007 21:30
Subject:
Re: [Andreas J. Koenig] C3 MRO breaks Params::Util and Graph
Message ID:
84621a60704232130u27e267cap443fbfb9b7524e08@mail.gmail.com
On 4/23/07, Brandon Black <blblack@gmail.com> wrote:
> On 4/23/07, Andreas J. Koenig <andreas.koenig.7os6VVqR@franz.ak.mind.de> wrote:
> > Brandon, maybe you didn't receive this mail, did you?
> >
> > http://groups.google.com/group/perl.perl5.porters/browse_thread/thread/47f5ec3f4be4e1ec/93e166ba411986c8#93e166ba411986c8
> >
> > I'm resending because you were posting a new patch but saw no answer
> > to my posting. You should know that Params::Util has a lot of
> > dependents and so my smoke testing has lost about 50 CPAN
> > distributions now. Would you mind having a look?
> >
>
> Sorry, I didn't notice that mail the first time, although I see it
> now.  I'll figure out it asap.
>

I've found the cause, and I have what looks to me like an ok fix.  The
issue here is that these packages are calling overload::Method on
something that is not a package or an object, but instead a simple
arrayref.

The overload code/docs don't seem to indicate that this is supposed to
work, it just happens to work anyways by fluke of implementation, and
by changing the implementation I've removed the fluke.  The fluke
amounted to the fact that it is assuming that all refs are blessed
objects, and using the reftype as the package name to search for the
method.  Since %{ARRAY::} and @ARRAY::ISA happen to be empty, the old
implementation failed to find the method and returned undef.  The mro
patch version balks at the fact that ARRAY isn't even a package
because it's using the mro code to search the @ISA parents (so that
overload works correctly with C3).

I've attached a very short patch to blead's overload.pm that seems to
work, by actually checking the blessed status of the passed thing, and
returning undef for things that are unblessed refs, which should
preserve the existing behavior in these cases.

I used Scalar::Util::blessed to get the blessed-ness.  Other code in
the same file was also using various functions from Scalar::Util
(including blessed), but overload.pm does not require Scalar::Util.
I'm guessing this is an oversight that evaded testing due to
overload.t having a "use Scalar::Util" line in it, so the patch adds a
"require Scalar::Util" line too, which was necessary.

This in turn has lead to another problem.  The "require Scalar::Util"
in overload.pm breaks ext/Dynaloader/t/Dynaloader.t, because it raises
some count of loaded xs modules by one during two of the tests.
Dynaloader itself is fine, its just that the new require line is
breaking one of its test assumptions, and I didn't see a clean fix for
that right now, and I'm heading to bed.

In summary, I think the patch I've attached is the right fix for
overload.pm, but we'll need to fix the Dynaloader test.

-- Brandon

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About