Bug #122252 for Future-AsyncAwait: Support perl version 5.14

Tue Jun 27 11:54:01 2017 leonerd-cpan [...] leonerd.org.uk - Ticket created

Subject:

Support perl versions prior to 5.24

Current implementation of the code requires 5.24+ for a variety of reasons, most notably the context stack rework that turned up around then. Ideally it would be nice to be able to support all the way to 5.14, when custom keywords were added. This may require some amount of compatibility shim around the context stack however. ((split out from https://rt.cpan.org/Ticket/Display.html?id=121569)) -- Paul Evans

Tue Jun 27 12:09:17 2017 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Or, so as not to confuse RT's URL parser: https://rt.cpan.org/Ticket/Display.html?id=121569 -- Paul Evans

Sun Jan 07 13:05:25 2018 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Some information may be found in https://www.nntp.perl.org/group/perl.perl5.porters/2016/01/msg233631.html -- Paul Evans

Sun Jan 07 16:58:42 2018 leonerd-cpan [...] leonerd.org.uk - Correspondence added

https://perl5.git.perl.org/perl.git/commitdiff/9d876b687d12d77fcb1012f1b865783476f00c7a?hp=d5eedefea11231da8139c9478d2c9069346b6560 -- Paul Evans

Mon Jan 15 12:26:54 2018 leonerd-cpan [...] leonerd.org.uk - Correspondence added

I now have an almost-working initial attempt on 5.22. Most things work, but one does not. The not-working test is the one about foreach loops, as it seems to have trouble saving and restoring the iterator variable which is part of the pre-5.24 context stack entries. There doesn't seem to be equivalent in the 5.24 reworked version of that for me to copy from, so it seems new things need adding. As yet I've not quite worked out how it all fits together. -- Paul Evans

Mon Jan 15 13:23:23 2018 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Mon Jan 15 12:26:54 2018, PEVANS wrote: Show quoted text

> I now have an almost-working initial attempt on 5.22. Most things > work, but one does not.

perl 5.22 now works as of -r121 I'm now looking at 5.20 -- Paul Evans

Mon Jan 15 14:06:17 2018 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Future-AsyncAwait 0.12 now supports back as far as perl 5.20. -- Paul Evans

Mon Jan 15 14:06:27 2018 leonerd-cpan [...] leonerd.org.uk - Subject changed from 'Support perl versions prior to 5.24' to 'Support perl versions prior to 5.20'

Tue Jan 16 11:13:32 2018 leonerd-cpan [...] leonerd.org.uk - Subject changed from 'Support perl versions prior to 5.20' to 'Support perl versions prior to 5.18'

Tue Jan 16 11:16:57 2018 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Latest development version now supports 5.18. Earlier than that, looking at 5.16. Making it compile isn't too hard - SAVEt_STRLEN needs to be #ifdef'ed out and a suitable SAVEt_INT_SMALL in replacement. The PADNAMEs structures need some compatibility macros. However, actually making it work and pass tests seems more difficult. There appears to be something odd about closures, that mean inner closures created after the first 'await' op don't work as expected, with captured lexical failing to stay shared. It may need some more research on how to make that work, but I think for now I'll stop at 5.18. -- Paul Evans

Tue Jan 16 12:04:58 2018 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Tue Jan 16 11:16:57 2018, PEVANS wrote: Show quoted text

> Latest development version now supports 5.18. > > Earlier than that, looking at 5.16. > > Making it compile isn't too hard - SAVEt_STRLEN needs to be #ifdef'ed > out and a suitable SAVEt_INT_SMALL in replacement. The PADNAMEs > structures need some compatibility macros. > > However, actually making it work and pass tests seems more difficult. > There appears to be something odd about closures, that mean inner > closures created after the first 'await' op don't work as expected, > with captured lexical failing to stay shared. It may need some more > research on how to make that work, but I think for now I'll stop at > 5.18.

I tried running ‘git log v5.16.0..v5.18.0 pad.c’ to jog my memory of what changed regarding closures, an area I have been involved in in recent years. It looks as though you may be running into old bugs that have since been fixed. See this commit, for instance: commit cae5dbbe30ba4a96ff5e570be0d90779f06fee71 Author: Father Chrysostomos <sprout@cpan.org> Date: Sat Aug 4 14:42:47 2012 -0700 Close over stale vars in active subs (I won’t repeat the whole commit message here. It’s long.) But most of the other bugs fixed back then had to do with formats closing over the wrong pad and crashing. And in the git log output there is a whole bunch of lexical sub stuff that you probably want to skip. I know I’m not being that helpful. But I thought I would at least take a look and report my meagre findings.

Tue Jan 16 12:04:59 2018 The RT System itself - Status changed from 'new' to 'open'

Tue Jan 16 12:23:45 2018 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Tue Jan 16 12:04:58 2018, SPROUT wrote: Show quoted text

> I tried running ‘git log v5.16.0..v5.18.0 pad.c’ to jog my memory of > what changed regarding closures, an area I have been involved in in > recent years. It looks as though you may be running into old bugs > that have since been fixed. See this commit, for instance: > > commit cae5dbbe30ba4a96ff5e570be0d90779f06fee71 > Author: Father Chrysostomos <sprout@cpan.org> > Date: Sat Aug 4 14:42:47 2012 -0700 > > Close over stale vars in active subs > > (I won’t repeat the whole commit message here. It’s long.)

For convenience the full commit is at https://perl5.git.perl.org/perl.git/commit/cae5dbbe30ba4a96ff5e570be0d90779f06fee71 The full description does indeed sound like it might be related though. Looking at the commit diff though, it suggests that if my test was affected by this bug I'd be seeing lots of Variable $foo is not available warnings, which I'm not. Still, an easy test might be to temporarily SvPADSTALE_off() all the PAD slots during cloning, just to artificially trick clone_sv() into keeping them. -- Paul Evans

Mon Jan 22 10:39:43 2018 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Actually, most of the failures in 5.16 turn out to be fixed by bugfixes applied in F-AA 0.14, relating to the way that outer lexical captures are handled by `cv_clone()`. Now, running against 5.16 the only failure is t/10pad.t .............. 1/? # Failed test '$captured in subD' # at t/10pad.t line 118. # got: undef # expected: 'ABC' # Failed test '$fret now ready after done for inner subs' # at t/10pad.t line 137. # got: 'ABCE' # expected: 'ABCDE' # Looks like you failed 2 tests of 10. t/10pad.t .............. Dubious, test returned 2 (wstat 512, 0x200) Ignoring that momentarily and looking ahead to 5.14, it seems it won't compile because 5.14 doesn't provide a `cv_clone()`. That feels a potentially harder one to work around at 5.14. That said, given all the hackery F-AA 0.14 has had to add to work around the fact that `cv_clone()` doesn't do the right thing for the odd case that we're presenting to it here, it might be worth looking into whether a different solution can be found that implements a CV copying function that will work better for us, than calling cv_clone - the latter being designed to copy protosubs into new anonsubs, which isn't really what we're using it for. -- Paul Evans

Tue Jan 23 21:46:58 2018 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Mon Jan 22 10:39:43 2018, PEVANS wrote: Show quoted text

> Ignoring that momentarily and looking ahead to 5.14, it seems it won't > compile because 5.14 doesn't provide a `cv_clone()`. That feels a > potentially harder one to work around at 5.14. > > That said, given all the hackery F-AA 0.14 has had to add to work > around the fact that `cv_clone()` doesn't do the right thing for the > odd case that we're presenting to it here, it might be worth looking > into whether a different solution can be found that implements a CV > copying function that will work better for us, than calling cv_clone - > the latter being designed to copy protosubs into new anonsubs, which > isn't really what we're using it for.

With the new replacement of cv_clone() now in latest development commit, 5.16 looks exactly the same - still fails this test. I might have to deploy Devel::MAT at the moment $subD is created to inspect its pad and see what happened there. Looking at 5.14 the situation is *slightly* improved - fails to compile still due to lack of `pad_new()` but as the pad structure on 5.14 was all bare AVs, this should be an easy one to provide by hax. -- Paul Evans

Tue Jan 23 22:13:15 2018 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Tue Jan 23 21:46:58 2018, PEVANS wrote: Show quoted text

> With the new replacement of cv_clone() now in latest development > commit, 5.16 looks exactly the same - still fails this test. I might > have to deploy Devel::MAT at the moment $subD is created to inspect > its pad and see what happened there.

As I suspected; `$captured` has captured the wrong thing. On t/10pad.t line 122 after $subD has been created, it already looks like: [ 4/$captured]=SCALAR(PV) at 0x1e771c8 = "ABC" ... [ 9/$subD ]=REF() at 0x1e77090 => CODE(PP,C) at 0x1e3e0b8 Show quoted text

pmat> show 0x1e3e0b8

CODE(PP,C) at 0x1e3e0b8 with refcount 1 ... pad[0]=PAD(4) at 0x1e3e3e8 Show quoted text

pmat> show 0x1e3e3e8

PAD(4) at 0x1e3e3e8 with refcount 1 ... [ 1/$captured]=NULL So it appears that the 5.16 implementation of cv_clone() here that's called as part of the OP_REFGEN op in the optree to implement my $subD = sub { ... }; has found the wrong `$captured` lexical. I wonder if we can patch this up somehow. -- Paul Evans

Tue Jan 23 23:04:06 2018 leonerd-cpan [...] leonerd.org.uk - Subject changed from 'Support perl versions prior to 5.18' to 'Support perl version 5.14'

Tue Jan 23 23:05:19 2018 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Tue Jan 23 22:13:15 2018, PEVANS wrote: Show quoted text

> I wonder if we can patch this up somehow.

Turns out in summary: yes we can. Latest devel (will become 0.15) supports 5.16. All that remains now is 5.14, and that has /lots/ of test failures right now :( -- Paul Evans

Wed Jan 09 10:17:39 2019 leonerd-cpan [...] leonerd.org.uk - Status changed from 'open' to 'stalled'