[perl #33156] perlapi.pod and the Perl_ prefix

On Mon, 10 Apr 2017 04:16:38 -0700, davem wrote:
> On Sat, Apr 08, 2017 at 10:08:27AM -0700, Karl Williamson via RT
> wrote:
> > On Thu, 07 Jul 2016 11:02:32 -0700, dcollinsn@gmail.com wrote:
> > > If we do want perlapi.pod to use the Perl_ prefix...
> > >
> > > embed.fnc:
> > >
> > > Afpda   |SV*    |newSVpvf       |NN const char *const pat|...
> > >
> > > :   f  Function takes a format string. If the function name
> > > /strftime/
> > > :      then its assumed to take a strftime-style format string as
> > > 1st
> > > arg;
> > > :      otherwise it's assumed to be a printf style format string,
> > > varargs
> > > :      (hence any entry that would otherwise go in embed.h is
> > > suppressed):
> > > :
> > > :         proto.h: add __attribute__format__ (or ...null_ok__)
> > > :
> > > :   p  Function in source code has a Perl_ prefix:
> > > :
> > > :         proto.h: function is declared as Perl_foo rather than foo
> > > :         embed.h: "#define foo Perl_foo" entries added
> > > :
> > > :   o  Has no Perl_foo or S_foo compatibility macro:
> > > :
> > > :         embed.h: suppress "#define foo Perl_foo"
> > > :
> > >
> > > Currently autodoc only looks for flags =~ /o/ to add the Perl_
> > > prefix
> > > to the function name. If the quote above is to be believed, it
> > > should
> > > also look for flags =~ /f/ && name !~ /strftime/
> > >
> > > But for some reason the f flag never makes it into docout() in
> > > autodoc.pl. I'm not sure why we don't just pass in all the flags
> > > from
> > > embed.fnc, but I modified autodoc.pl to at least pass in and check
> > > 'f', and prepend Perl_ if it is present and name !~ /strftime/.
> > >
> > > pod/perlapi.pod is not under version control, but comparing before
> > > and
> > > after gives these changes:
> > >
> > > 5844c5844
> > > <       char*   form(const char* pat, ...)
> > > ---
> > > > char*   Perl_form(const char* pat, ...)
> > > 5965c5965
> > > <       SV *    mess(const char *pat, ...)
> > > ---
> > > > SV *    Perl_mess(const char *pat, ...)
> > > 6006,6007c6006,6007
> > > <       int     my_snprintf(char *buffer, const Size_t len,
> > > <                           const char *format, ...)
> > > ---
> > > > int     Perl_my_snprintf(char *buffer, const Size_t len,
> > > >                          const char *format, ...)
> > > 9679c9679
> > > <       SV*     newSVpvf(const char *const pat, ...)
> > > ---
> > > > SV*     Perl_newSVpvf(const char *const pat, ...)
> > > 10027,10028c10027,10028
> > > <       void    sv_catpvf(SV *const sv, const char *const pat,
> > > <                         ...)
> > > ---
> > > > void    Perl_sv_catpvf(SV *const sv,
> > > >                        const char *const pat, ...)
> > > 10038,10039c10038,10039
> > > <       void    sv_catpvf_mg(SV *const sv,
> > > <                            const char *const pat, ...)
> > > ---
> > > > void    Perl_sv_catpvf_mg(SV *const sv,
> > > >                           const char *const pat, ...)
> > > 10887,10888c10887,10888
> > > <       void    sv_setpvf(SV *const sv, const char *const pat,
> > > <                         ...)
> > > ---
> > > > void    Perl_sv_setpvf(SV *const sv,
> > > >                        const char *const pat, ...)
> > > 10898,10899c10898,10899
> > > <       void    sv_setpvf_mg(SV *const sv,
> > > <                            const char *const pat, ...)
> > > ---
> > > > void    Perl_sv_setpvf_mg(SV *const sv,
> > > >                           const char *const pat, ...)
> > > 13998c13998
> > > <       void    croak(const char *pat, ...)
> > > ---
> > > > void    Perl_croak(const char *pat, ...)
> > > 14045c14045
> > > <       OP *    die(const char *pat, ...)
> > > ---
> > > > OP *    Perl_die(const char *pat, ...)
> > > 14125c14125
> > > <       void    warn(const char *pat, ...)
> > > ---
> > > > void    Perl_warn(const char *pat, ...)
> > >
> > > Patch to autodoc.pl is attached, if those changes are correct.
> >
> > No one responded to this.  Is there a reason not to apply this patch
> > in 5.27?
> 
> I don't understand this issue at all. Plenty of dual-lifed XS code
> (such
> as Encode) use a bare 'newSVpvf' rather than a full 'Perl_newSVpvf' in
> the
> src code. So why does perlapi need fixing to use the long form?

I finally took the time to understand this issue better.

Nowadays, the p flag means not what apparently it did at the time of the original ticket, but that the actual function in the source is named Perl_foo.  Generally, a macro is generated by the name 'foo' unless the 'o' flag is also specified, or perl is Configured with -DPERL_NO_SHORT_NAMES.  Thus there are plenty of calls to plain newSVpvf without a 'Perl_' prefix, and they work.  But they would fail under PERL_NO_SHORT_NAMES; and its any function, s not just newSVpvf.  It would seem that the fix to the proximal problem would be to change autodoc to add the o flag to every entry when PERL_NO_SHORT_NAMES is defined.  That would mean the generated perlapi would be much more configuration dependent than it is now.

But what happens if I do compile without short names.  I believe that any module I try to use that used the short names wouldn't compile.

And Devel::PPPort depends on modules using short names, so that it can potentially redefine the macro for a given short name to do the right thing, rather than the buggy first implementation.

And, concerning name space pollution.  Right now, if someone #defines a macro in a perl header file, that macro pollutes the name space of all XS code.  Apparently this hasn't been a problem.  There is at least a workaround for this, unlike real function calls, as the XS code can #undef the macro and redefine it.

Finally, in the ticket it looks like the example cited by Stas is violating the modern norm by pretending to be PERL_CORE when it isn't.  Perhaps that was acceptable back then; I wasn't involved at the time, but nowadays we don't support such things.

-- 
Karl Williamson

---
via perlbug:  queue: perl5 status: open
https://rt.perl.org/Ticket/Display.html?id=33156

• [perl #33156] perlapi.pod and the Perl_ prefix by Karl Williamson via RT