Bug #79901 for Monkey-Patch: Monkey::Patch causes "Prototype mismatch" warnings

Fri Sep 28 04:31:45 2012 niels [...] thykier.net - Ticket created

Subject:	Monkey::Patch causes "Prototype mismatch" warnings
Date:	Fri, 28 Sep 2012 10:31:25 +0200
To:	bug-Monkey-Patch [...] rt.cpan.org
From:	Niels Thykier <niels [...] thykier.net>

Hi I happened to notice #677145[1] in the Debian bug tracker and decided to forward it upstream. Jakub Wilk (the original reporter) found that when monkey patching a prototyped sub, it causes warnings (similar to the "sub redefined" warning that Monkey::Patch also suppresses). I have attached the test script and that patch submitted by Jakub, which silences the warning. I was thinking it might be prudent if the "patching" sub automatically got the prototype of the original sub. Just in case code loaded "later" depends on this prototype. Though I have no idea of how to determine the prototype of the original sub. The bug was filed against 0.03 in the Debian bug tracker. Perl 5.14.2 was installed on the system from where it was reported. If you want, I can give you the output of "perl -V", but the bug does not really seem to be system specific. If you need any additional information, let me know. ~Niels [1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=677145 You can submit information to the Debian bug by mailing 677145@bugs.debian.org Though the submitter will not see that by default, so if you want the submitter to see it, ALSO mail 677145-submitter@bugs.debian.org PS: I am not auto CC'ed to the Debian bug (I am not the maintainer of your package in Debian) nor am I the submitter, so please CC me directly if needed.

Message body is not shown because sender requested not to inline it.

Fri Sep 28 11:17:40 2012 FRODWITH [...] cpan.org - Correspondence added

This warning should be emitted if you overwrite a sub with a sub with a different prototype. If you don't want the warning, use a sub with the correct prototype, or squelch the warning yourself. I'd recommend the former, since using subs with mismatching prototypes can (and often does) significantly alter the way the sub is called.

Fri Sep 28 11:17:41 2012 The RT System itself - Status changed from 'new' to 'open'

Fri Sep 28 11:17:41 2012 FRODWITH [...] cpan.org - Status changed from 'open' to 'rejected'

Wed Sep 25 04:28:31 2013 niels [...] thykier.net - Correspondence added

Subject:	Re: [rt.cpan.org #79901] Monkey::Patch causes "Prototype mismatch" warnings
Date:	Wed, 25 Sep 2013 10:28:14 +0200
To:	bug-Monkey-Patch [...] rt.cpan.org
From:	Niels Thykier <niels [...] thykier.net>

On 2012-09-28 17:17, Paul Driver via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=79901 > > > This warning should be emitted if you overwrite a sub with a sub with a > different prototype. If you don't want the warning, use a sub with the > correct prototype, or squelch the warning yourself. I'd recommend the > former, since using subs with mismatching prototypes can (and often does) > significantly alter the way the sub is called. >

Hi, First off, sorry for the very late reply. Secondly, your recommendation does not appear to work. The warning is not triggered by the sub to Monkey::Patch, but by the wrapper generated by Monkey::Patch. Consider the "install" and the "wrapper" sub here for a moment: """ sub install { my $self = shift; my $name = $self->name; my $stack = $self->stack; [...] my $code = $self->wrapper; no warnings 'redefine'; *$name = $code; push(@$stack, $code); return $self; } sub wrapper { my $self = shift; unless ($self->{wrapper}) { weaken($self); $self->{wrapper} = sub { [...] }; } return $self->{wrapper}; } """ The sub generated in "wrapper" does not have a prototype (regardless of whether $self->{code} has one and "install" does not "fix" that problem. So Monkey::Patch causes a "Prototype mismatch" here, even if I gave a properly prototyped sub. Btw, I think fixing the prototype problem in Monkey::Patch is "just" something like: use Scalar::Util qw(set_prototype); my $wrapper = sub { [...] }; my $prototype = prototype($self->{code}); if (defined $prototype) { set_prototype(\&$wrapper); } $self->{wrapper} = $wrapper; ~Niels

Wed Sep 25 05:37:03 2013 niels [...] thykier.net - Correspondence added

Subject:	Re: [rt.cpan.org #79901] Monkey::Patch causes "Prototype mismatch" warnings
Date:	Wed, 25 Sep 2013 11:36:43 +0200
To:	bug-Monkey-Patch [...] rt.cpan.org
From:	Niels Thykier <niels [...] thykier.net>

On 2013-09-25 10:28, Niels Thykier wrote: Show quoted text

> On 2012-09-28 17:17, Paul Driver via RT wrote:

>> <URL: https://rt.cpan.org/Ticket/Display.html?id=79901 > >> >> This warning should be emitted if you overwrite a sub with a sub with a >> different prototype. If you don't want the warning, use a sub with the >> correct prototype, or squelch the warning yourself. I'd recommend the >> former, since using subs with mismatching prototypes can (and often does) >> significantly alter the way the sub is called. >>

> > Hi, > > [...] > Btw, I think fixing the prototype problem in Monkey::Patch is "just" > something like: > > use Scalar::Util qw(set_prototype); > > my $wrapper = sub { > [...] > }; > my $prototype = prototype($self->{code}); > if (defined $prototype) { > set_prototype(\&$wrapper); > } > $self->{wrapper} = $wrapper; > > ~Niels > >

Okay, I was reminded that the fix cannot be quite as simple, since the sub provided to monkey patch has to accept an extra "initial" argument (the $original or "previous" sub). So the sub given to Monkey::Patch cannot have the correct prototype[1]. ~Niels [1] Well, technically, it could because I think Monkey::Patch's call to it bypasses prototype checking. However, I am pretty sure most people would be surprised with code like: Monkey::Patch::patch_packagee 'pkg' => 'orig_sub' => sub ($) { my ($code, $arg) = @_; ...; };