Noticing that myself when I added HvENAME*, I was thinking of making HvNAME_HEK part of the API eventually, since HEKs already support UTF-8. But looking at perlapi now, I see things are not as straightforward as I imagined. It seems that HEKs are not really part of the API, although there are a few public functions that accept them as arguments. Maybe the easiest solution is to add four new public macros specific to stash names:
HvNAME_LEN
HvNAME_UTF8
HvENAME_LEN
HvENAME_UTF8
Best to be consistent with the existing stuff: HvNAMELEN() and
HvNAMEUTF8(). Also GvNAMEUTF8() when you get to it.
>What I did was merge newXS() into newXS_flags(), and simply leave newXS() as
>a wrapper to that with a 0 for the flags and proto.
That's a good resolution. Most of the foo()/foo_flags() pairs work that
way, newXS() was an oddity.
>Besides warnings, I worked on getting the gv.c functions a cleaner calling
>convention (i.e. _sv, _pv, and _pvn versions),
Good, we'll need that.
>This week I intend to finish the testing/cleanup of error messages -
I'm rather impressed that you're tackling these at this stage. There are
a lot of affected locations, and they're less interesting than the
substantive logic.
> - call_method()
Can take a UTF8 flag in the existing flags parameter. But taking only a
nul-terminated char* for the name, can it ever be used on arbitrary names?
*checks* Eek, method lookup loses on an embedded nul!
$ perl -lwe 'use Time::HiRes; print Time::HiRes->${\"VERSION\0foo"}'
1.9719
Not only is it not Unicode-clean, it's not nul-clean! Bring on the
_pvn/_pvs/_sv variants.
-zefram
> Moving on, one failing test revealed that I had missed a pretty important
> part these last weeks: sub X {} and sub X () {} are handled by different
> parts of the core, and the latter case remains unclean in the github repo.
> While I have it fixed locally, I'd rather get some input before pushing it
> into the review repo:
> Basically, newCONSTSUB() calls newXS_flags, which in turn calls newXS; This
> last one calls gv_fetchpv(), generating the stash/gv. However, even after
> adding a flags parameter to newCONSTSUB, since newXS doesn't take flags,
> gv_fetchpv ends up generating the wrong set of GVs for UTF-8 names.
> What I did was merge newXS() into newXS_flags(), and simply leave newXS() as
> a wrapper to that with a 0 for the flags and proto. This seems like the
> least intrusive change, but comments welcome.
Merging the two implementations feels like the right way to go.
I *think* I've looked at that before in the source and been bugged by it.
However, (as ever,) I fear that there will turn out to be subtle differences
in how the two handle certain corner cases, or bugs that have been fixed in
one but not the other, so we need to review the merge carefully to try to
minimise the "surprises".
Nicholas Clark