GSOC Status Report, Week 4

[Brian Fraser]

Howdy people.

I'm still going through all of the warning/error messages, so that's still uncommitted, but more on that in a couple of paragraphs.

First, let me quote Father C's mail to the previous report:

On Sun, Jun 19, 2011 at 4:17 PM, Father Chrysostomos <spr...@cpan.org> wrote:

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

 

I wouldn't know about HEKs being part of the api, but Reini's post here [0] mentions exposing HEKs, which prompted a similar woe in my last report.

I'm pretty sure that adding the macros would simplify the current stash code (I recall typing HEK_UTF8(HvNAME_HEK()) far too often), so if there's no objections to taking the easiest road, I'll go on ahead an add them. But assuming we go that way, a bit about the names of the macros: While I personally prefer HvNAME_LEN(), there are similar macros in gv.h that don't use underscores (GvNAMELEN, et al); Should I use use the underscore versions, or be consistent with the rest of the core? (Or consistently rename the GV macros? :P)

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.

Besides warnings, I worked on getting the gv.c functions a cleaner calling convention (i.e. _sv, _pv, and _pvn versions), but that's untested and undocumented, so I'm withholding the pushing; I also began tinkering with mro.c, which is apparently not going to be as excruciating as I had imagined, but I reserve the right to retract that once I get a couple of test cases going.

There was also a bit of an issue regarding compilation errors using ERRSV's PV (see doeval and die_unwind in pp_ctl.c), which meant that require/regexp compilation errors weren't clean - This is probably fixed, but I don't have a test for the "Unknown error" case.

This week I intend to finish the testing/cleanup of error messages - yyerrors aside. Plus, with some luck, finish mro too.

And finally, primarily to have them recorded somewhere outside of my computer, these are some stash/GV TODOs that I've been neglecting to address, in no particular order:

• call_method()
• SvUTF8() on GVs
• gv_fetchfile
  
• AUTOLOAD test cases

[0] http://www.nntp.perl.org/group/perl.perl5.porters/2011/06/msg173425.html 

[Zefram]

Brian Fraser wrote:
>assuming we go that way, a bit about the names of the macros: While I
>personally prefer HvNAME_LEN(), there are similar macros in gv.h that don't
>use underscores (GvNAMELEN, et al);

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

[Nicholas Clark]

On Tue, Jun 21, 2011 at 08:02:22AM -0300, Brian Fraser wrote:

> 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