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

Setting $/ to a FOO reference is only half forbidden #14245

Closed
p5pRT opened this issue Nov 16, 2014 · 7 comments
Closed

Setting $/ to a FOO reference is only half forbidden #14245

p5pRT opened this issue Nov 16, 2014 · 7 comments
Labels

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Nov 16, 2014

Migrated from rt.perl.org#123218 (status was 'resolved')

Searchable as RT123218$

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 16, 2014

From @cpansprout

$ ./perl -Ilib -e 'eval {$/={} }; print $@​; print ref $/, "\n"'
Setting $/ to a HASH reference is forbidden at -e line 1.
HASH

It croaks, but the assignment happens anyway, because the croak is too late. Either this should be a warning, or we should arrange for the previous value of $/ to be preserved.

--

Father Chrysostomos

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 16, 2014

From @leonerd

On Sat, 15 Nov 2014 16​:37​:31 -0800
Father Chrysostomos (via RT) <perlbug-followup@​perl.org> wrote​:

It croaks, but the assignment happens anyway, because the croak is
too late. Either this should be a warning, or we should arrange for
the previous value of $/ to be preserved.

Isn't this just what 'set' magic does though? It can't prevent the
assignment happening, because it's invoked afterwards. It just observes
the effect once it has.

Having the 'set' magic store the previous value and restore it on bad
assignments before croaking could be easy enough though; seems nicer
than downgrading it to a warning that still leads to broken behaviour.

--
Paul "LeoNerd" Evans

leonerd@​leonerd.org.uk
http​://www.leonerd.org.uk/ | https://metacpan.org/author/PEVANS

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 16, 2014

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 28, 2015

From @tonycoz

On Sun Nov 16 04​:47​:05 2014, leonerd@​leonerd.org.uk wrote​:

On Sat, 15 Nov 2014 16​:37​:31 -0800
Father Chrysostomos (via RT) <perlbug-followup@​perl.org> wrote​:

It croaks, but the assignment happens anyway, because the croak is
too late. Either this should be a warning, or we should arrange for
the previous value of $/ to be preserved.

Isn't this just what 'set' magic does though? It can't prevent the
assignment happening, because it's invoked afterwards. It just observes
the effect once it has.

Having the 'set' magic store the previous value and restore it on bad
assignments before croaking could be easy enough though; seems nicer
than downgrading it to a warning that still leads to broken behaviour.

The valued used by the interpreter is stored in PL_rs, we can just return that in Perl_get_magic().

Tony

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 28, 2015

From @tonycoz

0001-perl-123218-preserve-if-set-to-a-bad-value.patch
From a5fe86682f9440cd4c901ab9ee5aa1f49085ea39 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 28 Jan 2015 16:45:21 +1100
Subject: [PATCH] [perl #123218] "preserve" $/ if set to a bad value

and base/rs.t tests $/ not $!
---
 mg.c        |    1 +
 t/base/rs.t |   11 ++++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/mg.c b/mg.c
index 58427a4..237b404 100644
--- a/mg.c
+++ b/mg.c
@@ -1105,6 +1105,7 @@ Perl_magic_get(pTHX_ SV *sv, MAGIC *mg)
     case ':':
 	break;
     case '/':
+        sv_setsv(sv, PL_rs);
 	break;
     case '[':
 	sv_setiv(sv, 0);
diff --git a/t/base/rs.t b/t/base/rs.t
index c81b2dc..6f3c4b5 100644
--- a/t/base/rs.t
+++ b/t/base/rs.t
@@ -1,7 +1,7 @@
 #!./perl
-# Test $!
+# Test $/
 
-print "1..38\n";
+print "1..39\n";
 
 $test_count = 1;
 $teststring = "1\n12\n123\n1234\n1234\n12345\n\n123456\n1234567\n";
@@ -34,8 +34,13 @@ test_record(*TESTFILE);
 close TESTFILE;
 $test_count_end = $test_count;  # Needed to know how many tests to skip
 
+$/ = "\n";
+my $note = "\$/ preserved when set to bad value";
+# none of the setting of $/ to bad values should modify its value
 test_bad_setting();
-
+print +($/ ne "\n" ? "not " : "") .
+  "ok $test_count # \$/ preserved when set to bad value\n";
+++$test_count;
 
 # Now for the tricky bit--full record reading
 if ($^O eq 'VMS') {
-- 
1.7.10.4

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 4, 2015

From @tonycoz

On Tue Jan 27 21​:47​:01 2015, tonyc wrote​:

On Sun Nov 16 04​:47​:05 2014, leonerd@​leonerd.org.uk wrote​:

On Sat, 15 Nov 2014 16​:37​:31 -0800
Father Chrysostomos (via RT) <perlbug-followup@​perl.org> wrote​:

It croaks, but the assignment happens anyway, because the croak is
too late. Either this should be a warning, or we should arrange
for
the previous value of $/ to be preserved.

Isn't this just what 'set' magic does though? It can't prevent the
assignment happening, because it's invoked afterwards. It just
observes
the effect once it has.

Having the 'set' magic store the previous value and restore it on bad
assignments before croaking could be easy enough though; seems nicer
than downgrading it to a warning that still leads to broken
behaviour.

The valued used by the interpreter is stored in PL_rs, we can just
return that in Perl_get_magic().

No objections (or responses of any sort.)

Applied as 5fe499a.

Tony

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 4, 2015

@tonycoz - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this Feb 4, 2015
@p5pRT p5pRT added the Severity Low label Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.