develooper Front page | perl.perl5.porters | Postings from September 2010

Re: [perl #77548] [PATCH] Re: Runtime hinthash from XS

Thread Previous | Thread Next
Ben Morrow
September 2, 2010 09:37
Re: [perl #77548] [PATCH] Re: Runtime hinthash from XS
Message ID:
Quoth (Florian Ragwitz):
> Ben Morrow (via RT) <> writes:
> >
> > +Apd	|HV*	|hinthash_2hv	|NN const COP *cop
> > +: used for the macros below, not API itself
> > +EXopM	|SV*	|hinthash_fetchx|NN const COP *cop|NULLOK const SV *keysv \
> > +				|NULLOK const char *key|STRLEN klen \
> > +				|int flags|U32 hash
> > +Amd	|SV*	|hinthash_fetchsv|NN const COP *cop|NN SV *keysv|U32 hash
> > +Amd	|SV*	|hinthash_fetchpvn|NN const COP *cop|NN const char *key \
> > +				|STRLEN klen |int flags|U32 hash
> > +Amd	|SV*	|hinthash_fetchpvs|NN const COP *cop|NN const char *const key
> I'm not fully happy with the naming.
> All other hints-related code in the core I've seen so far refers to the
> hints hash as "hints", not "hinthash". Also a function called
> hinthash_2hv not actually taking a hinthash but returning the hinthash
> as an HV seems slightly off.
> cop_fetch_hint_hv/cop_fetch_hint_{x,sv,pvn,pvs}, maybe?

I agree, though I think I prefer cop_hints_fetch(sv|pv[ns]) and
cop_hints_2hv. I think the '2hv' is important, because it isn't
returning the actual hinthash but rather a copy of it.

> store_cop_label is also under discussion to be API-fied. Using
> cop_store_label there would give us some consistency, both with the
> functions operating on COPs, as well as with many other similarly named
> api functions like av_{fetch,store,len}, hv_{exists,store,fetch}, etc.

Yup, though again I think cop_label_{fetch,store} would be more
consistent with av_fetch.

> > +const PERL_CONTEXT *
> > +Perl_caller_cx(pTHX_ I32 count, const PERL_CONTEXT **dbcxp)
> Does this really belong into pp_ctl.c?

Well, I think so. There are already a fair few API functions in there,
such as dowantarray and find_runcv. In a sense that file is doing double
duty as cop.c, and perhaps the utility functions out to be split out
into there, but that's a separate question.

> Also, have you had a look at Devel::Caller's upcontext function? It was
> originally based on the code you factored out into caller_cx, but is now
> rather different. I'm curious if the new caller_cx function provides
> everything that'd be needed to have Devel::Caller run on it.
> There's also many more caller() implementation in xs, but they all seem
> to be cargo-culted from Devel::Caller, so I guess it'd be nice if at
> least that one was possible to be ported to caller_cx.

I haven't (or, at least, not recently, so I can't remember the details).
I did look at PadWalker, and AFAICT caller_cx is *not* sufficient for
that, but I wasn't entirely following what was going on there.

I'm not sure it's worth making a large effort to accomodate all these
variants, but if its easy it would certainly be worth doing. My aim here
was mostly to provide XS with functionality that already exists from
Perl. After all, another more advanced call-stack API can always be
added later, if any of these people ever actually get to the point of
talking to p5p about what they're doing. :)

I'll do up another patch soon, after I've had a look at D::C.


Thread Previous | Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About