develooper Front page | perl.perl5.porters | Postings from January 2018

Re: CvNAME_HEK_set() - public API?

Thread Previous | Thread Next
From:
Paul "LeoNerd" Evans
Date:
January 25, 2018 18:07
Subject:
Re: CvNAME_HEK_set() - public API?
Message ID:
20180125180727.669c3f57@shy.leonerd.org.uk
On 25 Jan 2018 17:33:43 -0000
Father Chrysostomos <sprout@cpan.org> wrote:

> Paul Evans wrote:
> > the chunk
> > of code I copypasted from pad.c in this case is
> > 
> >   if(CvNAMED(orig))
> >     CvNAME_HEK_set(new, share_hek_hek(CvNAME_HEK(orig)));
> >   else
> >     CvGV_set(new, CvGV(orig));
> > 
> > I notice that `CvNAME_HEK_set()` is available in cv.h, and feels the
> > sort of macro that would be public API.
> > 
> > I also notice that it doesn't work. Its definition calls
> > `unshare_hek()`, which embed.h only defines inside #ifdef PERL_CORE,
> > meaning it's not visible to XS users. If I simply steal the one-line
> > 
> >   #define unshare_hek(a)          Perl_unshare_hek(aTHX_ a)
> > 
> > then it appears to work fine, but it makes me feel cautious.  
> 
> HEKs are not actually part of the API.  Defining unshare_hek like
> that will work, but will be fragile and may break later on.

Turned out it didn't. It appeared to work on my Linux box because ELF
makes the symbol `Perl_unshare_hek` visible anyway, even though nobody
explicitly asked for that. On non-ELF platforms (e.g. MSWin32) it
breaks entirely.

> Ideally you shouldn't have to do this, because cv_clone should do
> what you need.

Well I'm using it for quite a specific purpose here that besides me, I
don't imagine anyone else would need to do.

> My suggestion would be to define unshare_hek for now (which is a
> hack),

See above re: MSWin32

Instead, what I did was observe that actually since I'm calling this on
a brand new CV, I can guess it won't have a HEK set on it already, so
the unshare part is unnecessary. I just set the field directly by
copypasteing that part of the macro.

At absolute worst case, I could just observe that these HEKs appear
only to be used in `my sub`s so I could just say "sorry, can't support
lexical subs" - it wouldn't be the worst unsupported feature in F-AA ;)

> and then see later on if cv_clone can be modified to fit your needs
> better.  Maybe we actually need a more general cv_clone_flags
> function with different options.  Determining that will depend on
> what you come up with in your version cv_clone, I suppose.

There is an initial version of my new attempt now on CPAN:

  https://metacpan.org/source/PEVANS/Future-AsyncAwait-0.15/lib/Future/AsyncAwait.xs#L471

It's a sortof-copy of cv_clone() .oO( A cv_clone-clone perhaps? ;) )
that does kind the same thing, bar a few differences. It creates a
new CV, copying the basic fields, then creates a new padlist with a copy
of the padnames and a new pad for depth 1. The key difference from
cv_clone is that it doesn't try to copy pad variables out of CvOUTSIDE,
but literally copies them from the actual live pad from the CV itself.

It's called very specifically `cv_dup_for_suspend()` to really draw
attention to two non-obvious features about it:

  * It does NOT copy CvSTART() of the original CV. Instead this is left
    as NULL.

  * It does NOT copy normal lexicals of the currently-live pad - only
    the "other" slots - the outer captures, globs, protosubs, etc..
    that also live in the pad. Instead these are left at NULL.

These two omissions are simply to save unnecessary work, because in
both cases suspend or resume logic later on are going to overwrite
those fields before the values that cv_dup_for_suspend() would have put
in had any effect, so it might as well not bother.

In spirit it's quite small and simple, except that before perl 5.18
there's an additional upset in that I then have to clone-and-adjust all
the protosubs of inner nested closures because at runtime those
wouldn't find the right "outside" scope.


If core wanted to provide a variant of this function, those omitted
steps wouldn't matter too much (other than a tiny amount of CPU and
memory churn), so the only real difference is my altered behaviour
about whether to peek up into CvOUTSIDE to find captured lexicals or
literally clone them from the live pad. Slightly tongue-in-cheekly I
could suggest a flag of

  CV *new = cv_clone_flags(CVCLONEf_DONT_GO_OUTSIDE);

-- 
Paul "LeoNerd" Evans

leonerd@leonerd.org.uk      |  https://metacpan.org/author/PEVANS
http://www.leonerd.org.uk/  |  https://www.tindie.com/stores/leonerd/

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About