You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
sub prototypes not tracking refalias \&foo = \&bar #16987
On Tue, 30 Apr 2019 16:50:20 -0700, jim.avera@gmail.com wrote:
Not sure whether this is intended behavior, but just in case it's
not...
The refalias
\&foo = \&bar
is documented as "foo() now means bar()", but it doesn't entirely:
In subsequently-compiled code, the compiler still wants calls
to foo() to match the old foo's prototype, not the prototype of bar.
#! /usr/bin/perl
use strict; use warnings; use v5.22.0;
use feature 'refaliasing';
no warnings 'experimental::refaliasing';
sub foo($$$$) { }
sub bar($) { }
BEGIN{ \&foo = \&bar; }
foo(1); # Gets "Not enough arguments for main::foo"
It's worse than that:
./perl -Ilib -Mfeature=refaliasing -e 'sub foo { print "foo\n" } sub bar { print "bar\n" } \&foo = \&bar; $n = "foo"; &$n'
Aliasing via reference is experimental at -e line 1.
foo
It seems to be caused by the optimization of storing coderefs as GVs - pp_refassign is expecting a GV on the left for your sample code, but gets the CVref instead and so nothing happens:
switch (left ? SvTYPE(left) : 0) {
case 0:
{
SV * const old = PAD_SV(ARGTARG);
PAD_SETSV(ARGTARG, SvREFCNT_inc_NN(SvRV(sv)));
SvREFCNT_dec(old);
if ((PL_op->op_private & (OPpLVAL_INTRO|OPpPAD_STATE))
== OPpLVAL_INTRO)
SAVECLEARSV(PAD_SVl(ARGTARG));
break;
}
case SVt_PVGV:
if (PL_op->op_private & OPpLVAL_INTRO) {
S_localise_gv_slot(aTHX_ (GV *)left, type);
}
gv_setref(left, sv);
SvSETMAGIC(left);
break;
case SVt_PVAV:
assert(key);
if (UNLIKELY(PL_op->op_private & OPpLVAL_INTRO)) {
S_localise_aelem_lval(aTHX_ (AV *)left, key,
SvCANEXISTDELETE(left));
}
av_store((AV *)left, SvIV(key), SvREFCNT_inc_simple_NN(SvRV(sv)));
break;
case SVt_PVHV:
if (UNLIKELY(PL_op->op_private & OPpLVAL_INTRO)) {
assert(key);
S_localise_helem_lval(aTHX_ (HV *)left, key,
SvCANEXISTDELETE(left));
}
(void)hv_store_ent((HV *)left, key, SvREFCNT_inc_simple_NN(SvRV(sv)), 0);
}
On Tue, 30 Apr 2019 16:50:20 -0700, jim.avera@gmail.com wrote:
Not sure whether this is intended behavior, but just in case it's
not...
The refalias
\&foo = \&bar
is documented as "foo() now means bar()", but it doesn't entirely:
In subsequently-compiled code, the compiler still wants calls
to foo() to match the old foo's prototype, not the prototype of bar.
#!/usr/bin/perl
use strict; use warnings; use v5.22.0;
use feature 'refaliasing';
no warnings 'experimental::refaliasing';
sub foo($$$$) { }
sub bar($) { }
BEGIN{ \&foo = \&bar; }
foo(1); # Gets "Not enough arguments for main::foo"
Fix attached.
With the fix your code produces:
$ ./perl -Ilib ../134072.pl
Subroutine main::foo redefined at ../134072.pl line 11.
Prototype mismatch: sub main::foo ($$$$) vs ($) at ../134072.pl line 11.
bar
From 9b8b423a9084a4e7ed3f6b704a88801fca6ce555 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 15 May 2019 15:59:49 +1000
Subject: (perl #134072) allow \&foo = \&bar to work in main::
subs in main:: are stored as a RV referring to a CV as a space
optimization, but the pp_refassign code expected to find a glob,
which made the assignment a no-op.
Fix this by upgrading the reference to a glob in the refassign check
function.
Note that this would be an issue in other packages if 1e2cfe157ca
was reverted (allowing the space savings in other packages too.)
---
op.c | 9 +++++++++
t/op/lvref.t | 15 ++++++++++++++-
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/op.c b/op.c
index 74761ca421..2e20dc886d 100644
--- a/op.c+++ b/op.c@@ -12465,7 +12465,16 @@ Perl_ck_refassign(pTHX_ OP *o)
OP * const kid = cUNOPx(kidparent)->op_first;
o->op_private |= OPpLVREF_CV;
if (kid->op_type == OP_GV) {
+ SV *sv = (SV*)cGVOPx_gv(kid);
varop = kidparent;
+ if (SvROK(sv) && SvTYPE(SvRV(sv)) == SVt_PVCV) {+ /* a CVREF here confuses pp_refassign, so make sure+ it gets a GV */+ CV *const cv = (CV*)SvRV(sv);+ SV *name_sv = sv_2mortal(newSVhek(CvNAME_HEK(cv)));+ (void)gv_init_sv((GV*)sv, CvSTASH(cv), name_sv, 0);+ assert(SvTYPE(sv) == SVt_PVGV);+ }
goto detach_and_stack;
}
if (kid->op_type != OP_PADCV) goto bad;
diff --git a/t/op/lvref.t b/t/op/lvref.t
index 3d5e952fb0..b900cbc12a 100644
--- a/t/op/lvref.t+++ b/t/op/lvref.t@@ -1,10 +1,11 @@+#!perl
BEGIN {
chdir 't';
require './test.pl';
set_up_inc("../lib");
}
-plan 164;+plan 167;
eval '\$x = \$y';
like $@, qr/^Experimental aliasing via reference not enabled/,
@@ -291,6 +292,18 @@ package CodeTest {
my sub bs;
\(&cs) = expect_list_cx;
is \&cs, \&ThatSub, '\(&statesub)';
++ package main {+ # this somehow is only a problem in main::+ sub sx { "x" }+ sub sy { "y" }+ is sx(), "x", "check original";+ my $temp = \&sx;+ \&sx = \&sy;+ is sx(), "y", "aliased";+ \&sx = $temp;+ is sx(), "x", "and restored";+ }
}
# Mixed List Assignments
--
2.11.0
Activity
p5pRT commentedon Apr 30, 2019
From @jimav
Created by @jimav
This is a bug report for perl from jim.avera@gmail.com,
generated with the help of perlbug 1.40 running under perl 5.26.2.
-----------------------------------------------------------------
Not sure whether this is intended behavior, but just in case it's not...
The refalias
\&foo = \&bar
is documented as "foo() now means bar()", but it doesn't entirely:
In subsequently-compiled code, the compiler still wants calls
to foo() to match the old foo's prototype, not the prototype of bar.
#!/usr/bin/perl
use strict; use warnings; use v5.22.0;
use feature 'refaliasing';
no warnings 'experimental::refaliasing';
sub foo($$$$) { }
sub bar($) { }
BEGIN{ \&foo = \&bar; }
foo(1); # Gets "Not enough arguments for main::foo"
Perl Info
p5pRT commentedon May 1, 2019
From @tonycoz
On Tue, 30 Apr 2019 16:50:20 -0700, jim.avera@gmail.com wrote:
It's worse than that:
./perl -Ilib -Mfeature=refaliasing -e 'sub foo { print "foo\n" } sub bar { print "bar\n" } \&foo = \&bar; $n = "foo"; &$n'
Aliasing via reference is experimental at -e line 1.
foo
It seems to be caused by the optimization of storing coderefs as GVs - pp_refassign is expecting a GV on the left for your sample code, but gets the CVref instead and so nothing happens:
switch (left ? SvTYPE(left) : 0) {
case 0:
{
SV * const old = PAD_SV(ARGTARG);
PAD_SETSV(ARGTARG, SvREFCNT_inc_NN(SvRV(sv)));
SvREFCNT_dec(old);
if ((PL_op->op_private & (OPpLVAL_INTRO|OPpPAD_STATE))
== OPpLVAL_INTRO)
SAVECLEARSV(PAD_SVl(ARGTARG));
break;
}
case SVt_PVGV:
if (PL_op->op_private & OPpLVAL_INTRO) {
S_localise_gv_slot(aTHX_ (GV *)left, type);
}
gv_setref(left, sv);
SvSETMAGIC(left);
break;
case SVt_PVAV:
assert(key);
if (UNLIKELY(PL_op->op_private & OPpLVAL_INTRO)) {
S_localise_aelem_lval(aTHX_ (AV *)left, key,
SvCANEXISTDELETE(left));
}
av_store((AV *)left, SvIV(key), SvREFCNT_inc_simple_NN(SvRV(sv)));
break;
case SVt_PVHV:
if (UNLIKELY(PL_op->op_private & OPpLVAL_INTRO)) {
assert(key);
S_localise_helem_lval(aTHX_ (HV *)left, key,
SvCANEXISTDELETE(left));
}
(void)hv_store_ent((HV *)left, key, SvREFCNT_inc_simple_NN(SvRV(sv)), 0);
}
Tony
p5pRT commentedon May 1, 2019
The RT System itself - Status changed from 'new' to 'open'
p5pRT commentedon May 15, 2019
From @tonycoz
On Tue, 30 Apr 2019 16:50:20 -0700, jim.avera@gmail.com wrote:
Fix attached.
With the fix your code produces:
$ ./perl -Ilib ../134072.pl
Subroutine main::foo redefined at ../134072.pl line 11.
Prototype mismatch: sub main::foo ($$$$) vs ($) at ../134072.pl line 11.
bar
Tony
Tony
p5pRT commentedon May 15, 2019
From @tonycoz
0001-perl-134072-allow-foo-bar-to-work-in-main.patch
p5pRT commentedon Jun 12, 2019
From @tonycoz
On Tue, 14 May 2019 23:09:38 -0700, tonyc wrote:
Applied as 40258da.
Tony
p5pRT commentedon Jun 12, 2019
@tonycoz - Status changed from 'open' to 'pending release'