Bug #78273 for Package-Stash: Undocumented inconsistency between unfiltered and filtered output

Mon Jul 09 09:17:49 2012 KENTNL [...] cpan.org - Ticket created

Subject:

Undocumented inconsistency between unfiltered and filtered output

In the attached code snippet, everything works as expected except for the type_filter 'CODE'.

There are a few anomalies in the difference between unfiltered and filtered lists, which are not documented.

The most obvious ones are 'isa' and 'can'. These methods are returned by ->get_all_symbols() , but not returned by ->get_all_symbols('CODE'), or any other call to ->get_all_symbols with a type_filter.

The confusing one is DESTROY, which comes up, despite being one of the subs I had expected not to be returned, is returned : "Note that if the package contained any BEGIN blocks, perl will leave an empty typeglob in the BEGIN slot, so this will show up if no filter is used (and similarly for INIT, END, etc)."

I had imagined "DESTROY" would be in the "etc" . Mostly because of the all caps deal, and that I never use DESTROY myself, and that, I'd forgotten that DESTROY is not END, but just a real method created by moose like 'new'. But perhaps it would be more appropriate to list all the 'GLOBS' that a user can expect to be visible without a type filter, but not with a type filter, instead of just stating 'etc'.

Attached test ,

Output:

prove /tmp/t.pl
/tmp/t.pl .. # Package Stash version: 0.33 , Implementation: XS
/tmp/t.pl .. 1/?
    #   Failed test 'Key "can" not found'
    #   at /tmp/t.pl line 59.

    #   Failed test 'Key "isa" not found'
    #   at /tmp/t.pl line 59.

    #   Failed test 'Unexpected key "DESTROY" found'
    #   at /tmp/t.pl line 62.
    # Looks like you failed 3 tests of 5.

Show quoted text

# Failed test 'has_keys(get_all_symbols(filter_code))'
# at /tmp/t.pl line 69.
# Looks like you failed 1 test of 6.

/tmp/t.pl .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/6 subtests

Test Summary Report
-------------------
/tmp/t.pl (Wstat: 256 Tests: 6 Failed: 1)
Failed test: 2
Non-zero exit status: 1
Files=1, Tests=6, 0 wallclock secs ( 0.02 usr 0.01 sys + 0.21 cusr 0.01 csys = 0.25 CPU)
Result: FAIL

Subject:

t.pl

#!/usr/bin/perl use strict; use warnings; { package Example; use Moose; use namespace::autoclean; __PACKAGE__->meta->make_immutable; } use Test::More; use Package::Stash; diag "Package Stash version: $Package::Stash::VERSION , Implementation: $Package::Stash::IMPLEMENTATION"; my $expectations = { unfiltered => { args => [], keys => [qw( can ISA BEGIN DESTROY isa new meta )], }, filter_array => { args => [qw( ARRAY )], keys => [qw( ISA )], }, filter_io => { args => [qw( IO )], keys => [qw( )], }, filter_hash => { args => [qw( HASH)], keys => [qw( )] }, filter_scalar => { args => [qw( SCALAR )], keys => [qw( )], }, filter_code => { args => [qw( CODE )], keys => [qw( can isa meta new )], }, }; sub has_keys { my ( $name, $x, @keys ) = @_; my $y = { %{$x} }; my $pass = 1; subtest "has_keys($name)" => sub { for my $key (@keys) { if ( exists $y->{$key} ) { delete $y->{$key}; pass("Key $key found"); next; } $pass = 0; fail("Key \"$key\" not found"); } for my $key ( keys %{$y} ) { fail("Unexpected key \"$key\" found"); $pass = 0; } if ($pass) { pass("All good for has_keys($name)"); } return $pass; }; } for my $name ( keys %{ $expectations } ) { my @args = @{ $expectations->{$name}->{args} }; my @keys = @{ $expectations->{$name}->{keys} }; has_keys( "get_all_symbols($name)" =>, Package::Stash->new('Example')->get_all_symbols(@args), @keys, ); } done_testing;

Thu Jul 11 10:09:12 2013 DOY [...] cpan.org - Correspondence added

This is all working correctly - can and isa don't exist in the stash (they are inherited from UNIVERSAL), and DESTROY is just a normal method, not a special phase subroutine. I'd be willing to take a doc patch to clarify this.

Thu Jul 11 10:09:12 2013 The RT System itself - Status changed from 'new' to 'open'

Thu Jul 11 12:29:20 2013 KENTNL [...] cpan.org - Correspondence added

On 2013-07-12 02:09:12, DOY wrote:
> This is all working correctly - can and isa don't exist in the stash
> (they are inherited from UNIVERSAL), and DESTROY is just a normal
> method, not a special phase subroutine. I'd be willing to take a
> doc patch to clarify this.

I think you're missing what I'm saying.

Despite some of the symbols turning up in the package via inheritance, some symbols are returned by get_all_symbols() anyway, despite them being inherited.

And those same symbols do not turn up when you apply a CODE filter.

{
"get_all_symbols()" => ["new", "meta", "DESTROY", "isa", "can", "ISA", "BEGIN"],
"get_all_symbols(q{CODE})" => ["new", "DESTROY", "meta"],
}

Why are 'isa', and 'can' showing up for get_all_symbols() to start with?

Subject:

e.pl

#!/usr/bin/env perl use strict; use warnings; { package Example; use Moose; use namespace::autoclean; __PACKAGE__->meta->make_immutable; } use Package::Stash; use Data::Dump qw(pp); my $stash = Package::Stash->new('Example'); pp({ 'get_all_symbols()' => [ keys $stash->get_all_symbols ] , 'get_all_symbols(q{CODE})' => [ keys $stash->get_all_symbols('CODE') ], });

Thu Jul 11 12:39:01 2013 doy [...] tozt.net - Correspondence added

Subject:	Re: [rt.cpan.org #78273] Undocumented inconsistency between unfiltered and filtered output
Date:	Thu, 11 Jul 2013 12:38:42 -0400
To:	Kent Fredric via RT <bug-Package-Stash [...] rt.cpan.org>
From:	Jesse Luehrs <doy [...] tozt.net>

On Thu, Jul 11, 2013 at 12:29:21PM -0400, Kent Fredric via RT wrote: Show quoted text

> Queue: Package-Stash > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=78273 > > > On 2013-07-12 02:09:12, DOY wrote:

> > This is all working correctly - can and isa don't exist in the stash > > (they are inherited from UNIVERSAL), and DESTROY is just a normal > > method, not a special phase subroutine. I'd be willing to take a > > doc patch to clarify this.

> > I think you're missing what I'm saying. > > Despite some of the symbols turning up in the package via inheritance, some > symbols are returned by get_all_symbols() anyway, despite them being inherited. > > And those same symbols do not turn up when you apply a CODE filter. > > { > "get_all_symbols()" => ["new", "meta", "DESTROY", "isa", "can", "ISA", > "BEGIN"], > "get_all_symbols(q{CODE})" => ["new", "DESTROY", "meta"], > } > > Why are 'isa', and 'can' showing up for get_all_symbols() to start with?

Ah, this is actually a different issue that I forgot about. Whenever you call a method, an (empty) glob is inserted into the stash to hold information about the method lookup cache. If you stick 'eval "Foo->bar"' before the calls to get_all_symbols, you'll see 'bar' showing up there too. For instance: $ perl -MData::Dump -e'eval "Foo->bar"; ddx(\%Foo::)' # -e:1: { AUTOLOAD => *Foo::AUTOLOAD, bar => *Foo::bar } This is just an artifact of how perl works. It should probably also be documented. -doy

Fri Jul 12 15:06:26 2013 DOY [...] cpan.org - Correspondence added

Are you interested in coming up with a doc patch?

Fri Jul 12 18:23:38 2013 KENTNL [...] cpan.org - Correspondence added

On 2013-07-13 07:06:26, DOY wrote:
> Are you interested in coming up with a doc patch?

I'll give it a stab.

However, I'm convinced that those 2 function calls should be more similar.

"virtual" stash slots that exist as a quirk of implementation detail is an abstraction detail leaking through to user space, ie: If 'can' exists on a package due to the existence of 'can' somewhere in a higher inherited class, and does not /really/ exist on the package beyond an implementation detail optimisation, then it would be better if we didn't know it existed to begin with.

I'm still not even sure what to call those symbols.

Bug #78273 for Package-Stash: Undocumented inconsistency between unfiltered and filtered output

Preferred bug tracker