t/op/try.t: one test fails during deparsing tests

[jkeenan]

In #22488 (comment), @iabyn noted that t/op/try.t was no longer passing when being run through TEST -deparse.

It turns out that the code which is failing has never passed when testing deparsing. It was added this past April in b8caf0c:

commit b8caf0c1f256ef2e52bae4109552667742ab2b2a
Author:     Paul Evans <[email protected]>
AuthorDate: Fri Apr 5 17:26:32 2024 +0000
Commit:     Paul Evans <[email protected]>
CommitDate: Thu Apr 11 16:07:33 2024 +0100

    Remove experimental warnings from basic try/catch syntax
    
    try/catch/finally remains experimental because of questions around
    double-exception

...

diff --git a/t/op/try.t b/t/op/try.t
index 79ee66a171..83d5565d21 100644
--- a/t/op/try.t
+++ b/t/op/try.t
...
+# experimental warnings
+{
+    my $warnings;
+    BEGIN { $SIG{__WARN__} = sub { $warnings .= shift; }; }
+
+    my ($lfinally) = (__LINE__+5);
+    try {
+    }
+    catch ($e) {
+    }
+    finally {
+    }
+
+    is($warnings, "try/catch/finally is experimental at $0 line $lfinally.\n",
+        'compiletime warnings');
+    BEGIN { undef $SIG{__WARN__}; }
+}
+
+no warnings 'experimental::try';
+

$ git checkout b8caf0c1f25; sh ./Configure -des -Dusedevel && make test_prep; cd t; ./TEST -deparse op/try.t; cd -

------------------------------------------------------------------------------
TESTING DEPARSER
------------------------------------------------------------------------------
t/op/try ... # Failed test 23 - compiletime warnings at op/try.t line 276
#      got "try/catch/finally is experimental at op/try.t line 275.\n"
# expected "try/catch/finally is experimental at op/try.t line 273.\n"
#  diff at 53
#    after "ally is experimental at op/try.t line 27"
#     have "5.\n"
#     want "3.\n"
FAILED at test 23
Failed 1 test out of 0, 0.00% okay.
	op/try.t
...

Since I've just begun to get familiar with TEST -deparse, I don't know whether this a bug or a nit. @leonerd, could you take a look? Thanks.

[iabyn]

On Sat, Aug 10, 2024 at 09:29:39AM -0700, James E Keenan wrote: Since I've just begun to get familiar with `TEST -deparse`, I don't know whether this a bug or a nit. @leonerd, could you take a look? Thanks.

The trick is to run 'TEST -deparse' with the KEEP_DEPARSE_FILES=1 env var set. This will leave the file t/op/try.t.dp, the round-cycle deparsed version of t/op/try.t. That file can then be inspected to see how the new code has been deparsed, and whether it's a bug in the deparser (B/Deparse.pm) or some sort of issue with the test code. Then either fix the test, the deparser, or if it can't be easily fixed, add the test file to Porting/deparse-skips.txt E.g.: $ cd t $ KEEP_DEPARSE_FILES=1 ./TEST -deparse op/try.t .... $ ./perl op/try.t.dp ... not ok 22 - try {} block is invisible to caller() # Failed test 22 - try {} block is invisible to caller() at op/try.t line 258 # got "main::B (op/try.t line 256)" # expected "main::B (op/try.t.dp line 256)" # diff at 17 # after "main::B (op/try.t" # have " line 256)" # want ".dp line 256)" not ok 23 - compiletime warnings # Failed test 23 - compiletime warnings at op/try.t line 276 # got "try/catch/finally is experimental at op/try.t line 275.\n" # expected "try/catch/finally is experimental at op/try.t.dp line 273.\n" # diff at 45 # after "atch/finally is experimental at op/try.t" # have " line 275.\n" # want ".dp line 273.\n" Then from the warnings and the code, it's clear that try.t.dp does #line 258 "op/try.t" to pretend the next line is from try.t rather than try.t.dp, but the test compares the emitted warning against $0 which contains 'try.t.dp'. The second failing test is similar. The fix is trivial. The second test also fails with seeing the wrong line number. This is due to an issue with (__LINE + constant) being constant folded. The fix here is to do the addition separately. Also, the deparser adds extra lines to the output, so add similar lines in the original. Which I've now done as #22496

…

-- I before E. Except when it isn't.

[jkeenan]

The trick is to run 'TEST -deparse' with the KEEP_DEPARSE_FILES=1 env var set. This will leave the file t/op/try.t.dp, the round-cycle deparsed version of t/op/try.t. That file can then be inspected to see how the new code has been deparsed, and whether it's a bug in the deparser (B/Deparse.pm) or some sort of issue with the test code. Then either fix the test, the deparser, or if it can't be easily fixed, add the test file to Porting/deparse-skips.txt

@iabyn, thanks for that explanation. Is that in the docs somewhere, or can we incorporate it therein?

[iabyn]

On Sat, Aug 10, 2024 at 03:34:01PM -0700, James E Keenan wrote: > The trick is to run 'TEST -deparse' with the KEEP_DEPARSE_FILES=1 env > var set. This will leave the file t/op/try.t.dp, the round-cycle > deparsed version of t/op/try.t. That file can then be inspected to see > how the new code has been deparsed, and whether it's a bug in the > deparser (B/Deparse.pm) or some sort of issue with the test code. Then > either fix the test, the deparser, or if it can't be easily fixed, add > the test file to Porting/deparse-skips.txt @iabyn, thanks for that explanation. Is that in the docs somewhere, or can we incorporate it therein?

I believe it's completely undocumented. I'm not sure where the best to document it should be. At the very least, I guess t/TEST aught to haver some basic documentation of what command-line switches and environment variables it supports.

…

-- 31 Dec 1661: "I have newly taken a solemne oath about abstaining from plays". 1 Jan 1662: "And after ... we went by coach to the play". -- The Diary of Samuel Pepys