[lucy-dev] [PATCH] Clownfish: compatibility with perl 5.27.6

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[lucy-dev] [PATCH] Clownfish: compatibility with perl 5.27.6

webmasters-4
The attached patch gets the Clownfish CPAN distribution working with perl 5.27.6.  The code that my patch changes was already subtly wrong: references in stashes, which have been put there by constant.pm since perl 5.10, do represent callable methods.  So anything that is a reference should be added to the list of methods.  On account of the change cited below, Clownfish now fails its tests with bleadperl.

From the perldelta for 5.27.6:

=head2 Subroutines no longer need typeglobs

Perl 5.22.0 introduced an optimization allowing subroutines to be stored in
packages as simple sub refs, not requiring a full typeglob (thus
potentially saving large amounts of memeory).  However, the optimization
was flawed: it only applied to the main package.

This optimization has now been extended to all packages.  This may break
compatibility with introspection code that looks inside stashes and expects
everything in them to be a typeglob.

When this optimization happens, the typeglob still notionally exists, so
accessing it will cause the stash entry to be upgraded to a typeglob.  The
optimization does not apply to XSUBs or exported subroutines, and calling a
method will undo it, since method calls cache things in typeglobs.

[perl #129916] [perl #132252]

open_yY0Tvpz7.txt (691 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [lucy-dev] [PATCH] Clownfish: compatibility with perl 5.27.6

Nick Wellnhofer
On 19/11/2017 20:42, [hidden email] wrote:
> The attached patch gets the Clownfish CPAN distribution working with perl 5.27.6.  The code that my patch changes was already subtly wrong: references in stashes, which have been put there by constant.pm since perl 5.10, do represent callable methods.  So anything that is a reference should be added to the list of methods.  On account of the change cited below, Clownfish now fails its tests with bleadperl.

We are aware of this issue and already patched it. A new release will be out
this week.

 
https://git1-us-west.apache.org/repos/asf?p=lucy-clownfish.git;a=commitdiff;h=f30ed78acd52df8425914b618430fe8816f226c8

Maybe you can have a quick look and check if our patch is OK.

Nick
Reply | Threaded
Open this post in threaded view
|

Re: [lucy-dev] [PATCH] Clownfish: compatibility with perl 5.27.6

webmasters-4

On Nov 20, 2017, at 5:38 AM, Nick Wellnhofer <[hidden email]> wrote:

> On 19/11/2017 20:42, [hidden email] wrote:
>> The attached patch gets the Clownfish CPAN distribution working with perl 5.27.6.  The code that my patch changes was already subtly wrong: references in stashes, which have been put there by constant.pm since perl 5.10, do represent callable methods.  So anything that is a reference should be added to the list of methods.  On account of the change cited below, Clownfish now fails its tests with bleadperl.
>
> We are aware of this issue and already patched it. A new release will be out this week.
>
> https://git1-us-west.apache.org/repos/asf?p=lucy-clownfish.git;a=commitdiff;h=f30ed78acd52df8425914b618430fe8816f226c8
>
> Maybe you can have a quick look and check if our patch is OK.

Yes, that will work, but note that you do still continue to skip constants, which are callable as methods.  I don’t know whether that matters to you.

Father Chrysostomos
Reply | Threaded
Open this post in threaded view
|

Re: [lucy-dev] [PATCH] Clownfish: compatibility with perl 5.27.6

Nick Wellnhofer
On 20/11/2017 15:22, [hidden email] wrote:
> Yes, that will work, but note that you do still continue to skip constants, which are callable as methods.  I don’t know whether that matters to you.

I think it doesn't matter much and it's OK to stick to the current behavior
for now.

It seems that constants are initially stored as refs in a stash (for example
as scalarrefs, arrayrefs, refs-to-refs), but if they're evaluated using method
syntax for the first time, they get converted to globs. So we treat constants
inconsistently which is a bug, although a really minor one.

If there is a way to detect constants reliably, maybe via %constant::declared,
we could make sure that they are ignored or throw an error if a constant
shadows a method name. OTOH, constants can be called with method syntax, so we
could also allow them as methods.

Nick