New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bizarre copy of ARRAY in last #1849
Comments
From @pjscottfor (1..4) { The behavior is the same in Perl 5.005_04, 5.005_03, and 5.6.0. Of course |
From [Unknown Contact. See original ticket]Peter Scott <Peter@PSDT.com> wrote
It fails back to at least 5.002. But it works in Perl4 if you add And it's not clear in what sense that's "wrong". It certainly shouldn't Mike Guy |
From @floatingatoll[Peter@PSDT.com - Sun Apr 16 10:54:31 2000]:
Continues to be present in @18673. |
From p5p@spam.wizbit.be
A somewhat shorter version of this problem: perl -le 'for (1) { push @a, last; }' (If in the initial problem parans are used around push then the error goes away ( push (@a, $_), last ... ) This problem still exists in bleadperl (and 5.8.8, and 5.9.3)... Regards, Animator |
From @gannett-ggreerOn Thu Apr 27 11:14:44 2006, animator wrote:
I've attached a stab at it. I added a new test and it passes the http://m-l.org/~perl/misc/0001-Fix-for-RT-3112-Bizarre-copy-of-ARRAY-when-using-las.patch http://github.com/greerga/perl/commit/aeb45651ef8a89da73a16a74c7b676305e8ec7cd -- |
From @gannett-ggreerOn Fri Jul 16 19:32:48 2010, greerga wrote:
http://github.com/greerga/perl/commit/aeb45651ef8a89da73a16a74c7b676305e8ec7cd Duh. Now with 100% more attaching action. -- |
From @gannett-ggreer0001-Fix-for-RT-3112-Bizarre-copy-of-ARRAY-when-using-las.patchFrom aeb45651ef8a89da73a16a74c7b676305e8ec7cd Mon Sep 17 00:00:00 2001
From: George Greer <perl@greerga.m-l.org>
Date: Fri, 16 Jul 2010 22:25:27 -0400
Subject: [PATCH] Fix for RT#3112: Bizarre copy of ARRAY when using last from push.
last() shouldn't try to make a mortal copy of push/unshift's array
on the stack because sv_mortalcopy() can't handle a SVt_PVAV.
---
pp_ctl.c | 3 ++-
t/op/loopctl.t | 8 +++++++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/pp_ctl.c b/pp_ctl.c
index a93d6dc..ea25810 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -2292,7 +2292,8 @@ PP(pp_last)
}
else if (gimme == G_ARRAY) {
while (++MARK <= SP) {
- *++newsp = ((pop2 == CXt_SUB) && SvTEMP(*MARK))
+ *++newsp = ((pop2 == CXt_SUB && SvTEMP(*MARK))
+ || SvTYPE(*MARK) == SVt_PVAV)
? *MARK : sv_mortalcopy(*MARK);
TAINT_NOT; /* Each item is independent */
}
diff --git a/t/op/loopctl.t b/t/op/loopctl.t
index d8faec1..54a4f27 100644
--- a/t/op/loopctl.t
+++ b/t/op/loopctl.t
@@ -36,7 +36,7 @@ BEGIN {
}
require "test.pl";
-plan( tests => 47 );
+plan( tests => 48 );
my $ok;
@@ -978,3 +978,9 @@ cmp_ok($ok,'==',1,'dynamically scoped');
cmp_ok("@a37725",'eq',"5 4 3 2",'bug 27725: reverse with empty slots bug');
}
+{
+ # RT#3112
+ fresh_perl_is(<<'EOI', 'ok', { }, 'last in push');
+ for (1, 2) { push @a, last } print "ok"
+EOI
+}
--
1.7.0.4
|
From @rgarciaOn 17 July 2010 04:32, George Greer via RT <perlbug-followup@perl.org> wrote:
Maybe it's better if you use >= instead of == for comparing the SV |
From @nwc10On Fri, Jul 16, 2010 at 07:34:17PM -0700, George Greer via RT wrote:
Which, frustratingly, I can't persuade my mailer to inline for a reply. So Inline Patchdiff --git a/pp_ctl.c b/pp_ctl.c
index a93d6dc..ea25810 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -2292,7 +2292,8 @@ PP(pp_last)
}
else if (gimme == G_ARRAY) {
while (++MARK <= SP) {
- *++newsp = ((pop2 == CXt_SUB) && SvTEMP(*MARK))
+ *++newsp = ((pop2 == CXt_SUB && SvTEMP(*MARK))
+ || SvTYPE(*MARK) == SVt_PVAV)
? *MARK : sv_mortalcopy(*MARK);
TAINT_NOT; /* Each item is independent */
}
Looking through the code I see roughly that idiom in a lot of places. 1: Why is this one special? Only it's *not* doing that. It's copying everything that isn't marked as Nicholas Clark |
From @greergaOn Mon, 19 Jul 2010, Nicholas Clark wrote:
Good questions, and I think I'll need to go diving with 'git blame' to I'll also check out Rafael's suggestion of >=, assuming that SvTYPE -- |
From @tonycozOn Mon Jul 19 21:10:20 2010, perl@greerga.m-l.org wrote:
Based on the discussion, and that the patch no longer applies, I'm This bug is still present in blead v5.19.1-178-gdc12741, so I'm I'm going to take the ticket to have a look at it at that far off time I Tony |
From @cpansproutOn Mon Jul 19 03:07:26 2010, nicholas wrote:
perl -e 'print do{{&{sub{"Just another Perl hacker,\n"}},last}}' I once had some production code relying on this. I looked at my code, It seems to be that for it to do that is a mistake, and it should simply This reminds me of grep and sort, which used to share some code (a -- Father Chrysostomos |
From @nwc10On Mon, Jul 01, 2013 at 12:06:14AM -0700, Father Chrysostomos via RT wrote:
Thanks for the example which demonstrates how to trigger this code path.
Yes, I'm not convinced that it's really a designed in feature. I can't think Given that the implementation would be quite a bit simpler if last simply Nicholas Clark |
From @cpansproutOn Mon Jul 01 02:23:14 2013, nicholas wrote:
And blessing it would involve blessing ‘Bizarre copy of ARRAY in last’, -- Father Chrysostomos |
From @cpansproutOn Mon Jul 19 03:07:26 2010, nicholas wrote:
http://github.com/greerga/perl/commit/aeb45651ef8a89da73a16a74c7b676305e8ec7cd
As already discussed, it doesn’t need to do that for last (just reset But for other things, like eval and subroutines, copying appears to be George’s patch will probably actually fix bug #119797, but it would only -- Father Chrysostomos |
From @cpansproutFixed in 0c0c317. -- Father Chrysostomos |
@cpansprout - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#3112 (status was 'resolved')
Searchable as RT3112$
The text was updated successfully, but these errors were encountered: