todo test for 14615 (<<>> deparses same as <>)

[1-1sam]

Submitting this PR as a response to the talk Karl gave about improving Perl. The test I added tests that while(<<>>) deparses correctly as while(defined($_ = <<>>)).

[jkeenan]

Submitting this PR as a response to the talk Karl gave about improving Perl. The test I added tests that while(<<>>) deparses correctly as while(defined($_ = <<>>)).

@1-1sam, thanks for your contribution. However, if I comment out the parts of your patch that pertain to TODO (and remember to add a final semicolon to the like statement once it is liberated from the TODO block), ...

diff --git a/t/run/todo.t b/t/run/todo.t
index 22613888be..6e5804d494 100644
--- a/t/run/todo.t
+++ b/t/run/todo.t
@@ -34,8 +34,8 @@ my $switches = "";
 
 our $TODO;
 
-TODO: {
-    local $::TODO = "GH 14615";
+#TODO: {
+#    local $::TODO = "GH 14615";
     my $results = fresh_perl(<<~'EOF', {});
         use B::Deparse;
         my $deparse = B::Deparse->new();
@@ -47,7 +47,8 @@ TODO: {
     is($?, 0, 'No assertion failure');
 
     like $results, qr/defined\(\$_ = <<>>\)/, 'while(<<>>) deparses as while (defined($_ = <<>>))'
-}
+    ;
+    #}
 
 TODO: {
     local $::TODO = "GH 16008";

... and I then run your code:

$ cd t;./perl harness -v run/todo.t; cd -

ok 1 - GH \#16894
ok 2 - No assertion failure
ok 3 - while(<<>>) deparses as while (defined($_ = <<>>))
not ok 4 - No assertion failure # TODO GH 16008
...

... I find that your test already PASSes -- in which case there's no need for it to be TODO-ed.

(That might mean that we should test this functionality in lib/B/Deparse.t, but admittedly that file has a different testing syntax that takes time to learn.)

[khwilliamson]

Thank you @1-1sam. We need you to run perl Porting/updateAUTHORS.pl so as to add your name and email address to AUTHORS. Once that is done, please re-run the test suite -- especially make test_porting -- to make sure all tests are passing before you re-push your branch.

It could be that this passing without needing the TODO means that this bug has already been fixed, in which case you would get the bounty. :)

[khwilliamson]

I bisected for the commit that fixed this bug #14615

commit acababb
Author: Yves Orton demerphq@gmail.com
Date: Mon Jan 9 22:34:13 2023 +0100

regexec.c - teach BRANCH and BRANCHJ nodes to reset capture buffers

In /((a)(b)|(a))+/ we should not end up with $2 and $4 being set at
the same time. When a branch fails it should reset any capture buffers
that might be touched by its branch.

We change BRANCH and BRANCHJ to store the number of parens before the
branch, and the number of parens after the branch was completed. When
a BRANCH operation fails, we clear the buffers it contains before we
continue on.

It is a bit more complex than it should be because we have BRANCHJ
and BRANCH. (One of these days we should merge them together.)

This is also made somewhat more complex because TRIE nodes are actually
branches, and may need to track capture buffers also, at two levels.
The overall TRIE op, and for jump tries especially where we emulate
the behavior of branches. So we have to do the same clearing logic if
a trie branch fails as well.

I looked at the commits surrounding this commit to see if there were others that seemed more directly related to this deparsing issue. They are all by @demerphq concerning pattern matching. My guess is that these fixed a regex used in the deparsing code to finally do the right thing.

Unless someone in the next couple days disagrees with my analysis, I will claim that this example shows that we may have a bunch of open tickets that have already been fixed by seemingly unrelated commits. And that my hunch that if we invested in writing TODO tests for those tickets, we could easily find ourselves closing a bunch of them, which would increase the eagerness to work on the rest.

[khwilliamson]

@1-1sam, you have earned the bounty. I'll contact you off line to arrange the logistics