Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.86k stars 528 forks source link

The value of pattern match vars. after `next' #899

Closed p5pRT closed 7 months ago

p5pRT commented 24 years ago

Migrated from rt.perl.org#1833 (status was 'open')

Searchable as RT1833$

p5pRT commented 24 years ago

From inaba@st.rim.or.jp

Inline Patch ```diff --- cop.h.org Sun Mar 28 02:57:30 1999 +++ cop.h Thu Nov 25 00:54:29 1999 @@ -205,8 +205,9 @@ PL_stack_sp = PL_stack_base + cx->blk_oldsp, \ PL_markstack_ptr = PL_markstack + cx->blk_oldmarksp, \ PL_scopestack_ix = cx->blk_oldscopesp, \ - PL_retstack_ix = cx->blk_oldretsp, \ - PL_curpm = cx->blk_oldpm + PL_retstack_ix = cx->blk_oldretsp + +#define RESTOREPM(cxix) PL_curpm = cxstack[cxix+1].blk_oldpm /* substitution context */ struct subst { --- pp_ctl.c.org Sun Mar 28 02:56:24 1999 +++ pp_ctl.c Thu Nov 25 01:13:20 1999 @@ -1757,9 +1757,10 @@ if (cxix < 0) DIE("Label not found for \"next %s\"", cPVOP->op_pv); } - if (cxix < cxstack_ix) + if (cxix < cxstack_ix) { dounwind(cxix); - + RESTOREPM(cxix); + } TOPBLOCK(cx); oldsave = PL_scopestack[PL_scopestack_ix - 1]; LEAVE_SCOPE(oldsave); @@ -1782,9 +1783,10 @@ if (cxix < 0) DIE("Label not found for \"redo %s\"", cPVOP->op_pv); } - if (cxix < cxstack_ix) + if (cxix < cxstack_ix){ dounwind(cxix); - + RESTOREPM(cxix); + } TOPBLOCK(cx); oldsave = PL_scopestack[PL_scopestack_ix - 1]; LEAVE_SCOPE(oldsave); @@ -1890,8 +1892,10 @@ cxix = dopoptosub(cxstack_ix); if (cxix < 0) DIE("Can't goto subroutine outside a subroutine"); - if (cxix < cxstack_ix) + if (cxix < cxstack_ix){ dounwind(cxix); + RESTOREPM(cxix); + } TOPBLOCK(cx); if (CxTYPE(cx) == CXt_EVAL && cx->blk_eval.old_op_type == OP_ENTEREVAL) DIE("Can't goto subroutine from an eval-string"); @@ -2168,6 +2172,7 @@ if (ix < 0) ix = 0; dounwind(ix); + RESTOREPM(ix); TOPBLOCK(cx); oldsave = PL_scopestack[PL_scopestack_ix]; LEAVE_SCOPE(oldsave); ```
p5pRT commented 24 years ago

From [Unknown Contact. See original ticket]

Inaba Hiroto writes​:

After `next' or `redo'\, pattern match variables are reset to the block's initial value.

$_ = foo; /foo/; for (qw/bar baz/) { print "$&\n" unless /bar/; } # print "bar\n" for (qw/bar baz/) { next if /bar/; print "$&\n"; } # print "foo\n"

I think it is a bug

What made you think so? What other block-local things behave like this?

Ilya

p5pRT commented 24 years ago

From [Unknown Contact. See original ticket]

Ilya Zakharevich wrote​:

Inaba Hiroto writes​:

After `next' or `redo'\, pattern match variables are reset to the block's initial value.

$_ = foo; /foo/; for (qw/bar baz/) { print "$&\n" unless /bar/; } # print "bar\n" for (qw/bar baz/) { next if /bar/; print "$&\n"; } # print "foo\n"

I think it is a bug

What made you think so? What other block-local things behave like this?

I think $& etc. are block local like "{ local $var = $var; ... }".

  $x = "foo";   for (qw/bar baz/) { local $x=$x; print "$x\n" unless /bar/ and $x="bar"; }   for (qw/bar baz/) { local $x=$x; next if /bar/ and $x="bar"; print "$x\n"; }

This code print "foo\nfoo\n". If the former code print "foo\nfoo\n"\, I think it is consistent except the description of $& in "perlvar.pod".

  $& The string matched by the last successful pattern   match (not counting any matches hidden within a   BLOCK or eval() enclosed by the current BLOCK).

So I think the former code should print "bar\nbar\n" and current behavior is a bug. --   Inaba Hiroto \inaba@&#8203;st\.rim\.or\.jp

p5pRT commented 24 years ago

From [Unknown Contact. See original ticket]

Inaba Hiroto \inaba@&#8203;st\.rim\.or\.jp wrote

So I think the former code should print "bar\nbar\n" and current behavior is a bug.

I certainly see a bug here\, but I'm not certain it's the same bug as you see.

Here's a more thoroughly instrumented example​:

%perl -wl $_ = 'foo'; /foo/; print "1​:$&"; for (qw/bar baz/) { print "2​:$&"; print "3​:$&" unless /bar/; print "4​:$&" }; for (qw/bar baz/) { print "5​:$&"; next if /bar/; print "6​:$&" }; print "7​:$&"; __END__ 1​:foo 2​:foo 4​:bar 2​:bar ( I think this should be 2​:foo ) 3​:bar 4​:bar 5​:foo 5​:foo ( You think this should be 5​:bar ?? ) 6​:foo 7​:foo

The issue seems to be that there are two scopes around\, and it isn't self-evident (let alone documented :-) which is the "right" one. But it one of the two scopes should be used consistently; the example shows that they currently aren't.

The two scopes are (i) the whole of the for loop\, so the variables are localised just once\, or (ii) the block which is the body of the for loop\, so the variables are localised on each cycle. These two can be illustrated without using regexen​:

%perl -wl $x = 3; print "1​:$x"; for (1..2) { local $x = $x; print "2​:$x"; $x = 2 if /1/; print "3​:$x" }; for (local $x=1\, 2) { print "4​:$x"; $x = 4; next if /1/; print "5​:$x" }; print "6​:$x"; __END__ 1​:3 2​:3 3​:2 2​:3 ( Variable relocalised on second cycle ) 3​:3 4​:1 5​:4 4​:4 ( Variable not relocalised on second cycle ) 5​:4 6​:4 ( Eh???? Why isn't this back to 3 ? )

Your example shows that we get behaviour (i) on the first loop\, and behaviour (ii) on the second. I think we should get (i) in both cases; if I understand\, you think we should get (ii) in both cases.

Although I prefer (i) to (ii) as being more intuitive\, the important point is that it should be consistent.

Mike Guy

p5pRT commented 24 years ago

From [Unknown Contact. See original ticket]

M.J.T. Guy writes​:

The issue seems to be that there are two scopes around\, and it isn't self-evident (let alone documented :-) which is the "right" one. But it one of the two scopes should be used consistently; the example shows that they currently aren't.

The two scopes are (i) the whole of the for loop\, so the variables are localised just once\, or (ii) the block which is the body of the for loop\, so the variables are localised on each cycle. These two can be illustrated without using regexen​:

If what you want is consistency\, you know where to find Python... :-(

But it would be nice if lexicals introduced inside if(){} and while(){} had the same scope...

Ilya

p5pRT commented 16 years ago

From nobull@cpan.org

   $&      The string matched by the last successful pattern
           match \(not counting any matches hidden within a
           BLOCK or eval\(\) enclosed by the current BLOCK\)\.

This should be addressed as a documentation bug.

Should say

The string matched by the last successful pattern in the current dynamic scope or a parent scope.

The phrase "This variable is ... dynamically scoped to the current BLOCK."

Should be removed because it's not strictly true and doesn't add anything. (It's not the variable that's dynamically scoped it's the last-match context).

The wording of the description of all other match variables @​+ @​- $+ &` &' should all be changed to match.

A more detailed explanation of the scoping rules should be placed in one place and referred to by each of the variables.

p5pRT commented 13 years ago

From @gannett-ggreer

On Sun Aug 19 13​:51​:59 2007\, nobull@​cpan.org wrote​:

   $&      The string matched by the last successful pattern
           match \(not counting any matches hidden within a
           BLOCK or eval\(\) enclosed by the current BLOCK\)\.

This should be addressed as a documentation bug.

Should say

The string matched by the last successful pattern in the current dynamic scope or a parent scope.

The phrase "This variable is ... dynamically scoped to the current BLOCK."

Should be removed because it's not strictly true and doesn't add anything. (It's not the variable that's dynamically scoped it's the last-match context).

The wording of the description of all other match variables @​+ @​- $+ &` &' should all be changed to match.

A more detailed explanation of the scoping rules should be placed in one place and referred to by each of the variables.

This would be a good (easy?) fix for the Perl 5 Documentation Team. Code examples are given in previous entries of the ticket for experimentation and evaluation.

http​://rt.perl.org/rt3/Ticket/Display.html?id=1833

-- George Greer

p5pRT commented 13 years ago

From @cowens

On Sat\, Jul 10\, 2010 at 15​:35\, George Greer via RT \perlbug\-followup@&#8203;perl\.org wrote​:

On Sun Aug 19 13​:51​:59 2007\, nobull@​cpan.org wrote​:

       $&      The string matched by the last successful pattern                match (not counting any matches hidden within a                BLOCK or eval() enclosed by the current BLOCK).

This should be addressed as a documentation bug.

Should say

The string matched by the last successful pattern in the current dynamic scope or a parent scope.

The phrase "This variable is ... dynamically scoped to the current BLOCK."

Should be removed because it's not strictly true and doesn't add anything. (It's not the variable that's dynamically scoped it's the last-match context).

The wording of the description of all other match variables @​+ @​- $+ &` &' should all be changed to match.

A more detailed explanation of the scoping rules should be placed in one place and referred to by each of the variables.

This would be a good (easy?) fix for the Perl 5 Documentation Team. Code examples are given in previous entries of the ticket for experimentation and evaluation.

http​://rt.perl.org/rt3/Ticket/Display.html?id=1833

-- George Greer

I am taking a look at it now.

-- Chas. Owens wonkden.net The most important skill a programmer can have is the ability to read.

p5pRT commented 13 years ago

From @cowens

On Sun\, Jul 11\, 2010 at 12​:35\, Chas. Owens \chas\.owens@&#8203;gmail\.com wrote​:

On Sat\, Jul 10\, 2010 at 15​:35\, George Greer via RT \perlbug\-followup@&#8203;perl\.org wrote​:

On Sun Aug 19 13​:51​:59 2007\, nobull@​cpan.org wrote​:

       $&      The string matched by the last successful pattern                match (not counting any matches hidden within a                BLOCK or eval() enclosed by the current BLOCK).

This should be addressed as a documentation bug.

Should say

The string matched by the last successful pattern in the current dynamic scope or a parent scope.

The phrase "This variable is ... dynamically scoped to the current BLOCK."

Should be removed because it's not strictly true and doesn't add anything. (It's not the variable that's dynamically scoped it's the last-match context).

The wording of the description of all other match variables @​+ @​- $+ &` &' should all be changed to match.

A more detailed explanation of the scoping rules should be placed in one place and referred to by each of the variables.

This would be a good (easy?) fix for the Perl 5 Documentation Team. Code examples are given in previous entries of the ticket for experimentation and evaluation.

http​://rt.perl.org/rt3/Ticket/Display.html?id=1833

-- George Greer

I am taking a look at it now.

-- Chas. Owens wonkden.net The most important skill a programmer can have is the ability to read.

Well\, I am still trying to suss out the rules so I can write the documentation. The latest wrinkle​:

perl -E '"a"=~/a/;say $&;{goto A} A​: say $&'

compare with

perl -E '"a"=~/a/;say $&;goto A if shift; A​: say $&' 1

-- Chas. Owens wonkden.net The most important skill a programmer can have is the ability to read.

p5pRT commented 13 years ago

From nobull@cpan.org

I don't think there is a reasonable explaination of why goto out of an inner scope resets the last match context to the start of the outer scope. We could call it a bug if we want and it may even get fixed in a later version. This is just another reason why goto(LABEL) is evil.

For the purpose of documentation I think your best bet is a footnote that following a goto(LABEL) the last match scoping rules may not be followed properly.

What's more important is to make sure that the documentation makes it clear that it is the last-sucessful-match context buffer that is scoped and not the variables. The variables (and hence any aliases for them) are "magic" and access the last match buffer.

toddr commented 4 years ago

@demerphq does a doc patch for this make sense?

khwilliamson commented 2 years ago

@demerphq ping

demerphq commented 2 years ago

On Mon, 25 Apr 2022, 22:30 Karl Williamson, @.***> wrote:

@demerphq https://github.com/demerphq ping

Will look soon.

Yves

demerphq commented 1 year ago

Patch pushed as #20791

jkeenan commented 7 months ago

Patch pushed as #20791

https://github.com/Perl/perl5/pull/20791 was merged on Feb 12 2023, and there has been no activity in this ticket since then. This ticket was originally created in November 1999! I think we can safely put it out of its misery. Closing now.