Skip to content

Conversation

tonycoz
Copy link
Contributor

@tonycoz tonycoz commented Sep 18, 2023

This largely reverts a0d2bbd and adds a text for the documented behaviour of eval in package DB.

Fixes #19370.

a0d2bbd was originally added to fix #11286, but it also broke long documented behaviour as reported in #19370 that's used by the debugger.

The original eval-in-DB was implemented by @iabyn

But they do need CvOUTSIDE() for eval-in-package-DB's scope
magic to work correctly.

This also incidentally fixes a TODO Deparse, since the CvOUTSIDE
is now available

Fixes Perl#19370 aka rt89544

This breaks Perl#11286, but I don't think that's fixable without breaking
eval-in-DB's special scope behaviour.

This reverts (most of) commit a0d2bbd.
@tonycoz tonycoz requested review from Leont and iabyn September 18, 2023 04:20
@iabyn
Copy link
Contributor

iabyn commented Sep 19, 2023

The "ignore current lexical scope for evals in package DB" behaviour was actually present before my time, albeit in a slightly borked form. I accidentally broke the behaviour with my "jumbo closure fix", perl-5.8.0-1527-gb5c19bd7c1, then fixed it properly with perl-5.8.0-519-gd819b83ae9 (with that fix, it then completely ignored the current lexical scope, while originally it only partly did)

My feeling is that NULLing CvOUTSIDE() was the wrong fix for leaking closures, and reverting it is the correct thing to do, especially as the restored DB/eval behaviour is documented in perlfunc.

I haven't looked closely, but my gut feeling is that the CvOUTSIDE() leak thing might be fixable by a suitable application of the CvWEAKOUTSIDE flag, or in the alternative, applying a similar measure.

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

Labels

None yet

2 participants