-
Notifications
You must be signed in to change notification settings - Fork 577
Fatalize my() in false conditional in perl-5.30 #16702
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
Comments
From @jkeenanI am here creating a ticket for an item mentioned on the mailing list: According to the perl-5.28.0 source code, the following deprecation is ##### And that entails revision of this documentation: ##### I will attach patches which constitute a (crude) first pass at Thank you very much. |
From @jkeenanSummary of my perl5 (revision 5 version 29 subversion 4) configuration: Characteristics of this binary (from libperl): |
From @jkeenanOn Tue, 25 Sep 2018 19:58:32 GMT, jkeenan@pobox.com wrote:
Smoke-testing branch: smoke-me/jkeenan/133543-my-false-conditional Patches attached. Thank you very much. |
From @jkeenan133543-0001-Implement-scheduled-fatalization-of-my-in-false-cond.patchFrom 3dc9e310219a210aad9a0cadbd4c9b9f691fcbf5 Mon Sep 17 00:00:00 2001
From: James E Keenan <[email protected]>
Date: Tue, 25 Sep 2018 15:28:46 -0400
Subject: [PATCH 1/2] Implement scheduled fatalization of my() in false
conditional
op.c: substitute exception for warning. Move documentation in perldiag
from W to F. Remove tests for warnings for such statements.
TODO: Create equivalent unit tests for fatal errors.
TODO: What RT is this connected to?
---
op.c | 4 ++++
pod/perldiag.pod | 24 ++++++++++++++++++++++++
t/lib/warnings/op | 14 --------------
3 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/op.c b/op.c
index d0dcffbecb..ad59f43f8c 100644
--- a/op.c
+++ b/op.c
@@ -8260,9 +8260,13 @@ S_new_logop(pTHX_ I32 type, I32 flags, OP** firstp, OP** otherp)
&& o2->op_private & OPpLVAL_INTRO
&& !(o2->op_private & OPpPAD_STATE))
{
+ /*
Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED),
"Deprecated use of my() in false conditional. "
"This will be a fatal error in Perl 5.30");
+ */
+ Perl_croak(aTHX_ "This use of my() in false conditional is "
+ "no longer allowed");
}
*otherp = NULL;
diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index 2c1fe74a87..fa18450087 100644
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -6109,6 +6109,30 @@ a dirhandle. Check your control flow.
(W unopened) You tried to use the tell() function on a filehandle that
was either never opened or has since been closed.
+=item This use of my() in false conditional is no longer allowed
+
+(F) You used a declaration similar to C<my $x if 0>. There
+has been a long-standing bug in Perl that causes a lexical variable
+not to be cleared at scope exit when its declaration includes a false
+conditional. Some people have exploited this bug to achieve a kind of
+static variable. Since we intend to fix this bug, we don't want people
+relying on this behavior. You can achieve a similar static effect by
+declaring the variable in a separate block outside the function, eg
+
+ sub f { my $x if 0; return $x++ }
+
+becomes
+
+ { my $x; sub f { return $x++ } }
+
+Beginning with perl 5.10.0, you can also use C<state> variables to have
+lexicals that are initialized only once (see L<feature>):
+
+ sub f { state $x; return $x++ }
+
+This use of C<my()> in a false conditional was deprecated beginning in
+Perl 5.10 and became a fatal error in Perl 5.30.
+
=item That use of $[ is unsupported
(F) Assignment to C<$[> is now strictly circumscribed, and interpreted
diff --git a/t/lib/warnings/op b/t/lib/warnings/op
index 54e2e3de20..c193d47837 100644
--- a/t/lib/warnings/op
+++ b/t/lib/warnings/op
@@ -1675,13 +1675,6 @@ Useless localization of match position at - line 49.
Useless localization of vec at - line 50.
########
# op.c
-my $x1 if 0;
-my @x2 if 0;
-my %x3 if 0;
-my ($x4) if 0;
-my ($x5,@x6, %x7) if 0;
-0 && my $z1;
-0 && my (%z2);
# these shouldn't warn
our $x if 0;
our $x unless 0;
@@ -1690,13 +1683,6 @@ if (my $w2) { $a=1 }
if ($a && (my $w3 = 1)) {$a = 2}
EXPECT
-Deprecated use of my() in false conditional. This will be a fatal error in Perl 5.30 at - line 2.
-Deprecated use of my() in false conditional. This will be a fatal error in Perl 5.30 at - line 3.
-Deprecated use of my() in false conditional. This will be a fatal error in Perl 5.30 at - line 4.
-Deprecated use of my() in false conditional. This will be a fatal error in Perl 5.30 at - line 5.
-Deprecated use of my() in false conditional. This will be a fatal error in Perl 5.30 at - line 6.
-Deprecated use of my() in false conditional. This will be a fatal error in Perl 5.30 at - line 7.
-Deprecated use of my() in false conditional. This will be a fatal error in Perl 5.30 at - line 8.
########
# op.c
$[ = 1;
--
2.17.1
|
From @jkeenan133543-0002-Test-for-expected-error-messages-for-my-in-false-con.patchFrom c5941fb0024f03fd78e1822edeba718226a87903 Mon Sep 17 00:00:00 2001
From: James E Keenan <[email protected]>
Date: Tue, 25 Sep 2018 15:55:10 -0400
Subject: [PATCH 2/2] Test for expected error messages for my() in false
conditional.
---
t/op/my.t | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/t/op/my.t b/t/op/my.t
index 35211068d7..59c9087d1d 100644
--- a/t/op/my.t
+++ b/t/op/my.t
@@ -154,5 +154,21 @@ is( $@, '', "eval of my() passes");
eval 'my($a,$b),$x,my($c,$d)';
pass("RT #126844");
+# RT # to come
+my @false_conditionals = (
+ 'my $x1 if 0;',
+ 'my @x2 if 0;',
+ 'my %x3 if 0;',
+ 'my ($x4) if 0;',
+ 'my ($x5,@x6, %x7) if 0;',
+ '0 && my $z1;',
+ '0 && my (%z2);',
+);
+for my $fc (@false_conditionals) {
+ eval $fc;
+ like( $@, qr/^This use of my\(\) in false conditional is no longer allowed/,
+ "my() in false conditional");
+}
+
#Variable number of tests due to the way the while/for loops are tested now
done_testing();
--
2.17.1
|
From [Unknown Contact. See original ticket]On Tue, 25 Sep 2018 19:58:32 GMT, jkeenan@pobox.com wrote:
Smoke-testing branch: smoke-me/jkeenan/133543-my-false-conditional Patches attached. Thank you very much. |
From @jkeenanOn Tue, 25 Sep 2018 20:03:34 GMT, jkeenan wrote:
I realize that the branch I created yesterday was forked from another branch rather than from blead. I have created a new branch (with the same name) based off blead. In this branch I have refined the tests a bit but have made no further changes to op.c.
Better patches attached. Thank you very much. -- |
From @jkeenan133543-0001-Implement-scheduled-fatalization-of-my-in-false-cond.patchFrom ebcefe3eec2fc12fcb7d3c494efb33515cd02391 Mon Sep 17 00:00:00 2001
From: James E Keenan <[email protected]>
Date: Tue, 25 Sep 2018 15:28:46 -0400
Subject: [PATCH 1/2] Implement scheduled fatalization of my() in false
conditional
op.c: substitute exception for warning. Move documentation in perldiag
from W to F. Remove tests for warnings for such statements.
---
op.c | 5 ++---
pod/perldiag.pod | 39 +++++++++++++++++++++++++++++++++++++++
t/lib/warnings/op | 14 --------------
3 files changed, 41 insertions(+), 17 deletions(-)
diff --git a/op.c b/op.c
index 03f066dfc6..146407ba70 100644
--- a/op.c
+++ b/op.c
@@ -8260,9 +8260,8 @@ S_new_logop(pTHX_ I32 type, I32 flags, OP** firstp, OP** otherp)
&& o2->op_private & OPpLVAL_INTRO
&& !(o2->op_private & OPpPAD_STATE))
{
- Perl_ck_warner_d(aTHX_ packWARN(WARN_DEPRECATED),
- "Deprecated use of my() in false conditional. "
- "This will be a fatal error in Perl 5.30");
+ Perl_croak(aTHX_ "This use of my() in false conditional is "
+ "no longer allowed");
}
*otherp = NULL;
diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index 17b96caad1..2c063685c0 100644
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -6110,6 +6110,21 @@ a dirhandle. Check your control flow.
(W unopened) You tried to use the tell() function on a filehandle that
was either never opened or has since been closed.
+=item That use of $[ is unsupported
+
+(F) Assignment to C<$[> is now strictly circumscribed, and interpreted
+as a compiler directive. You may say only one of
+
+ $[ = 0;
+ $[ = 1;
+ ...
+ local $[ = 0;
+ local $[ = 1;
+ ...
+
+This is to prevent the problem of one module changing the array base out
+from under another module inadvertently. See L<perlvar/$[> and L<arybase>.
+
=item The alpha_assertions feature is experimental
(S experimental::alpha_assertions) This feature is experimental
@@ -6198,6 +6213,30 @@ key traversal, but this Perl has been compiled without it. You should
report this warning to the relevant upstream party, or recompile perl
with default options.
+=item This use of my() in false conditional is no longer allowed
+
+(F) You used a declaration similar to C<my $x if 0>. There
+has been a long-standing bug in Perl that causes a lexical variable
+not to be cleared at scope exit when its declaration includes a false
+conditional. Some people have exploited this bug to achieve a kind of
+static variable. Since we intend to fix this bug, we don't want people
+relying on this behavior. You can achieve a similar static effect by
+declaring the variable in a separate block outside the function, eg
+
+ sub f { my $x if 0; return $x++ }
+
+becomes
+
+ { my $x; sub f { return $x++ } }
+
+Beginning with perl 5.10.0, you can also use C<state> variables to have
+lexicals that are initialized only once (see L<feature>):
+
+ sub f { state $x; return $x++ }
+
+This use of C<my()> in a false conditional was deprecated beginning in
+Perl 5.10 and became a fatal error in Perl 5.30.
+
=item times not implemented
(F) Your version of the C library apparently doesn't do times(). I
diff --git a/t/lib/warnings/op b/t/lib/warnings/op
index a2a1e2e3fa..85297836d2 100644
--- a/t/lib/warnings/op
+++ b/t/lib/warnings/op
@@ -1675,13 +1675,6 @@ Useless localization of match position at - line 49.
Useless localization of vec at - line 50.
########
# op.c
-my $x1 if 0;
-my @x2 if 0;
-my %x3 if 0;
-my ($x4) if 0;
-my ($x5,@x6, %x7) if 0;
-0 && my $z1;
-0 && my (%z2);
# these shouldn't warn
our $x if 0;
our $x unless 0;
@@ -1690,13 +1683,6 @@ if (my $w2) { $a=1 }
if ($a && (my $w3 = 1)) {$a = 2}
EXPECT
-Deprecated use of my() in false conditional. This will be a fatal error in Perl 5.30 at - line 2.
-Deprecated use of my() in false conditional. This will be a fatal error in Perl 5.30 at - line 3.
-Deprecated use of my() in false conditional. This will be a fatal error in Perl 5.30 at - line 4.
-Deprecated use of my() in false conditional. This will be a fatal error in Perl 5.30 at - line 5.
-Deprecated use of my() in false conditional. This will be a fatal error in Perl 5.30 at - line 6.
-Deprecated use of my() in false conditional. This will be a fatal error in Perl 5.30 at - line 7.
-Deprecated use of my() in false conditional. This will be a fatal error in Perl 5.30 at - line 8.
########
# op.c
use warnings 'void';
--
2.17.1
|
From @jkeenan133543-0002-Test-for-expected-error-messages-for-my-in-false-con.patchFrom f5ad196364e0930e46cc2bb734949030406896c7 Mon Sep 17 00:00:00 2001
From: James E Keenan <[email protected]>
Date: Tue, 25 Sep 2018 15:55:10 -0400
Subject: [PATCH 2/2] Test for expected error messages for my() in false
conditional.
Make new tests more self-documenting.
For: RT 133543
---
t/op/my.t | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/t/op/my.t b/t/op/my.t
index 35211068d7..5ac382cfe7 100644
--- a/t/op/my.t
+++ b/t/op/my.t
@@ -154,5 +154,21 @@ is( $@, '', "eval of my() passes");
eval 'my($a,$b),$x,my($c,$d)';
pass("RT #126844");
+# RT # 133543
+my @false_conditionals = (
+ 'my $x1 if 0;',
+ 'my @x2 if 0;',
+ 'my %x3 if 0;',
+ 'my ($x4) if 0;',
+ 'my ($x5,@x6, %x7) if 0;',
+ '0 && my $z1;',
+ '0 && my (%z2);',
+);
+for (my $i=0; $i<=$#false_conditionals; $i++) {
+ eval $false_conditionals[$i];
+ like( $@, qr/^This use of my\(\) in false conditional is no longer allowed/,
+ "RT #133543: my() in false conditional: $false_conditionals[$i]");
+}
+
#Variable number of tests due to the way the while/for loops are tested now
done_testing();
--
2.17.1
|
The RT System itself - Status changed from 'new' to 'open' |
From @jkeenanOn Wed, 26 Sep 2018 14:34:08 GMT, jkeenan wrote:
Could the most recent version of each patch (or the smoke-me/jkeenan/133543-my-false-conditional branch) be evaluated with respect to: * Use of the Perl_croak function * Choice of warning message * Performing the fatalization scheduled for perl-5.30 Thank you very much. |
From @jkeenanOn Sat, 29 Sep 2018 23:37:03 GMT, jkeenan wrote:
Warnocked. Pushed to blead in commit 1f692f6. Please continue to review with respect to the 3 criteria mentioned in the previous post. I'm taking this ticket for the purpose of closing it within 7 days unless we get objections. Thank you very much. |
@jkeenan - Status changed from 'open' to 'pending release' |
From [email protected]Hi, This code still does not cause an error even on latest blead perl: my $x = 0 if 0; / Andreas G |
From @jkeenan
If I recall correctly, we were not able to fatalize *all* instances of ##### Thank you very much. |
From @khwilliamsonThank you for filing this report. You have helped make Perl better. With the release today of Perl 5.30.0, this and 160 other issues have been Perl 5.30.0 may be downloaded via: If you find that the problem persists, feel free to reopen this ticket. |
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#133543 (status was 'resolved')
Searchable as RT133543$
The text was updated successfully, but these errors were encountered: