Re: Package namespace and Safe init cleanup for plperl [PATCH]

On Mon, Jan 25, 2010 at 12:53, Tim Bunce <Tim.Bunce@pobox.com> wrote:
> - Added the 'warnings' pragma to the list of modules to load into Safe.
>  So plperl functions can now "use warnings;" - added test for that.

*yay*

> - Added 'use 5.008001;' to plc_perlboot.pl as a run-time check to
>  complement the configure-time check added by Tom Lane recently.

For the curious the above works quite nicely.:
=> do $$ 1; $$ language plperl;
ERROR:  Perl v5.20.1 required--this is only v5.10.1, stopped at line 1.BEGIN failed--compilation aborted at line 1.

A couple of comments. *note* I have not tested this as a whole yet
(due to rejects).

in plc_perboot.pl
+$funcsrc .= qq[ package main; undef *{'$name'}; *{'$name'} = sub {
$BEGIN $prolog $src } ];

Any thoughts on using a package other than main?  Maybe something like
package PlPerl or what not?  Not that I think it really matters...
But it might be nicer to see in say NYTprof ?

in plc_safe_ok.pl
+use vars qw($PLContainer $SafeClass @EvalInSafe @ShareIntoSafe);

Is there some reason not to use my here?  The only reason I can think
of is you want the *_init gucs to be able to stick things into
@ShareIntoSafe and friends?  And if so should we document that
somewhere (or was that in an earlier patch that i missed?)? Or does
vars do some other magic that im not seeing that we need here?

Also whats the use case for $SafeClass?  Multiple versions of Safe.pm?Other modules that are like Safe.pm? Im just
curious...

Hrm I also dont see the point of the new "use Safe;"  as we still eval
it in in plperl_safe_init() care to enlighten a lost soul?

Other than those really quite minor questions that are arguably me
nitpicking...  It looks great to me.