evalempire / method-signatures Goto Github PK

Reread the entire POD. Generally looks very good now. A few suggestions:

• The example under "named parameters" uses a class method without $class: in the sig. This might be confusing.
• In the "invocant parameter" section, should we clarify that method signatures have a default of $self:? I.e. func signatures don't.
• Should we mention $METHOD_SIGNATURES_DEBUG under "debugging"? I find that extraordinarily useful, and it works more often than B::Deparse (which often breaks down under the load of MooseX::Declare). Also, there are two debugging sections (one main section, one under "bugs, caveats, and notes") that are very similar ... the second one is probably redundant.
• too_many_args_error should be added under "extending". I will fix this one.
• Typo in the "no source filter" section. I will fix this one as well.
• Am I crazy, or are all the checks you say you'd like to add under "error checking" now added? I think you might be able to remove this section.
• We still have MXMS in the "see also" section ... should we remove it? or at least note that MSM can do what it does (only better ;->)?

Similar to #30, turn the signature as a whole into an object.

The next step after this is likely to make a Method::Signatures object and hang the signature object off that... or just meld them together.

PPI has a bunch of uses of eval which do not localize $@ causing issues for us where $@ is lost.

Get PPI patched to protect $@ in the uses of eval.

Updating our PPI dependency to use that is in question as it will make Method::Signatures harder to use based on vendor supplied PPI packages which are likely to be out of date and it's a very low priority fix.

Okay, now I'm trying it on 5.8.9. I'm seeing two issues:

