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
wrong match order inside replacement #6149
Comments
From perl-5.8.0@ton.iguana.beCreated by perl-5.8.0@ton.iguana.beperl -le '$_="CCCGGG"; s!.!<@a{print("$&"),/[$&]/g}>!g; print' Unmatched [ in regex; marked by <-- HERE in m/[ <-- HERE ]/ at -e line 1. $& should be the result of /./ each time. It however seems to on perl perl, v5.7.0 built for sun4-solaris: Here it is even stranger, the last three C's should have been G's Giving an extra scope solves it: Perl Info
|
From @schwernConfirmed to still exist in 5.8.6 and bleadperl. |
The RT System itself - Status changed from 'new' to 'open' |
From @cpansproutA slightly simpler case: $ perl -le '$_="CCCGGG"; s!.!@a{print("[$&]"),/./}!g' What’s happening is that the s/// does not reset PL_curpm for each iteration, because it doesn’t usually have to. The RHS’s scoping takes care of it most of the time. This happens with the /e modifier and with @{...}. In this example, though, we have a subscript, not a block. This subscript is in the same scope as the s/// itself. The assumption that the substitution operator will never have to reset PL_curpm itself appears to be incorrect. The attached patch fixes it. |
From @cpansproutInline Patchdiff -Nup blead/pp_ctl.c blead-19078-subst-with-rhs-match-in-same-scope/pp_ctl.c
--- blead/pp_ctl.c 2010-07-21 05:59:10.000000000 -0700
+++ blead-19078-subst-with-rhs-match-in-same-scope/pp_ctl.c 2010-07-31 18:38:59.000000000 -0700
@@ -377,6 +377,7 @@ PP(pp_substcont)
(void)ReREFCNT_inc(rx);
cx->sb_rxtainted |= RX_MATCH_TAINTED(rx);
rxres_save(&cx->sb_rxres, rx);
+ PL_curpm = pm;
RETURNOP(pm->op_pmstashstartu.op_pmreplstart);
}
diff -Nurp blead/t/re/subst.t blead-19078-subst-with-rhs-match-in-same-scope/t/re/subst.t
--- blead/t/re/subst.t 2010-07-17 18:51:14.000000000 -0700
+++ blead-19078-subst-with-rhs-match-in-same-scope/t/re/subst.t 2010-07-31 22:28:49.000000000 -0700
@@ -7,7 +7,7 @@ BEGIN {
}
require './test.pl';
-plan( tests => 170 );
+plan( tests => 172 );
# Stolen from re/ReTest.pl. Can't just use the file since it doesn't support
# like() and it conflicts with test.pl
@@ -724,3 +724,25 @@ fresh_perl_is( '$_="abcef"; s/bc|(.)\G(.
$string =~ s/./\777/;
is($string, chr 0x1FF, "Verify that handles s/foo/\\777/");
}
+
+# Scoping of s//the RHS/ when there is no /e
+# Tests based on [perl #19078]
+{
+ local *_;
+ my $output = ''; my %a;
+ no warnings 'uninitialized';
+
+ $_="CCCGGG";
+ s!.!<@a{$output .= ("$&"),/[$&]/g}>!g;
+ $output .= $_;
+ is(
+ $output, "CCCGGG< >< >< >< >< >< >",
+ 's/// sets PL_curpm for each iteration even when the RHS has set it'
+ );
+
+ s/C/$a{m\G\}/;
+ is(
+ "$&", G =>
+ 'Match vars reflect the last match after s/pat/$a{m|pat|}/ without /e'
+ );
+} |
From @demerphqOn 1 August 2010 21:19, Father Chrysostomos <sprout@cpan.org> wrote:
Hi, thanks for the patch. Im a little concerned that this might have Yves -- |
From @khwilliamsondemerphq wrote:
How will we find out more?
|
From @demerphqOn 4 August 2010 15:47, karl williamson <public@khwilliamson.com> wrote:
The problem is that there is a lot of code around that tries to deal So, for instance I am wondering what happens with this patch and PL_curpm and everything related to it really deserves some deep Yves -- |
From @cpansproutOn Aug 4, 2010, at 7:27 AM, demerphq wrote:
Could you explain to me what the two PL_curpm assignments in regexec.c are for? One is in S_regtry; the other in restore_pos. When are those two functions called? (All other assignments to PL_curpm look fine to me and do not conflict with the patch.) |
From @demerphqOn 8 August 2010 21:24, Father Chrysostomos <sprout@cpan.org> wrote:
regtry() is a setup wrapper around regmatch(). Essentailly matching restore_pos() is called prior to exiting p_regexec(). To explain a bit.... Originally regexes were basically stored entirely in the opcode, there At some point in the evolution of the regex engine we ended up with Anyway, the reason why I asked to hold off on this, and I apologize cheers, -- |
From @cpansproutOn Aug 10, 2010, at 12:57 AM, demerphq wrote:
I’ve had a look, and I see that S_regtry assigns PL_curpm to PL_reg_oldcurpm before clobbering the former. restore_pos restores to PL_curpm the value saved in PL_reg_oldcurpm. That all happens within the call to pregexec, so the value of PL_curpm should always be the same after pregexec as it was before.
That would explain the scoping bug. $u = ",echle etn sJ";
I know about the light copies, since I hacked around them to fix bug #70764. :-)
Even if this is cleaned up (I’d like to do it, but not any time soon, as I need more time to digest the source code), I think my patch to pp_substcont would still apply, as pp_substcont is effectively ‘outside’ the regexp engine. In fact, if PL_curpm is replaced with some other global, it will still need to be set in pp_substcont, so in a sense my patch marks the right spot for future reference. I’ve not been able to come up with a test case that my patch breaks. |
From @demerphqOn 15 August 2010 22:24, Father Chrysostomos <sprout@cpan.org> wrote:
Well if you have reviewed the source aware of my general concerns and yves -- |
From @cpansproutOn Sun Aug 01 12:19:41 2010, sprout wrote:
Applied as af9838c. |
@cpansprout - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#19078 (status was 'resolved')
Searchable as RT19078$
The text was updated successfully, but these errors were encountered: