avoid closing over values in string eval when comparing values

[haarg]

When comparing values using Test::Builder::cmp_ok, a string eval is used to match the caller location, and to inject an arbitrary comparison op. When used with Devel::Cover, the string eval may close over any variables used in it, and maintain them for the lifetime of the program.

Instead of comparing the values directly in the string eval, generate an anonymous sub that will perform the comparison, and call it in a block eval.

This is a reoccurrance of a previous problem that was fixed with 4fe707c. Some combination of perl or module updates has brought back the issue. While there was a test included with the bug, it only fails when run with Devel::Cover, which isn't done during the automated or release testing.

[haarg]

The test script for this could be patched to try running with Devel::Cover, but it's rather ugly. Here's a patch that adds this:

diff --git i/t/Legacy/Regression/is_capture.t w/t/Legacy/Regression/is_capture.t
index 1b8c73e10..b36f8d264 100644
--- i/t/Legacy/Regression/is_capture.t
+++ w/t/Legacy/Regression/is_capture.t
@@ -1,3 +1,31 @@
+BEGIN {
+    if ($ENV{RELEASE_TESTING}) {
+        if (!$INC{'Devel/Cover.pm'}) {
+            delete $ENV{DEVEL_COVER_OPTIONS};
+            require File::Temp;
+            my $temp = File::Temp::tempdir(
+                'Test-Simple-XXXXXX',
+                TMPDIR  => 1,
+                CLEANUP => 1,
+            );
+            require Devel::Cover;
+            Devel::Cover->import(
+                -ignore   => '.',
+                -summary  => 0,
+                -silent   => 1,
+                -coverage => 'none',
+                -blib     => 0,
+                -dir      => $temp,
+                -db       => $temp,
+            );
+        }
+    }
+    else {
+        require Test2::Tools::Tiny;
+        Test2::Tools::Tiny::skip_all('Only running under RELEASE_TESTING');
+        exit;
+    }
+}
 use strict;
 use warnings;
 use Test2::Tools::Tiny;

I can add this to the PR if desired. It would also need to install Devel::Cover during the automated testing, which has its own complications.

[exodist]

I agree the test is ugly, and we can skip it. However I would like a comment added to the patch explaining why we do the sub wrapping, that will help avoid repeating the bug later by any future contributors.

[haarg]

Updated to include a comment.