• t/error_interruption.t has the exact same issue that 5.10 has (see #18).
• t/override_errors.t has some bizarre thing going on where some of the code I'm trying to pass to throws_ok is getting run at compile time instead of eval'ed.

Specifically, there are 4 calls to throw_ok in that file. 1 and 4 are fine. 2 and 3 are being run at compile time, which shouldn't even be possible. First off, if you run it via the debugger, it works perfectly (which reminds me why I hate debuggers, which is why I hardly ever use it in Perl). So I verified that it is in fact running the code blocks at compile time by putting debugging statements wrapped in BEGIN blocks. Then I changed the throws_ok (just those two) to eval's. If I eval them as code blocks, same thing--it throws the exception before the script is even done compiling. If I change them to strings, works perfectly. No clue what's going on there.

I'm considering just changing the throw_ok's to eval's and calling it a day. The tests don't fail, after all; it's just the test script fails because it's not catching the exception to check to see if it is what it should be. But I figured that I ought to ask your opinion before I throw my hands up.

I've got a gist which demonstrates the error.

I'm not sure why I can't reproduce this error with Method::Signatures alone; it seems like it ought to have the same problem, but it doesn't. Only when I'm using Method::Signatures::Modifiers with MooseX::Declare. This leads me to believe that I'm doing something bad (or not doing something good) in MSM, so I'm hesitant to just jump in and "fix" the problem by changing MS's %args to %__METHOD_SIGNATURES_ARGS_WHICH_YOU_SHOULD_NOT_USE or something like that. Seems a bit like cheating anyway.

@schwern: Thoughts?

With a fresh checkout, before changing anything, I get this test failure:

[tyr:~/proj/method-signatures] prove -l t/syntax_errors.t
t/syntax_errors.t .. 1/?
#   Failed test 'Bad syntax generates stack trace'
#   at t/syntax_errors.t line 14.
#                   'BEGIN not safe after errors--compilation aborted at t/lib/Bad.pm line 13.
# Compilation failed in require at t/syntax_errors.t line 9.
# '
#     doesn't match '(?ms-xi:^PPI failed to find statement for '\$bar')'
# Looks like you failed 1 test of 3.
t/syntax_errors.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests

Test Summary Report
-------------------
t/syntax_errors.t (Wstat: 256 Tests: 3 Failed: 1)
  Failed test:  3
  Non-zero exit status: 1
Files=1, Tests=3,  0 wallclock secs ( 0.02 usr  0.01 sys +  0.09 cusr  0.00 csys =  0.12 CPU)
Result: FAIL

I guess this is related to issue #1 somehow. I took a look at the code and the test, but I can't see why I'm getting a different error (but still not the desired error, which is in the TODO block). Should I just ignore this (which is what I've been doing), or do I need to investigate further? (And, if so, any pointers on where to look would be much appreciated.)

We forgot to add a dependency on Any::Moose.
http://www.cpantesters.org/cpan/report/5ff13bd2-54ab-11e0-8a39-a371061ad747

As discussed on p5-syntax, Thing $param is ambiguous because one doesn't know if Thing is supposed to be a type or a class or a role and this can have consequences having to do with the order things are loaded.

Add the traits isa, does and type to unambiguously check that the parameter isa class, does a role or is of the type, respectively. There can be multiple checks on one parameter.

When a function has a syntactically incorrect signature, the error message fails to show the method, file and line.

use Method::Signatures;

func hello ($a, @b, $c) {
    warn $a; warn join(",",@b); warn $c;
}

Use of uninitialized value $method in pattern match (m//) at lib/Method/Signatures/Parser.pm line 220.
Use of uninitialized value $method in pattern match (m//) at lib/Method/Signatures/Parser.pm line 220.
Use of uninitialized value $pack in pattern match (m//) at lib/Method/Signatures/Parser.pm line 220.
Use of uninitialized value $method in concatenation (.) or string at lib/Method/Signatures.pm line 1127.
Use of uninitialized value $file in concatenation (.) or string at lib/Method/Signatures.pm line 1127.
Use of uninitialized value $line in concatenation (.) or string at lib/Method/Signatures.pm line 1127.
In call to (), slurpy parameter @b must come at the end at  line .

This is inside carp_location_for which looks up the stack but at that point Perl is still compiling so the stack doesn't look like it does for a runtime error.

@ = Method::Signatures::Parser::carp_location_for('Method::Signatures') called from file `lib/Method/Signatures.pm' line 1126
. = Method::Signatures::signature_error(ref(Method::Signatures), 'slurpy parameter @b must come at the end') called from file `lib/Method/Signatures.pm' line 927
. = Method::Signatures::_check_signature(ref(Method::Signatures)) called from file `lib/Method/Signatures.pm' line 857
@ = Method::Signatures::parse_func(ref(Method::Signatures), 'proto', ref(ARRAY), 'signature', ref(HASH)) called from file `lib/Method/Signatures.pm' line 782
@ = Method::Signatures::parse_signature(ref(Method::Signatures), 'proto', '$a, @b, $c', 'invocant', undef, 'pre_invocant', undef) called from file `lib/Method/Signatures.pm' line 755
@ = Method::Signatures::parse_proto(ref(Method::Signatures), '$a, @b, $c') called from file `/Users/schwern/perl5/perlbrew/perls/perl-5.14.1/lib/site_perl/5.14.1/darwin-thread-multi-2level/Devel/Declare/MethodInstaller/Simple.pm' line 65
. = Devel::Declare::MethodInstaller::Simple::parser(ref(Method::Signatures), 'func', 0, 1) called from file `/Users/schwern/perl5/perlbrew/perls/perl-5.14.1/lib/site_perl/5.14.1/darwin-thread-multi-2level/Devel/Declare/MethodInstaller/Simple.pm' line 24
. = Devel::Declare::MethodInstaller::Simple::__ANON__[/Users/schwern/perl5/perlbrew/perls/perl-5.14.1/lib/site_perl/5.14.1/darwin-thread-multi-2level/Devel/Declare/MethodInstaller/Simple.pm:24]('func', 0) called from file `/Users/schwern/perl5/perlbrew/perls/perl-5.14.1/lib/site_perl/5.14.1/darwin-thread-multi-2level/Devel/Declare.pm' line 281
. = Devel::Declare::linestr_callback('const', 'func', 0) called from file `/Users/schwern/tmp/test.plx' line 4

The important bit is that the stack never gets out of Method::Signatures or Devel::Declare, all of which are correct to skip for a runtime error but not a compile time one. That last Devel::Declare::linestr_callback has the file and line we want. It's up to us to put the proper package and method name in.

I think carp_location_for needs to be split into runtime and compile time versions.

Do we already have a test for compile time signature errors?

It looks to me like all these are the same:

method foo {}
method foo () {}
method foo (@_) {}

Should they be? Seems there isn't much point to the @_ signature if they are. Or is there some circumstance where they're different that I just didn't run across in my super-simple/stupid tests?

Boy, it's amazing what you can uncover running your code on different machines ...

Okay, remember when I said that method modifiers (only) had to have a "sub " inserted in front of the function body? I handled that by adding a small wrapper around inject_if_block() into MSM. However, I'm seeing a failure on Perl 5.10.0 even on the method keyword in MXD. The weird thing is, it doesn't appear to happen on every "method" ... so far just one. I can't tell what's different about that particular method yet, but I've verified that moving that inject_if_block() wrapper from MSM to MS fixes it. I also backported that to Perl 5.12, where it doesn't seem to change anything there. So it's theoretically not a bad change to make in general. However, since it's changing the way MS operates for everything, everywhere, I thought I'd better run this by you first.

I'm also seeing some minor test failures in 5.10: some error messages are slightly different than expected, and the new test for localizing $@ in the parser is failing. I don't think these are any big deal, but obviously they'll throw up bad reports on CPAN Testers, so I suppose they have to be addressed at some point. I'm just not sure I'm going to have a good opportunity to spend any time on it any time soon.

That is, this code fails:

package Foo;
use Method::Signatures;

method foo ($class: @args) {}

my @a;
Foo->foo(@a);

with this error:

In call to Foo::foo(), missing required argument @args at t line 7.

which is obviously wrong. I'll try to hack up a patch soon.

Fix and failing test case here: barefootcoder/method-signatures@e0cd9bc9897a486e2bb3

Basically, it looks like the fact that there's a pending error in $@ is confusing the eval inside PPI (specifically, PPI::Node's _wanted(), which is called from find_first()). Localizing $@ inside split_proto() fixes the issue.

Its painfully clear in the code that the parsed signature hash should be an object.

This issue is for step one, turn the parsed individual parameter (currently %sig) into an object. Don't worry about a public API, we can hash that out later, make something internally useful.

Use Mouse, we depend on it anyway might as well use it. It will drastically cut down the performance hit of an object.

 Int $Positive_Int where { $_ > 0 };

As an adjunct to the type system, free form constraints.

This is compatible with Perl 6 and MooseX::Method::Signatures.

Okay, handling modifiers adds a whole new wrinkle on the error reporting side, and there are no great solutions to this. Here's the ones I've considered and why they're (potentially) problematic:

1. use Carp with @CARP_NOT
   This seems oh so promising, but it breaks down because of the "trust model" that it (supposedly) represents. I can say that I "trust" my caller, so my caller gets skipped, and I can say that I "trust" the Moose stuff, but that doesn't matter, because the only way Carp will skip Moose (or, more accurately, Class::MOP) is if my caller trusts it. What I say doesn't matter. So the only way to make it work this way is to monkey patch it by jamming things into my caller's @CARP_NOT array (and possibly into Moose's and Class::MOP's as well). Which is icky. Additionally, @CARP_NOT appeared in Perl 5.8.x, so it means it won't work for earlier Perl's, which may or may not be an issue.
2. use Carp with $CarpLevel
   This is doable, but tricky, because the exact level you need to set varies depending on which modifier you're in. Besides, it's deprecated, so we'd have to worry about it breaking in future Perl's.
3. use Carp with %CarpInternal
   This is totally cheating, certainly not recommended, and possibly has side-effects.
4. use Carp::Clan
   I didn't actually think this would work, but then I realized I was thinking about it wrong. Then I tried it, and I realized it doesn't work for a whole 'nother reason. Basically, it's tricky to set it up right, and then it doesn't work anyway, because it only allows you to specify one namespace to skip, and, if you specify that, it doesn't even skip you yourself by default. I think it might be possible to cheat slightly by passing a full-on pattern, something like 'Method::Signatures|Class::MOP|Moose' or somesuch, but I dunno if that's really feasible. Additionally, it jams its own opinion of the caller's sub name onto the front of your error message, and I'm not sure if I can suppress that.
5. screw Carp; roll your own error message
   I never actually realized it, but this is exactly what signature_error is doing. I probably should have just adapted that all along. This is almost certainly the direction I'm going to go in, especially since now the other errors (required param missing, bad named param, etc) are going to end up with the same problem otherwise. The disadvantage here is that other modules which are trying to play nicely and use @CARP_NOT or whatever are going to be ignored by us. Also you can't set $Carp::Verbose (although Carp::Always would still work).

Plus I'm not entirely sure what wrinkles subclassing is going to throw into it (not MSM, but other people subclassing us with arbitrary package names).

If anyone has any thoughts on this, please let me know. I think error reporting is my last outstanding issue ATM.

On a hunch, I contacted Zefram to see if he had any bright ideas about replacing Devel::BeginLift. Turns out, he does.

Michael G Schwern wrote:
> I have two possibilities I wonder if you have any thoughts on.  The first is
> can I use Devel::Declare to insert a BEGIN block around the function
> definition?

You have the problem that "func" or whatever is still seen outside the
BEGIN block, and you can't take that back, but you can turn "func foo {
... }" into something like "func do { BEGIN { *foo = sub { ... }; } };".

>             Possibly ignoring for a moment the hairy bit where I have to find
> the end of the subroutine block...

That's a solved problem.  Some of the code you're invoking from
Devel-Declare is already using the necessary trick, to inject a semicolon
after the block.  The trick is that immediately after the open brace
of the block you inject a BEGIN block, the code in which invokes
B::Hooks::EndOfScope to get code run when the end of the block is seen,
and *that* code injects whatever you need after the block.  You'll need
to do that to inject the close brace of the outer BEGIN block.

I'm going to try and do that in the feature/inject_BEGIN branch.

That func should happen at compile time is pretty clear. It jives with sub and it's not uncommon to write a script where you put all the functions at the bottom.

That method should happen at compile time is less clear. It's rare that one defines a class in the same file as they use it. There are other runtime components which must be resolved for the class to be fully functional, such as has if you're using Mo[ou]se.

So... should method be changed to a runtime thing?

NOTE This used to be about getting rid of Devel::BeginLift and all compile time measures, which is why the discussion might seem a bit odd.

Now that we support type check syntax, we should do something with it by default. The simplest thing to do would be to use Any::Moose to do the type checking. It's a light dependency and Moose compatible.

The following dies with "cannot create after modifier in package Foo for non-existent method foo at...":

use MooseX::Declare;
use Method::Signatures::Modifiers;

role Foo {
    has foo => ( is => 'ro' );
    after foo { }
}

It works with MooseX::Method::Signatures.

t/zero_defaults.t is failing on 5.10.0. The problem is the copy_cat() method. It does not consider an empty string to be a default. The issue is with how $foo ~~ 0 behaves.

copy_cat expands out into this:

sub copy_cat {
    my $self = shift;
    Method::Signatures->required_arg('$this') unless (@_ > 0);
    my $this = $_[0];
    my $that = !(@_ > 1) ? ( $this ) : do {
        no warnings;
        my $arg = $_[1];
        $arg ~~ 0 ? $this : $arg
    };
    Method::Signatures->too_many_args_error(2) if @_ > 2; 

    return $that;
}

On 5.10.0, "" ~~ 0 is false. From 5.10.1 on it's true.

Smart match before 5.10.1 is unstable. Simplest thing to do might be to boost the minimum requirement for that feature to 5.10.1.

Any::Moose will use Moose if anything loads Moose. This has two problems for MS. First, Moose is slower than Mouse. Second, even if your class is using Mouse, MS will use Moose.

Method::Signatures should choose to resolve types using Moose IF AND ONLY IF the current package is using Moose. For everything else, use Mouse.

Another issue will cover a swith to explicitly control which is being used.

This works fine.

func foo( $thing = [1,2,3] when [] ) {
    print @$thing
}

This is a messy parse error.

func foo( $thing = { foo => 23 } when {} ) {
    print keys %$thing;
}

Not enough arguments for grep at /Users/schwern/tmp/test.plx line 3, near "} $arg"
syntax error at /Users/schwern/tmp/test.plx line 3, near "} $arg"
syntax error at /Users/schwern/tmp/test.plx line 5, near "}"
Execution of /Users/schwern/tmp/test.plx aborted due to compilation errors.

Something's gone wrong with the injection.

my $thing = !(@_ > 0) ? ( { foo => 23 } ) : do{ no warnings; my $arg = $_[0]; (grep  {} $arg) ? ( { foo => 23 } ) : $arg}; Method::Signatures->too_many_args_error(1) if @_ > 1; 

This gist implements the problem

There seems to be a problem with loading Method::Signatures when roles get involved in the order things are getting loaded. perl -c on OuterUserWorks.pm goes fine, but on OuterUserBroken.pm I get

Couldn't load class (InnerRole) because: Devel::Pragma: scope overflow at /home/cmckay/perl5/perlbrew/perls/perl-5.14.0/lib/site_perl/5.14.0/i686-linux-thread-multi/Devel/Pragma.pm line 34.
BEGIN failed--compilation aborted at InnerRole.pm line 3.
Compilation failed in require at /home/cmckay/perl5/perlbrew/perls/perl-5.14.0/lib/site_perl/5.14.0/i686-linux-thread-multi/Class/MOP.pm line 117.
 at /home/cmckay/perl5/perlbrew/perls/perl-5.14.0/lib/site_perl/5.14.0/i686-linux-thread-multi/Class/MOP.pm line 123
        Class::MOP::__ANON__('Devel::Pragma: scope overflow at /home/cmckay/perl5/perlbrew/...') called at /home/cmckay/perl5/perlbrew/perls/perl-5.14.0/lib/site_perl/5.14.0/Try/Tiny.pm line 100
        Try::Tiny::try('CODE(0x8ab3e40)', 'Try::Tiny::Catch=REF(0x924c2f8)') called at /home/cmckay/perl5/perlbrew/perls/perl-5.14.0/lib/site_perl/5.14.0/i686-linux-thread-multi/Class/MOP.pm line 128
        Class::MOP::load_first_existing_class('InnerRole') called at /home/cmckay/perl5/perlbrew/perls/perl-5.14.0/lib/site_perl/5.14.0/i686-linux-thread-multi/Class/MOP.pm line 140
        Class::MOP::load_class('InnerRole', undef) called at /home/cmckay/perl5/perlbrew/perls/perl-5.14.0/lib/site_perl/5.14.0/i686-linux-thread-multi/Moose/Util.pm line 131
        Moose::Util::_apply_all_roles('Moose::Meta::Class=HASH(0x9205650)', undef, 'InnerRole') called at /home/cmckay/perl5/perlbrew/perls/perl-5.14.0/lib/site_perl/5.14.0/i686-linux-thread-multi/Moose/Util.pm line 98  
        Moose::Util::apply_all_roles('Moose::Meta::Class=HASH(0x9205650)', 'InnerRole') called at /home/cmckay/perl5/perlbrew/perls/perl-5.14.0/lib/site_perl/5.14.0/i686-linux-thread-multi/Moose.pm line 65
        Moose::with('Moose::Meta::Class=HASH(0x9205650)', 'InnerRole') called at /home/cmckay/perl5/perlbrew/perls/perl-5.14.0/lib/site_perl/5.14.0/i686-linux-thread-multi/Moose/Exporter.pm line 356
        Moose::with('InnerRole') called at OuterImplementor.pm line 4
        require OuterImplementor.pm called at OuterUserBroken.pm line 4
        OuterUserBroken::BEGIN() called at OuterImplementor.pm line 0
        eval {...} called at OuterImplementor.pm line 0
Compilation failed in require at OuterUserBroken.pm line 4.
BEGIN failed--compilation aborted at OuterUserBroken.pm line 4.

Moving the Method::Signatures load to after the class using the role as seen in OuterUserWorks.pm makes all the problems go away. Originally came across this issue using Method::Signatures::Modifiers with MooseX::Declare and eventually reduced it to this implementation of the error. Unfortunately The magic of all these mixed together is beyond my understanding so I can't figure out any more than it's broken in this manner.

Tested on 2 platforms:

Summary of my perl5 (revision 5 version 14 subversion 0) configuration:

  Platform:
    osname=linux, osvers=2.6.18-238.9.1.el5, archname=i686-linux-thread-multi
    uname='linux cmckay.devve.iseek.com.au 2.6.18-238.9.1.el5 #1 smp tue apr 12 18:10:56 edt 2011 i686 i686 i386 gnulinux '
    config_args='-de -Dprefix=/home/cmckay/perl5/perlbrew/perls/perl-5.14.0 -Dusethreads'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=undef, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.1.2 20080704 (Red Hat 4.1.2-50)', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    libc=/lib/libc-2.5.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.5'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector'


Characteristics of this binary (from libperl): 
  Compile-time options: MULTIPLICITY PERL_DONT_CREATE_GVSV
                        PERL_IMPLICIT_CONTEXT PERL_MALLOC_WRAP
                        PERL_PRESERVE_IVUV USE_ITHREADS USE_LARGE_FILES
                        USE_PERLIO USE_PERL_ATOF USE_REENTRANT_API
  Built under linux
  Compiled at May 16 2011 11:27:19
  %ENV:
    PERLBREW_PATH="/home/cmckay/perl5/perlbrew/bin:/home/cmckay/perl5/perlbrew/perls/perl-5.14.0/bin"
    PERLBREW_PERL="perl-5.14.0"
    PERLBREW_ROOT="/home/cmckay/perl5/perlbrew"
    PERLBREW_VERSION="0.20"
    PERL_AUTOINSTALL="--alldeps"
  @INC:
    /home/cmckay/perl5/perlbrew/perls/perl-5.14.0/lib/site_perl/5.14.0/i686-linux-thread-multi
    /home/cmckay/perl5/perlbrew/perls/perl-5.14.0/lib/site_perl/5.14.0
    /home/cmckay/perl5/perlbrew/perls/perl-5.14.0/lib/5.14.0/i686-linux-thread-multi
    /home/cmckay/perl5/perlbrew/perls/perl-5.14.0/lib/5.14.0
    .

with

• Method::Signatures 20111020
• Moose 2.0205

and

Summary of my perl5 (revision 5 version 8 subversion 8) configuration:
  Platform:
    osname=linux, osvers=2.6.18-53.el5,
    archname=x86_64-linux-thread-multi
    uname='linux builder10.centos.org 2.6.18-53.el5 #1 smp mon nov 12
    02:14:55 est 2007 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Doptimize=-O2 -g -pipe -Wall
    -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
    --param=ssp-buffer-size=4 -m64 -mtune=generic -Dversion=5.8.8
    -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc
    -Dcf_by=Red Hat, Inc. -Dinstallprefix=/usr -Dprefix=/usr
    -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64
    -Dprivlib=/usr/lib/perl5/5.8.8
    -Dsitelib=/usr/lib/perl5/site_perl/5.8.8
    -Dvendorlib=/usr/lib/perl5/vendor_perl/5.8.8
    -Darchlib=/usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi
    -Dsitearch=/usr/lib64/perl5/site_perl/5.8.8/x86_64-linux-thread-multi
    -Dvendorarch=/usr/lib64/perl5/vendor_perl/5.8.8/x86_64-linux-thread-multi
    -Darchname=x86_64-linux-thread-multi -Dvendorprefix=/usr
    -Dsiteprefix=/usr -Duseshrplib -Dusethreads -Duseithreads
    -Duselargefiles -Dd_dosuid -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm
    -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio
    -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly
    -Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto
    -Ud_endhostent_r_proto -Ud_sethostent_r_proto
    -Ud_endprotoent_r_proto -Ud_setprotoent_r_proto
    -Ud_endservent_r_proto -Ud_setservent_r_proto
    -Dinc_version_list=5.8.7 5.8.6 5.8.5 -Dscriptdir=/usr/bin'
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=define use5005threads=undef useithreads=define
    usemultiplicity=define
    useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
    use64bitint=define use64bitall=define uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing
    -pipe -Wdeclaration-after-statement -I/usr/local/include
    -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm',
    optimize='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
    -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe
    -Wdeclaration-after-statement -I/usr/local/include
    -I/usr/include/gdbm'
    ccversion='', gccversion='4.1.2 20080704 (Red Hat 4.1.2-50)',
    gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t',
    lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='gcc', ldflags =''
    libpth=/usr/local/lib64 /lib64 /usr/lib64
    libs=-lresolv -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread
    -lc
    perllibs=-lresolv -lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    libc=, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.5'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E
    -Wl,-rpath,/usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi/CORE'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -g -pipe -Wall
    -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
    --param=ssp-buffer-size=4 -m64 -mtune=generic'


Characteristics of this binary (from libperl):
  Compile-time options: MULTIPLICITY PERL_IMPLICIT_CONTEXT
                        PERL_MALLOC_WRAP USE_64_BIT_ALL USE_64_BIT_INT
                        USE_ITHREADS USE_LARGE_FILES USE_PERLIO
                        USE_REENTRANT_API
  Built under linux
  Compiled at Jun 13 2011 05:55:31
  %ENV:
    PERL5LIB="/home/bkolera/lib/perl5/lib/perl5/x86_64-linux-thread-multi:/home/bkolera/lib/perl5/lib/perl5"
    PERL_LOCAL_LIB_ROOT="/home/bkolera/lib/perl5"
    PERL_MB_OPT="--install_base /home/bkolera/lib/perl5"
    PERL_MM_OPT="INSTALL_BASE=/home/bkolera/lib/perl5"
  @INC:
    /home/bkolera/lib/perl5/lib/perl5/x86_64-linux-thread-multi
    /home/bkolera/lib/perl5/lib/perl5/x86_64-linux-thread-multi
    /home/bkolera/lib/perl5/lib/perl5
    /usr/lib64/perl5/site_perl/5.8.8/x86_64-linux-thread-multi
    /usr/lib/perl5/site_perl/5.8.8
    /usr/lib/perl5/site_perl
    /usr/lib64/perl5/vendor_perl/5.8.8/x86_64-linux-thread-multi
    /usr/lib/perl5/vendor_perl/5.8.8
    /usr/lib/perl5/vendor_perl
    /usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi
    /usr/lib/perl5/5.8.8
    .

with

• Method::Signatures 20110923.1726
• Moose 2.0007

A better name for $sig->{exists} which is a chunk of code to check for the existence of a parameter.

See barefootcoder@fa3bbd3#commitcomment-419433

Putting this here to remind me to do it after I'm done reviewing.

Our ad hoc parser only works as long as there's nothing balanced we need to parse. Defaults need balanced parsing, so they use PPI. where needs parsing, too.

We already run everything through PPI once to split the list of prototypes (to account for complicated defaults), and we'll need to do it again to differentiate where from defaults.

So we might as well do it all in PPI.

This will use PPI to tokenize the structure and then we walk it. Hopefully PPI will tokenize the signature without modification, but we might have to make a subclass. @barefootcoder has been looking into it. We've been talking about it in email, but here is better so @chipdude can chime in.

CC: @barefootcoder

The new alpha is working well, so I want to make a release. But I also want our debugger test fixed on 5.13. Devel::Declare just got a patch to fix that.
https://rt.cpan.org/Ticket/Display.html?id=67022

So once that's out I can release a stable. Good?

Update the change log with what's changed since the last alpha.

Adapting Michael Schwern's benchmarking script mentioned in a blog post, I used it to test Method::Signatures type constraints. According to Method::Signature's documentation, the "run-time penalty if you do declare types should be very similar to using Mouse::Util::TypeConstraints (or Moose::Util::TypeConstraints) directly." I may have made a mistake in my own testing, but this appears to not be the case:

Testing Perl 5.014002, Method::Signatures 20111020, Moo 0.009011, Object::InsideOut 3.84, Mouse 0.97, Moose 2.0205
Benchmark: timing 6000000 iterations of Method::Signatures, Moo, Mouse, Object::InsideOut, manual...
Method::Signatures: 17 wallclock secs (17.10 usr +  0.00 sys = 17.10 CPU) @ 350877.19/s (n=6000000)
Moo:                10 wallclock secs (10.42 usr +  0.00 sys = 10.42 CPU) @ 575815.74/s (n=6000000)
Mouse:              2 wallclock secs ( 1.97 usr +  0.00 sys =  1.97 CPU) @ 3045685.28/s (n=6000000)
Object::InsideOut:  5 wallclock secs ( 5.56 usr +  0.00 sys =  5.56 CPU) @ 1079136.69/s (n=6000000)
manual:             11 wallclock secs (10.55 usr +  0.00 sys = 10.55 CPU) @ 568720.38/s (n=6000000)
Moose:              12 wallclock secs (12.26 usr +  0.00 sys = 12.26 CPU) @ 489396.41/s (n=6000000)

In order to get Method::Signatures to use Mouse, I commented out Moose and then reran the test with it included (incidentally, Method::Signature's time jumped to 30 seconds when it used Moose). My fork of the benchmark script can be found at https://gist.github.com/1321873 .

This appears to have been caused by 4e03081 which moved injected_if_block "from MSM to MS".

This can be reproduced with:

perl -MMethod::Signatures -e '[method () {},1]'

Reverting 4e03081 fixes the error.

Hi,

I've implemented subj. Didn't expect it to be so easy. Only a few tests and no docs. Will accept this new feature for inclusion? Example from tests:

# Test that undef, 0 and '' override defaults
method echo($message = 'what?') { return $message }
is( Stuff->echo(),          'what?' );
is( Stuff->echo(undef),     undef   );  
is( Stuff->echo(''),        ''      );  
is( Stuff->echo(0),         0       );  
is( Stuff->echo('who?'),    'who?'  );  

# Test defaults modifiers
method echo_or ($message ||= 'what?') { return $message }
is( Stuff->echo_or(),          'what?' );
is( Stuff->echo_or(undef),     'what?' );
is( Stuff->echo_or(''),        'what?' );
is( Stuff->echo_or(0),         'what?' );
is( Stuff->echo_or('who?'),    'who?'  );  

method echo_dor($message //= 'what?') { return $message }
is( Stuff->echo_dor(),          'what?' );
is( Stuff->echo_dor(undef),     'what?' );
is( Stuff->echo_dor(''),        ''      );  
is( Stuff->echo_dor(0),         0       );  
is( Stuff->echo_dor('who?'),    'who?'  );  

method echo_eor($message ''= 'what?') { return $message }
is( Stuff->echo_eor(),          'what?' );
is( Stuff->echo_eor(undef),     'what?' );
is( Stuff->echo_eor(''),        'what?' );
is( Stuff->echo_eor(0),         0       );  
is( Stuff->echo_eor('who?'),    'who?'  );  

into is a weird feature, injecting signatures into another package/scope. It plays badly with lexical options like compile_at_BEGIN. Do we need it? What is its uses and do they have alternatives?

I'm going to withhold documenting this in a stable release until this is resolved.

While working on my talk, I realized that what I was doing in the import for MSM was using two different objects for parsing, depending on whether it's a method or a method modifier being declared. For the former, I'm instantiating an object of class BASE. For the latter, I'm instantiating an object of class PACKAGE. So I realized that that's not extensible, and I really should be using $class that I pull out of import()'s argument list. But using BASE isn't extensible either. For instance, if I subclass MSM and, let's say, override type_error(), I'll get my new errors for method modifiers, but not for regular methods. That's bad.

The problem is basically that I made MSM understand how to do modifiers but not how to do "method". (Remember way back on April 6, when I laid out those four choices?) So I have to delegate the "method" keyword back to base Method::Signatures. But that doesn't work for subclassing.

So I think the proper fix is to go back to our four choices and choose # 3 (instead of # 2): that is, make MSM be able to handle everything. I think I can make code_for() handle both, probably by going back to my extra "methodmodifier" flag idea (which I had taken out when we went to the subclass because it wasn't needed with choice # 2), the only other choice being to make it dependent on the actual "method" declarator, which I can already tell I'll regret one day. All my other ideas for fixing it are moderately insane (like giving the user the ability to create a layer in between MS and MSM by calculating the parent at import time, or moving the code_for() branching back into MS). So, do you agree that this is the right way?

t/debugger.t ................... 1/? perl: pad.c:1525: Perl_cv_clone: Assertion `depth || ((svtype)((proto)->sv_flags & 0xff)) == SVt_PVFM' failed.
Aborted
t/debugger.t ................... 5/?
#   Failed test 'debugger'
#   at t/debugger.t line 31.
#          got: ''
#     expected: '42'
# Looks like you failed 1 test of 6.
t/debugger.t ................... Dubious, test returned 1 (wstat 256, 0x100)

Summary of my perl5 (revision 5 version 12 subversion 1) configuration:

  Platform:
    osname=linux, osvers=2.6.32-26-server, archname=x86_64-linux
    uname='linux foo 2.6.32-26-server #48-ubuntu smp wed nov 24 10:28:32 utc 2010 x86_64 gnulinux '
    config_args='-des -Duseshrplib -Duselargefiles -Duse64bitint -Duse64bitall -Doptimize=-march=core2 -O2 -g3 -Wall -pipe -Dprefix=/opt/perl -Dvendorprefix=/opt/perl'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-march=core2 -O2 -g3 -Wall -pipe',
    cppflags='-DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.4.3', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib /lib64 /usr/lib64
    libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
    libc=/lib/libc-2.11.1.so, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.11.1'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/opt/perl/lib/5.12.1/x86_64-linux/CORE'
    cccdlflags='-fPIC', lddlflags='-shared -march=core2 -O2 -g3 -Wall -pipe -L/usr/local/lib -fstack-protector'

Characteristics of this binary (from libperl):
  Compile-time options: DEBUGGING PERL_DONT_CREATE_GVSV PERL_MALLOC_WRAP
                        USE_64_BIT_ALL USE_64_BIT_INT USE_LARGE_FILES
                        USE_PERLIO USE_PERL_ATOF
  Built under linux
  Compiled at Jan 20 2011 12:15:02

I propose we raise the minimum dependency to 5.10.1.

I don't remember why I made 5.8.1 the minimum, but I don't have any need for it, and I don't test it much any more. Amazingly enough, aside from some silly test mistakes in line numbers, it still works. I'm going to try out travis-ci.org to do CI testing and they don't provide a 5.8.

I'd go with 5.10.1, 5.10.0 has annoying bugs.

I have been using MooseX::Declare with Method::Signatures::Modifiers for quite a while now, mostly for prototyping my new module. As I am getting closer to release however, I have been removing MX::D to use regular Moose. But then I had a shock! I hadn't realized that the modifiers are deeply connected to MX::D.

To restate, Method::Signature doesn't provide the modifiers at all. Method::Signatures::Modifiers only works from within MX::D. What would be involved in trying to get a the modifiers to work from regular Moose? Is it even possible? Could this be added to M::S or M::S::M or even another subordinate (or separately released) module?

method and func both happen at compile time, just like sub. This really annoys some people and it's incompatible with MooseX::Method::Signatures.

Make an option so that you can turn this behavior off.

@schwern: I'm planning to release on Wed unless you have any objections. Let me know if you want me to delay for any reason.

Two scenarios here:

• A comment in between elements of the signature, such as when the signature is split across multiple lines.
• A comment between the end of the signature and the opening curly brace of the method.

I have a fix for the former, although I haven't checked it in yet. I don't believe it would fix the latter problem, though. I'm currently trying to come up with a test case to reproduce the failure, but so far I haven't been able to get it exactly right.

This (and the previous issue) came up while integrating the current developer release of MS into our existing codebase.

The EXPERIMENTING section probably needs a rewrite. It refers to make_proto_unwrap, which doesn't apparently exixst any more. It also says this:

This interface is experimental, unstable and will change between versions.

We may want to soften that somewhat.

Right now, if you pass in a named parameter that you didn't declare, that's an error. However, the same is not true of positional parameters. For instance:

use Method::Signatures;

func foo($a, $b) {}

foo(1, 2, 3);

does not generate an error. Should it?

@thoughtstream mused...

Maybe we can have:

sub foo ($arg = 'default')               # existence test
sub foo ($arg = 'default' when undef)    # default if $arg ~~ undef
sub foo ($arg = 'default' when '')       # default if $arg ~~ ''
sub foo ($arg = 'default' when 0)        # default if $arg ~~ 0
sub foo ($arg = 'default' when * < 0 )   # default if $arg < 0

Just wanted to put there here before i forgot it: On my primary machine, I'm running Mouse 0.90. I tried a different machine using an older Mouse (0.44), and one of the tests fails. Upgrading Mouse (in this case it went to 0.92) fixes the problem.

I'm not sure how crucial this particular test failure is (it properly throws an error, just not the error I was expecting), but I'm also not sure how much effort I want to put into accounting for it when we can just up our dependent version and call it a day. So I'm leaving it up to you (Schwern) as to how to proceed on this issue.

As part of pull request #56, I asked @thoughtstream why \ (to make a parameter an alias for the passed-in variable) could not be combined with : (to indicate a named parameter). He replied:

The problem is that, without the check, MS current would unpack named and aliased parameters using code like this:

my %args = @_[0..$#_]; 
alias my $named_and_aliased = delete $args{named_and_aliased};

In other words, named args are copied into %args, then deleted (i.e. copied) out of it again. So you're aliasing a copy of a copy, not the original argument.

The generated code would have to be changed to something like:

my %args; 
alias $args{shift @_} = shift @_ while @_;
my $named_and_aliased = $args{named_and_aliased};
delete $args{named};

Feel free to do that if you like. It would certainly be a vast improvement.
(PS: Untested code above...don't rely on it!)

Damian

This should be fixed, and Damian's code added and tested, because the current restriction is unnecessary.

Okay, big one: I can't replace MooseX::Method::Signatures inside MooseX::Declare with Method::Signatures, because MS doesn't work with method modifiers (e.g. before, after, around).

I thought I had tested this previously, but apparently I was doing something wrong, because it just doesn't work. Here are the problems I've identified which must be fixed:

1. 'around' needs an $orig, which the Moose people have thoughtfully put in @_ before $self, theoretically just to mess with people like us.
2. The anonymous sub created by Devel::Declare::MethodInstaller::Simple's code_for() is totally wrong for a modifier; it needs to call add_whatever_modifier() instead of whatever the hell it's doing now.
3. I can't seem to make it work without sneaking a 'sub' into the generated declaration, which is slightly tough to do from inside MS. (Why this is not needed for 'method', I have no idea. I suspect there's a magical protoype being set somewhere.)
4. The error messages now all report as coming from Class::MOP::Method::Wrapped.

I think I have addressed the first 3 of these problems at least. The basic approach involves subclassing MS and creating a new class, Method::Signatures::Modifiers, which only handles method modifiers. This approach (and name) was arrived at after consultation with Schwern.

The idea is that you should be able to use it one of two different ways:

use MooseX::Declare;
use Method::Signatures::Modifiers;

or

use MooseX::Declare;

class My::Declare extends MooseX::Declare
{
    sub import
    {
        use Method::Signatures::Modifiers;
        goto &MooseX::Declare::import;
    }
}

followed by simply

use My::Declare;

(That may not be exactly right, but it's the right gist.)

Fixes for problems 1-3 are mostly here: barefootcoder/method-signatures@7011bac8d30b4d977e6c, with slight tweaks here: barefootcoder/method-signatures@56eae9fb3e7a2bfc8afd and here barefootcoder/method-signatures@8e8b35d471e008ba2ff2.

First thing to note is that MSM only handles the modifiers; note that MSM's import uses MS to handle the method keyword. Also note that my code to replace MXMS inside MXD is a bit hacky; it does the following admittedly Bad Things(tm):

• It does a rebless. This is horrible. But MXMS hasa context, whereas MS isa context, so I couldn't really figure out how else to do it.
• It uses a private accessor. It turns out that the context that MXMS has isn't even a Devel::Declare::Context::Simple; it's something which contains a DDCS. So I have to get that out, but there's no public accessor for it.
• It just jams the invocant in there. Since I'm not going through MS's import, I need to get that in there somehow.
• The first thing MS's parser() (which is really DDMIS's parser()) does is set its declarator and offset to the arguments it's passed. But the context passed in by MXD already has those set. MXMS handles this by having an 'initialized_context' flag, but I can't do that, because it would mean changing DDMIS. So I'm pulling out the declarator and offset, then passing them to parser(), where they'll be reset to what they already were. Inefficient, obviously.

However, it does the following Good Thing(tm):

• It works.

Better ideas always welcome, of course.

Internally, it solves the problems above as follows:

1. I hacked a "pre-invocant" into MS for the special case of around. I don't necessarily like the name, but I like it better than MXMS's, which is "prototype_injections".
2. I overrode code_for() in MSM and I do not call SUPER::code_for(). I just create an entirely different method which calls the appropriate add_X_modifier method on the metaclass. Additionally, I can't use BeginLift here, or else it tries to add the method modifier before resolving the actual method (in the case where said method comes from a superclass or role, which is most of the time).
3. I overrode inject_if_block and just stuffed "sub " into $before. It's inelegant, but, again, it works.

The error messages I'm still working on (probably raise a separate issue for that).

perl5i has the ability to introspect signatures to ask questions like "how many positional parameters do you take?" This is useful for creating things like loops that now how many elements to iterate at a time.

my @names = ("Joe", "Smith", "Jim", "Dandy", "Jane", "Lane");
@names->foreach( func($fname, $lname) {
    say "Person: $fname $lname";
});

perl5i's signature API is very simple right now. You can ask it for the original signature, a list of parameters, how many positional vs named parameters there are, what the invocant is and if its a method.

Method::Signatures can do that and more. It will fall out of what we need internally for #30 and #31.

The functional requirement is to allow perl5i to use Method::Signatures, but I think its very useful.

In issue #54, @larryl gave us a way for subclasses to change the default behavior of run-time errors. In that issue, @schwern asked:

Perhaps it can be extended to cover other error messages we have?

We only have three calls to die in the module (not counting the one in Method::Signatures::Modifiers, which I think is a separate issue):

1. run-time errors
2. compile-time errors
3. pre-existing errors at parse time, which we rethrow

So, 1) is handled by #54. 3) we should probably not pass to our subclasses (given that it doesn't come from us). So that just leaves 2).

Compile-time errors are handled by sig_parsing_error in Method::Signatures::Parser. We could just change that die to a call to signature_error_handler and call it a day. However, there are three considerations that I'd like to clear up first.

• It means passing $class down into some places where it currently isn't. Probably not a big deal, but a bit of extra work.
• Just because we could doesn't mean we should. Should we, in fact, allow subclasses to grab compile-time (i.e. signature parsing) errors?
• Assuming we should, should we then modify signature_error_handler to be able to distinguish run-time from compile-time errors? I can imagine a subclass wanting to handle those differently.

I'm going to go ahead and get the fix for #54 live, so this is not a gating factor or anything. But I'd like some further discussion before proceeding with adding compile-time errors to this.

Title says it all. I have a fix here: barefootcoder/method-signatures@b4b865815927b6dadaf3, slightly modified here: barefootcoder/method-signatures@fa3bbd3b414c1c0e8cf0.

Add a lexical, compile time switch to allow the user to choose which type system will be used, Moose or Mouse (or a 3rd thing we may add in the future). This would override Any::Moose.

It could be used both for performance and for a non-M[ou]se class to decide which set of types it wants to use.

Related to #34.

@barefootcoder does bring up an important issue in his blog post with his around example: sometimes people will add more arguments to your method. If these are named arguments (ie. not positional and optional) then they're probably safe to ignore. Right now he's using (@_) to cover that, but that only works if your method takes no other arguments.

Do we need an option which ways "ignore further named arguments"?