Skip to content

Conversation

@wolfsage
Copy link
Contributor

There are many cases where we throw exceptions if you attempt to goto somewhere you have no business going. We'd also throw a deprecation warning in these cases, which just seems silly. Deprecated means "this will stop working eventually", so there's no need to throw that when we're just going to die right now.

This probably warrants a changelog entry.

IMO, this could (should) be merged, and then #23782 could be built on this, changing the deprecate_fatal_in to a DIE() to match the other goto exceptions...

@wolfsage wolfsage requested review from jkeenan and tonycoz November 14, 2025 23:16
There are many cases where we throw exceptions if you attempt to goto
somewhere you have no business going. We'd *also* throw a deprecation
warning in these cases, which just seems silly. Deprecated means "this
will stop working eventually", so there's no need to throw that when
we're just going to die right now.
@jkeenan
Copy link
Contributor

jkeenan commented Nov 15, 2025

There are many cases where we throw exceptions if you attempt to goto somewhere you have no business going. We'd also throw a deprecation warning in these cases, which just seems silly. Deprecated means "this will stop working eventually", so there's no need to throw that when we're just going to die right now.

This probably warrants a changelog entry.

IMO, this could (should) be merged, and then #23782 could be built on this, changing the deprecate_fatal_in to a DIE() to match the other goto exceptions...

Thanks for taking a look at this. I hope to get to this Sunday or early next week.

jkeenan added a commit to jkeenan/perl5 that referenced this pull request Nov 15, 2025
This branch explores the question:  What if we accept Matt Horsfall's
suggested revision in pp_ctl.c (GH Perl#23922) into blead, then accept the
goto-label-fatal branch underlying p.r. GH Perl#23782 into blead?

Once a couple of merge conflicts were revised, the result would
configure and build but would fail two test files:

  op/goto.t   (Wstat: 0 Tests: 75 Failed: 23)
    Failed tests:  53-75
  uni/labels.t (Wstat: 0 Tests: 11 Failed: 1)
    Failed test:  10

The failures in t/op/goto.t would be not at all surprising.  Those tests
are tests which currently pass in blead but which would fail if the code
changes in pp_ctl.c in 23782 were directly merged into blead.  I first
deleted those tests from t/op/goto.t, then added them back in so that we
would have a tally of the ways in which 'goto LABEL' would *no longer*
work.  So if we first merged 23922, we would presumably re-work those 23
test to pass in an informative way.

I'll need to look further into t/uni/labels.t.
@jkeenan
Copy link
Contributor

jkeenan commented Nov 16, 2025

Your pull request changes t/lib/croak/pp_ctl (which gets incorporated into the test suite's t/lib/croak.t) by deleting 6 instances of no warnings 'deprecated';.

I would expect that if right now I hacked up blead to simply comment out those 6 instances -- without yet making any changes in pp_ctl.c ...

$ git diff |cat
diff --git a/t/lib/croak/pp_ctl b/t/lib/croak/pp_ctl
index 96f40cd458..7666357a80 100644
--- a/t/lib/croak/pp_ctl
+++ b/t/lib/croak/pp_ctl
@@ -1,26 +1,26 @@
 __END__
 # NAME goto into foreach
-no warnings 'deprecated';
+#no warnings 'deprecated';
 goto f;
 foreach(1){f:}
 EXPECT
 Can't "goto" into the middle of a foreach loop at - line 3.
 ########
 # NAME goto into given
-no warnings 'deprecated';
+#no warnings 'deprecated';
 goto f;
 CORE::given(1){f:}
 EXPECT
 Can't "goto" into a "given" block at - line 3.
 ########
 # NAME goto from given topic expression
-no warnings 'deprecated';
+#no warnings 'deprecated';
 CORE::given(goto f){f:}
 EXPECT
 Can't "goto" into a "given" block at - line 2.
 ########
 # NAME goto into expression
-no warnings 'deprecated';
+#no warnings 'deprecated';
 eval { goto a; 1 + do { a: } }; warn $@;
 eval { goto b; meth { b: }   }; warn $@;
 eval { goto c; map { c: } () }; warn $@;
@@ -32,14 +32,14 @@ Can't "goto" into a binary or list expression at - line 4.
 Can't "goto" into a binary or list expression at - line 5.
 ########
 # NAME dump with computed label
-no warnings 'deprecated';
+#no warnings 'deprecated';
 my $label = "foo";
 CORE::dump $label;
 EXPECT
 Can't find label foo at - line 3.
 ########
 # NAME when outside given
-use 5.01; no warnings 'deprecated';
+use 5.01; #no warnings 'deprecated';
 when(undef){}
 EXPECT
 Can't "when" outside a topicalizer at - line 2.

... I would probably get 6 test failures. Instead, I only get 3 test failures.

$ cd t; ./perl harness -v lib/croak.t; cd -
...
ok 94 - Attempt to bless into a reference after method changes
not ok 95 - goto into foreach
FILE: lib/croak/pp_ctl ; line 2
PROG: 
#no warnings 'deprecated';
goto f;
foreach(1){f:}
EXPECTED:
Can't "goto" into the middle of a foreach loop at - line 3.
EXIT STATUS: != 0
GOT:
Use of "goto" to jump into a construct is deprecated, and will become fatal in Perl 5.42 at - line 2.
Can't "goto" into the middle of a foreach loop at - line 3.
EXIT STATUS: 255
# Failed test 95 - goto into foreach at lib/croak/pp_ctl line 2
not ok 96 - goto into given
FILE: lib/croak/pp_ctl ; line 9
PROG: 
#no warnings 'deprecated';
goto f;
CORE::given(1){f:}
EXPECTED:
Can't "goto" into a "given" block at - line 3.
EXIT STATUS: != 0
GOT:
Use of "goto" to jump into a construct is deprecated, and will become fatal in Perl 5.42 at - line 2.
Can't "goto" into a "given" block at - line 3.
EXIT STATUS: 255
# Failed test 96 - goto into given at lib/croak/pp_ctl line 9
not ok 97 - goto from given topic expression
FILE: lib/croak/pp_ctl ; line 16
PROG: 
#no warnings 'deprecated';
CORE::given(goto f){f:}
EXPECTED:
Can't "goto" into a "given" block at - line 2.
EXIT STATUS: != 0
GOT:
Use of "goto" to jump into a construct is deprecated, and will become fatal in Perl 5.42 at - line 2.
Can't "goto" into a "given" block at - line 2.
EXIT STATUS: 255
# Failed test 97 - goto from given topic expression at lib/croak/pp_ctl line 16
ok 98 - goto into expression
ok 99 - dump with computed label
ok 100 - when outside given
...
Failed 3/312 subtests 
	(less 3 skipped subtests: 306 okay)

Test Summary Report
-------------------
lib/croak.t (Wstat: 0 Tests: 312 Failed: 3)
  Failed tests:  95-97
Files=1, Tests=312,  1 wallclock secs ( 0.07 usr  0.00 sys +  0.61 cusr  0.85 csys =  1.53 CPU)

Can you explain why I'm only getting 3 failures instead of 6?

@jkeenan
Copy link
Contributor

jkeenan commented Nov 16, 2025

[snip]

Can you explain why I'm only getting 3 failures instead of 6?

I'm going to answer my own question by proposing p.r. #23943. The 3 failures I was expecting but not getting are unit tests which, per my research, no longer need no warnings 'deprecated'; because they either never did emit such warnings, or, if they did, no longer do so. If we apply 23943 to blead, then we reduce the scope of what we're considering here in 23922.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants