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

Re: newCONSTSUB_flags() fiddles with PL_curcop

Thread Previous | Thread Next
From:
Father Chrysostomos
Date:
September 9, 2012 22:30
Subject:
Re: newCONSTSUB_flags() fiddles with PL_curcop
Message ID:
A973F0D9-1BB4-4DDA-BB9F-4B2B4BD3911C@cpan.org
Nicholas Clark wrote:
> newCONSTSUB_flags() starts with this code:
> 
>     if (IN_PERL_RUNTIME) {
> 	/* at runtime, it's not safe to manipulate PL_curcop: it may be
> 	 * an op shared between threads. Use a non-shared COP for our
> 	 * dirty work */
> 	 SAVEVPTR(PL_curcop);
> 	 SAVECOMPILEWARNINGS();
> 	 PL_compiling.cop_warnings = DUP_WARNINGS(PL_curcop->cop_warnings);
> 	 PL_curcop = &PL_compiling;
>     }
>     SAVECOPLINE(PL_curcop);
>     CopLINE_set(PL_curcop, PL_parser ? PL_parser->copline : NOLINE);
> 
> 
> This all feels hacky. Why does it need to be set...
> 
> 
...
> "line 4" vs "line 1" despite the fact that the only difference between
> the two overlong 1-liners is the six character string "BEGIN "
> 
> 
> So, I'd like to take the above code out. If I remove it, the build fails:
> 
> GLOB_CSH is not a valid File::Glob macro at ../lib/File/Glob.pm line 66
> 
> The problem comes down to this bit of Perl_gv_fetchpvn_flags():
> 
>     if (!stash) {
>     no_stash:
> 	if (len && isIDFIRST_lazy(name)) {
> 
> ...
> 
> 	    if (global)
> 		stash = PL_defstash;
> 	    else if (IN_PERL_COMPILETIME) {
> 		stash = PL_curstash;
> ...
> 	    }
> 	    else
> 		stash = CopSTASH(PL_curcop);
> 
> 
> $expletive
> 
> The behaviour of the function Perl_gv_fetchpvn_flags() differs between
> "Compile Time" and "Run Time". That's really, um, crap.

PL_curstash is the copstash field of PL_compiling (sort of).  Most code should be using PL_curstash in preference to CopSTASH(&PL_compiling).  Generally CopSTASH(&PL_compiling) and PL_curstash are kept in tandem.  The only exception I know of is newCONSTSUB_flags, and that because I deleted the code that set CopSTASH(&PL_compiling), because the value wasn’t being used.

What gv_fetchpvn_flags is doing there is perfectly fine.

Formerly, newCONSTSUB would modify PL_curcop, but it was changed to use PL_compiling for thread-safety.

> 
> Specifically, the way that newCONSTSUB_flags() actually controls how
> gv_fetchpvn_flags() gets a stash to default from is by
> 
> * setting PL_curstash to the stash to use
> * assigning &PL_compiling to PL_curcop to make gv_fetchpvn_flags() notice this.
> 
> 
> This is not sane.

Oh yes it is!

Or: No it is not, because it should not even be getting to gv_fetchpvn_flags.  *That* is the real problem.

> 
> I think that the right way to fix this is to have a way to pass the default
> stash into Perl_gv_fetchpvn_flags(). Or even split it into two - one half
> that locates the stash to use, and the other half that takes a stash, and
> does the initialisation.

That in itself might be a worthy goal, but is unrelated to this issue.  See the very bottom of this message.

> A very ugly proof of concept is attached.

> diff --git a/embed.fnc b/embed.fnc
> index cb26c72..9fe6711 100644
> --- a/embed.fnc
> +++ b/embed.fnc
> @@ -888,6 +888,12 @@ Apda	|OP*	|newSLICEOP	|I32 flags|NULLOK OP* subscript|NULLOK OP* listop
>  Apda	|OP*	|newSTATEOP	|I32 flags|NULLOK char* label|NULLOK OP* o
>  Abm	|CV*	|newSUB		|I32 floor|NULLOK OP* o|NULLOK OP* proto \
>  				|NULLOK OP* block
> +p	|CV *	|newXS_len_flags_xxx|NULLOK const char *name|STRLEN len \
> +				|NN XSUBADDR_t subaddr\
> +				|NN const char *const filename \
> +				|NULLOK const char *const proto \
> +				|NULLOK SV **const_svp|U32 flags \
> +				|NULLOK HV *stash
>  p	|CV *	|newXS_len_flags|NULLOK const char *name|STRLEN len \
>  				|NN XSUBADDR_t subaddr\
>  				|NN const char *const filename \

There is no need to create another newXS* function there.  newXS_len_flags’ sole reason for existence is the need to have an internal (non-API) version with a palimpsestic parameter prototype.

> But it's clearly a hack. I'm not sure of the right way to move forwards.

Either set the line number of PL_compiling (the smallest change), or modify newXS_len_flags not to call gv_fetchpvn_flags when passed a GV (and add a GV parameter).


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