-
Notifications
You must be signed in to change notification settings - Fork 601
Description
This issue addresses case NNN mentioned in #23889 (comment) and is very similar to the problem reported in #23958 (recently closed).
The refactoring conducted in commit abf573d altered the control flow such that one exception in pp_ctl.c -- specifically, in function pp_ctl.c -- that was previously exercised by the test suite no longer was so exercised. Investigation suggests that this code is no longer reachable and may be removed from pp_ctl.c without harm.
We'll go back to the commit just before abf573d, mangle some exception messages so that we can identify which similarly-described exceptions are being tested by specific unit tests, compile and run the test suite (though I'll only show the results of the more relevant files in the test suite). If we get a test failure due to a mangled exception message, then we can be confident that that exception was actually being exercised ("covered") by the test suite. If we do not get a test failure due to a mangled exception message, then we'll infer that that exception was not being covered by the test suite, hypothesize that it is unreachable and therefore removable.
v5.37.6-101-g3288b24352
We checkout abf573d and modify pp_ctl.c as follows:
$ git diff -w HEAD^..HEAD
diff --git a/pp_ctl.c b/pp_ctl.c
index 07dd227b4f..7f99707711 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -3191,10 +3191,10 @@ PP(pp_goto)
break;
case CXt_FORMAT:
case CXt_NULL:
- DIE(aTHX_ "Can't \"goto\" out of a pseudo block");
+ DIE(aTHX_ "Can't \"goto\" MMM:out of a pseudo block");
case CXt_DEFER:
/* diag_listed_as: Can't "%s" out of a "defer" block */
- DIE(aTHX_ "Can't \"%s\" out of a \"%s\" block", "goto", S_defer_blockname(cx));
+ DIE(aTHX_ "Can't \"%s\" NNN:out of a \"%s\" block", "goto", S_defer_blockname(cx));
default:
if (ix)
DIE(aTHX_ "panic: goto, type=%u, ix=%ld",
(See: branch https://github.com/jkeenan/perl5/tree/3288b24352-pp_ctl-20251126 and commit
jkeenan@a4cc968.)
We compile and run the test suite -- but here we'll only look at a run of relevant tests.
$ cd t; TEST_JOBS=1 ./perl harness op/defer.t op/goto.t op/sort.t lib/croak.t ; cd -
op/defer.t ... 1/30 # Failed test 25 - Cannot goto out of defer block at op/defer.t line 277
# got 'Can\'t \"goto\" NNN:out of a \"defer\" block at op/defer.t line 271.\n'
# expected /(?^:^Can't "goto" out of a "defer" block )/
op/defer.t ... Failed 1/30 subtests
op/goto.t .... ok
op/sort.t .... 1/203 # Failed test 147 - goto out of a pseudo block 1 at op/sort.t line 812
# got "Can\'t \"goto\" MMM:out of a pseudo b"
# expected eq "Can\'t \"goto\" out of a pseudo block"
op/sort.t .... Failed 1/203 subtests
lib/croak.t .. ok
Test Summary Report
-------------------
op/defer.t (Wstat: 0 Tests: 30 Failed: 1)
Failed test: 25
op/sort.t (Wstat: 0 Tests: 203 Failed: 1)
Failed test: 147
Exception MMM is covered by t/op/sort.t; exception NNN is covered by t/op/defer.t.
v5.37.6-102-gabf573d165
We now checkout abf573d and similarly mangle the same exception messages.
diff --git a/pp_ctl.c b/pp_ctl.c
index 07dd227b4f..7f99707711 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -3191,10 +3191,10 @@ PP(pp_goto)
break;
case CXt_FORMAT:
case CXt_NULL:
- DIE(aTHX_ "Can't \"goto\" out of a pseudo block");
+ DIE(aTHX_ "Can't \"goto\" MMM:out of a pseudo block");
case CXt_DEFER:
/* diag_listed_as: Can't "%s" out of a "defer" block */
- DIE(aTHX_ "Can't \"%s\" out of a \"%s\" block", "goto", S_defer_blockname(cx));
+ DIE(aTHX_ "Can't \"%s\" NNN:out of a \"%s\" block", "goto", S_defer_blockname(cx));
default:
if (ix)
DIE(aTHX_ "panic: goto, type=%u, ix=%ld",
(See: branch https://github.com/jkeenan/perl5/tree/abf573d165-pp_ctl-20251126 and commit
jkeenan@14fde1e.)
We compile and run the test suite -- but again we'll only look at a run of relevant tests.
$ cd t; TEST_JOBS=1 ./perl harness op/defer.t op/goto.t op/sort.t lib/croak.t ; cd -
op/defer.t ... ok
op/goto.t .... ok
op/sort.t .... 1/203 # Failed test 147 - goto out of a pseudo block 1 at op/sort.t line 812
# got "Can\'t \"goto\" MMM:out of a pseudo b"
# expected eq "Can\'t \"goto\" out of a pseudo block"
op/sort.t .... Failed 1/203 subtests
lib/croak.t .. ok
Test Summary Report
-------------------
op/sort.t (Wstat: 0 Tests: 203 Failed: 1)
Failed test: 147
Inference:
Of the two dummied-up exception messages, only MMM is now appearing. t/op/defer.t is now PASSing. pp_ctl.c was not modified in abf573d. t/op/defer.t was modified in that commit, but only to the extent that some tests were removed therefrom and reworked into t/lib/croak/op, which runs as part of t/lib/croak.t -- and that file PASSed.
I'm not at all surprised that the NNN exception was/is no longer exercised by the test suite, as the whole point of abf573d was to alter the control flow. From the perldelta for that commit:
+=head2 Forbidden control flow out of C<defer> or C<finally> now detected at compile-time
+
+It is forbidden to attempt to leave a C<defer> or C<finally> block by means
+of control flow such as C<return> or C<goto>. Previous versions of perl could
+only detect this when actually attempted at runtime.
+
+This version of perl adds compile-time detection for many cases that can be
+statically determined. This may mean that code which compiled successfully on
+a previous version of perl is now reported as a compile-time error with this
+one. This only happens in cases where it would have been an error to actually
+execute the code anyway; the error simply happens at an earlier time.
+
I believe that we have a block of code in pp_ctl.c that can no longer be reached. It can be removed without causing any test failures. Pull request forthcoming.