Skip to content
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

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

Open
p5pRT opened this issue Nov 28, 1999 · 10 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Nov 28, 1999

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

Searchable as RT1833$

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 28, 1999

From inaba@st.rim.or.jp

Inline Patch
--- 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

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 29, 1999

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

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 3, 1999

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@​st.rim.or.jp>

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 3, 1999

From [Unknown Contact. See original ticket]

Inaba Hiroto <inaba@​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

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 6, 1999

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

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 19, 2007

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

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 10, 2010

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

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 11, 2010

From @cowens

On Sat, Jul 10, 2010 at 15​:35, George Greer via RT
<perlbug-followup@​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

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 13, 2010

From @cowens

On Sun, Jul 11, 2010 at 12​:35, Chas. Owens <chas.owens@​gmail.com> wrote​:

On Sat, Jul 10, 2010 at 15​:35, George Greer via RT
<perlbug-followup@​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

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 18, 2010

From nobull@cpan.org

On 13 July 2010 02​:06, Chas Owens via RT <perlbug-followup@​perl.org> wrote​:

On Sun, Jul 11, 2010 at 12​:35, Chas. Owens <chas.owens@​gmail.com> wrote​:

On Sat, Jul 10, 2010 at 15​:35, George Greer via RT
<perlbug-followup@​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).

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.

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.