Perl / perl5

šŸŖ The Perl programming language
https://dev.perl.org/perl5/
Other
1.96k stars 555 forks source link

*CORE::GLOBAL::caller=\&CORE::caller doesn't work #12061

Open p5pRT opened 12 years ago

p5pRT commented 12 years ago

Migrated from rt.perl.org#112504 (status was 'open')

Searchable as RT112504$

p5pRT commented 12 years ago

From @xdg

Created by @xdg

Assigning *CORE​::GLOBAL​::caller = \&CORE​::caller does not appear to set up the CORE​::GLOBAL override in a way that allows later localization.

However\, *CORE​::GLOBAL​::caller = sub { goto &CORE​::caller } works just fine\, so I suspect that however &CORE​::caller is made to work does so in a way that doesn't quite resemble a compiled sub.

This seems likely to lead to subtle errors down the line.

I have not tested any of the other CORE subs for similar behavior.

Example files that demonstrate the problem are included inline below. (sigh -- anyone want to implement file attachment for perlbug\, since we don't yet have web-based RT?)

__Foo.pm__ use v5.15.3; use warnings; package Foo;

# fails *CORE​::GLOBAL​::caller = \&CORE​::caller;

# works # *CORE​::GLOBAL​::caller = sub { goto &CORE​::caller };

sub override {   my $sub = shift;   no warnings 'redefine';   local *CORE​::GLOBAL​::caller = \&fake_caller;   $sub->(); }

sub fake_caller {   return ("fake"\, "fake.pl"\, 1); }

1;

__test.pl__ use v5.15.3; use warnings; use Test​::More;

use Foo;

sub join_caller {   join " - "\, caller; }

# line 99 sub wrapper { shift->() }

is( wrapper( \&join_caller )\, "main - test.pl - 99"\, "not localized" ); is( Foo​::override( \&join_caller )\, "fake - fake.pl - 1"\, "localized" );

done_testing;

__output.log__ ok 1 - not localized not ok 2 - localized # Failed test 'localized' # at test.pl line 102. # got​: 'Foo - Foo.pm - 15' # expected​: 'fake - fake.pl - 1' 1..2 # Looks like you failed 1 test of 2.

Perl Info ``` Flags: category=core severity=medium Site configuration information for perl 5.15.9: Configured by david at Fri Apr 6 20:23:57 EDT 2012. Summary of my perl5 (revision 5 version 15 subversion 9) configuration: Platform: osname=linux, osvers=3.0.0-16-generic, archname=x86_64-linux uname='linux vulcan 3.0.0-16-generic #28-ubuntu smp fri jan 27 17:44:39 utc 2012 x86_64 x86_64 x86_64 gnulinux ' config_args='-de -Dprefix=/home/david/perl5/perlbrew/perls/perl-5.15.9 -Dusedevel' hint=recommended, useposix=true, d_sigaction=define useithreads=undef, usemultiplicity=undef useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2', cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='', gccversion='4.6.1', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='cc', ldflags =' -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib libs=-lnsl -ldb -ldl -lm -lcrypt -lutil -lc perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc libc=, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.13' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector' Locally applied patches: @INC for perl 5.15.9: /home/david/perl5/perlbrew/perls/perl-5.15.9/lib/site_perl/5.15.9/x86_64-linux /home/david/perl5/perlbrew/perls/perl-5.15.9/lib/site_perl/5.15.9 /home/david/perl5/perlbrew/perls/perl-5.15.9/lib/5.15.9/x86_64-linux /home/david/perl5/perlbrew/perls/perl-5.15.9/lib/5.15.9 . Environment for perl 5.15.9: HOME=/home/david LANG=en_US.UTF-8 LANGUAGE=en_US:en LC_COLLATE=C LC_CTYPE=en_US.UTF-8 LC_MESSAGES=en_US.UTF-8 LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/david/perl5/perlbrew/bin:/home/david/perl5/perlbrew/perls/perl-5.15.9/bin:~/bin:~/git/utility-scripts:/opt/perl/current/bin:/home/david/bin:/usr/lib/lightdm/lightdm:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:. PERLBREW_BASHRC_VERSION=0.39 PERLBREW_HOME=/home/david/.perlbrew PERLBREW_MANPATH=/home/david/perl5/perlbrew/perls/perl-5.15.9/man PERLBREW_PATH=/home/david/perl5/perlbrew/bin:/home/david/perl5/perlbrew/perls/perl-5.15.9/bin PERLBREW_PERL=perl-5.15.9 PERLBREW_ROOT=/home/david/perl5/perlbrew PERLBREW_VERSION=0.39 PERL_BADLANG (unset) PERL_EXTUTILS_AUTOINSTALL=--defaultdeps SHELL=/bin/bash ```
p5pRT commented 12 years ago

From @cpansprout

On Tue Apr 17 11​:28​:13 2012\, dagolden@​cpan.org wrote​:

This is a bug report for perl from dagolden@​cpan.org\, generated with the help of perlbug 1.39 running under perl 5.15.9.

----------------------------------------------------------------- [Please describe your issue here]

Assigning *CORE​::GLOBAL​::caller = \&CORE​::caller does not appear to set up the CORE​::GLOBAL override in a way that allows later localization.

However\, *CORE​::GLOBAL​::caller = sub { goto &CORE​::caller } works just fine\, so I suspect that however &CORE​::caller is made to work does so in a way that doesn't quite resemble a compiled sub.

This seems likely to lead to subtle errors down the line.

I have not tested any of the other CORE subs for similar behavior.

Example files that demonstrate the problem are included inline below. (sigh -- anyone want to implement file attachment for perlbug\, since we don't yet have web-based RT?)

__Foo.pm__ use v5.15.3; use warnings; package Foo;

# fails *CORE​::GLOBAL​::caller = \&CORE​::caller;

# works # *CORE​::GLOBAL​::caller = sub { goto &CORE​::caller };

sub override { my $sub = shift; no warnings 'redefine'; local *CORE​::GLOBAL​::caller = \&fake_caller; $sub->(); }

sub fake_caller { return ("fake"\, "fake.pl"\, 1); }

1;

__test.pl__ use v5.15.3; use warnings; use Test​::More;

use Foo;

sub join_caller { join " - "\, caller; }

# line 99 sub wrapper { shift->() }

is( wrapper( \&join_caller )\, "main - test.pl - 99"\, "not localized" ); is( Foo​::override( \&join_caller )\, "fake - fake.pl - 1"\, "localized" );

done_testing;

__output.log__ ok 1 - not localized not ok 2 - localized # Failed test 'localized' # at test.pl line 102. # got​: 'Foo - Foo.pm - 15' # expected​: 'fake - fake.pl - 1' 1..2 # Looks like you failed 1 test of 2.

Argh!!! I wrote a two-page response and the RT server failed to respond. Then when I clicked ā€˜Backā€™\, Firefox insisted on reloading the page. :-(

The new core subs use cv_set_call_checker to inline themselves. Any XS code could do the same thing with any sub (whether XS or no) to achieve the same effect.

The problem here is that we have two different things that people expect of subroutines​: (1) they should be able to inline themselves for speed and (2) lookup should always happen at run time for overridability.

These two expectations conflict\, resulting in things like bug #24250.

I donā€™t really know what the best way forward is. Iā€™m not even sure whether this should be called a bug.

One approach might be to special-case the way built-in keywords fall back to CORE​::GLOBAL​::\, by forcing the use of an entersub op. But that would break any CORE​::GLOBAL overrides that have prototypes. (Prototype application is done by the default call checker.)

--

Father Chrysostomos

p5pRT commented 12 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 12 years ago

From @cpansprout

On Tue Apr 17 14​:51​:16 2012\, sprout wrote​:

But that would break any CORE​::GLOBAL overrides that have prototypes.

Including a proper global caller override\, which would not exhibit this​:

$ perl5.15.9 -ce 'caller(1\,2)' Too many arguments for caller at -e line 1\, near "2) " -e had compilation errors.

$ perl5.15.9 -ce 'BEGIN { *CORE​::GLOBAL​::caller = sub { goto &CORE​::caller }; } caller(1\,2)' -e syntax OK

--

Father Chrysostomos

p5pRT commented 12 years ago

From @xdg

On Tue\, Apr 17\, 2012 at 5​:51 PM\, Father Chrysostomos via RT \perlbug\-followup@​perl\.org wrote​:

Argh!!! Ā I wrote a two-page response and the RT server failed to respond. Ā Then when I clicked ā€˜Backā€™\, Firefox insisted on reloading the page. :-(

Sorry to hear that. I hate it when that happens.

The problem here is that we have two different things that people expect of subroutines​: (1) they should be able to inline themselves for speed and (2) lookup should always happen at run time for overridability.

These two expectations conflict\, resulting in things like bug #24250.

That's a long and twisty bug report. I'd like to ignore the general case and focus on the *CORE​::GLOBAL​::XXX case\, which I think has a more limited set of issues.

I donā€™t really know what the best way forward is. Ā Iā€™m not even sure whether this should be called a bug.

I agree that the "right" approach is not obvious. I can see not wanting to sacrifice inline speed of CORE​::* functions for the sake of less frequently used CORE​::GLOBAL​::* functions\, but I think the current state is a bug at least in the sense that assigning an inlined subroutine (whether CORE​::* or anything else) to CORE​::GLOBAL should fail (or warn)\, because it explicitly breaks the existing behavior that allows overriding a CORE​::GLOBAL subroutine early in compilation and then localizing it at runtime to get dynamic behavior.

-- David

p5pRT commented 12 years ago

From @cpansprout

On Tue Apr 17 17​:04​:04 2012\, xdaveg@​gmail.com wrote​:

On Tue\, Apr 17\, 2012 at 5​:51 PM\, Father Chrysostomos via RT \perlbug\-followup@​perl\.org wrote​:

Argh!!! Ā I wrote a two-page response and the RT server failed to respond. Ā Then when I clicked ā€˜Backā€™\, Firefox insisted on reloading the page. :-(

Sorry to hear that. I hate it when that happens.

The problem here is that we have two different things that people expect of subroutines​: (1) they should be able to inline themselves for speed and (2) lookup should always happen at run time for overridability.

These two expectations conflict\, resulting in things like bug #24250.

That's a long and twisty bug report. I'd like to ignore the general case and focus on the *CORE​::GLOBAL​::XXX case\, which I think has a more limited set of issues.

I donā€™t really know what the best way forward is. Ā Iā€™m not even sure whether this should be called a bug.

I agree that the "right" approach is not obvious. I can see not wanting to sacrifice inline speed of CORE​::* functions for the sake of less frequently used CORE​::GLOBAL​::* functions\, but I think the current state is a bug at least in the sense that assigning an inlined subroutine (whether CORE​::* or anything else) to CORE​::GLOBAL should fail (or warn)\, because it explicitly breaks the existing behavior that allows overriding a CORE​::GLOBAL subroutine early in compilation and then localizing it at runtime to get dynamic behavior.

In my head I just canā€™t make this fit together. :-(

CORE​::GLOBAL allows the prototypes of overridable keywords to be changed.

Allowing the prototypes to change (i.e.\, calling the call checker) also allows things to be inlined.

So how do we force the sub call to compile down to an entersub while still allowing prototypes to work?

BTW\, we already have this problem with sub () { 1 }.

--

Father Chrysostomos

p5pRT commented 12 years ago

From @nwc10

On Tue\, Apr 17\, 2012 at 05​:12​:34PM -0700\, Father Chrysostomos via RT wrote​:

On Tue Apr 17 17​:04​:04 2012\, xdaveg@​gmail.com wrote​:

On Tue\, Apr 17\, 2012 at 5​:51 PM\, Father Chrysostomos via RT

The problem here is that we have two different things that people expect of subroutines​: (1) they should be able to inline themselves for speed and (2) lookup should always happen at run time for overridability.

These two expectations conflict\, resulting in things like bug #24250.

That's a long and twisty bug report. I'd like to ignore the general case and focus on the *CORE​::GLOBAL​::XXX case\, which I think has a more limited set of issues.

I don't think that you can ignore the general case.

Because I'm pretty sure that the *CORE​::GLOBAL​::XXX case has all the ingredients of the general case. Detail below.

I don't really know what the best way forward is. Ā I'm not even sure whether this should be called a bug.

I agree that the "right" approach is not obvious. I can see not wanting to sacrifice inline speed of CORE​::* functions for the sake of less frequently used CORE​::GLOBAL​::* functions\, but I think the current state is a bug at least in the sense that assigning an inlined subroutine (whether CORE​::* or anything else) to CORE​::GLOBAL should fail (or warn)\, because it explicitly breaks the existing behavior that allows overriding a CORE​::GLOBAL subroutine early in compilation and then localizing it at runtime to get dynamic behavior.

Right. Components of this are​:

1​: There is a concern that subroutine invocation is slow.   One way to solve this is to permit subroutines that automatically inline   themselves at compile time.

  By implication of the above design description\, these cannot be   overridden by localising at runtime.

2​: Historically the built in functions have been special\, in that they aren't   *actually* functions. For example\,

  a) you couldn't take references to them\, because they aren't actually   functions   b) there's no way to access feature-enabled built in functions other   than by declaring a scope with that feature enabled   c) XS code doesn't have any easy way to call OPs.   d) the tokeniser/parser has to deal with them differently from real   functions

2a - 2c are addressed by changing %CORE​:: so that one can get a reference to a function that acts as the opcode

However\, it gives a lot of scope to *also* solve 2d\, and simplify a very messy thing\, if these functions aren't just "acts as the opcode" but *is* the opcode. This lets to an implementation whereby most of the internals code doesn't *need* to make a distinction between the (historical) built-in functions and any other function\, because the same mechanism is used for both.

This\, 2d\, means that the functions provided in %CORE​:: need to be of the self-inlining variety.

They are new. This isn't a change from old behaviour.

 

In my head I just can't make this fit together. :-(

I'm not convinced that all the desired features do fit together.

CORE​::GLOBAL allows the prototypes of overridable keywords to be changed.

Without warning. I'm wondering if it should warn\, given that this warns​:

$ perl -le 'sub CORE​::GLOBAL​::gzgetss($$;$) {}; *CORE​::GLOBAL​::gzgetss = sub {}' Prototype mismatch​: sub CORE​::GLOBAL​::gzgetss ($$;$) vs none at -e line 1.

ie %CORE​::GLOBAL​:: should either *be* pre-populated with references to all the built-ins\, or should act as if they are.

Should that be a new ticket?

Allowing the prototypes to change (i.e.\, calling the call checker) also allows things to be inlined.

So how do we force the sub call to compile down to an entersub while still allowing prototypes to work?

BTW\, we already have this problem with sub () { 1 }.

And we will have this problem with any other inlining subroutine that someone puts in %CORE​::GLOBAL​::

which gets to

3​: Consistency - if subroutine in %CORE​::GLOBAL​:: is one that inlines\, it   should inline.

which\, I would argue\, means that things like this

  *CORE​::GLOBAL​::getppid = \&CORE​::getppid;

is

a) taking a reference to a known self-inlining subroutine b) assigning a reference to a self-inlining subroutine into the parser's   global lookup table

and hence should behave the same way as

  sub PI () { 4; };   *CORE​::GLOBAL​::getppid = \*PI;

If we have self-inlining subroutines\, then those subroutines by intent can't be localised at runtime\, because they're already been inlined away If we have %CORE​:: exposing the built-ins are self-inlining subroutines\, then anywhere they are used\, they can't be localised at runtime.

We break consistency of the internals if the built in functions exposed by %CORE​:: aren't self-inlining. We break consistency of %CORE​::GLOBAL​:: if the parser treats values there differently if they came from %CORE​::

To do that seems to end up with a few possible ways to cope

1) any self-inlining function found in %CORE​::GLOBAL​:: doesn't inline   Even end-user XS code written with the intent of overridden builtins 2) functions from %CORE​:: are specially magic\, and if found in   %CORE​::GLOBAL​:: don't inline. That lets the core do something that XS   can't. It's special case. It seems particularly undesirable. 3) functions referenced by %CORE​:: have an additional flag "don't self-   inline when found in %CORE​::GLOBAL​::"   This flag is not the default (XS writers generally happy\, as this now   gives them the choice to write either sort of code)

I appreciate the desire to be able to run-time localise any function that is called\, I can't see a way to square this with self-inlining functions. Given that this *does* work as a way to run-time localise​:

  *CORE​::GLOBAL​::getppid = sub () { goto &CORE​::getppid; };

I'm still preferring what I'd view as option 0

0) any self-inlining function found in %CORE​::GLOBAL​:: inlines.   If you want to prevent this\, use a wrapper.

as it makes the common things easy\, the uncommon things are still possible\, and it has the least special cases\, exceptions or flags.

Nicholas Clark

p5pRT commented 12 years ago

From zefram@fysh.org

Nicholas Clark wrote​:

I'm still preferring what I'd view as option 0

0) any self-inlining function found in %CORE​::GLOBAL​:: inlines. If you want to prevent this\, use a wrapper.

+1. Runtime-swappable overriding of builtins is a rather specialised requirement\, and we shouldn't compromise either architecture or normal-case efficiency for it. Those who really want such funny overrides can just be required to hook sufficiently early and sufficiently explicitly.

-zefram

p5pRT commented 12 years ago

From @xdg

On Fri\, Apr 20\, 2012 at 10​:05 AM\, Nicholas Clark \nick@​ccl4\.org wrote​:

I don't think that you can ignore the general case.

Because I'm pretty sure that the *CORE​::GLOBAL​::XXX case has all the ingredients of the general case. Detail below.

I see *CORE​::GLOBAL​:XXX as a special case because it (more or less) replaces an op with a function call for all subsequent compilation (yes\, I'm handwaving the mechanics a bit\, please bear with me).

In other words\, it was an *explicit* mechanism for replacing an "inline" thing (an op) with a dynamic thing (a function).

That is very distinct from the more general desire that Foo​::bar() resolve at runtime which conflicts with the desire for a Foo​::bar() that gets inlined. The *mechanism* is the similar between the cases\, but the semantics are different.

(Arguably\, the original intended semantic was "replace this builtin with something that acts like a builtin" and "becomes dynamic" was an implementation detail -- but it's worked like that for so long that the semantic is effectively both things together.)

CORE​::GLOBAL allows the prototypes of overridable keywords to be changed.

Without warning. I'm wondering if it should warn\, given that this warns​:

$ perl -le 'sub CORE​::GLOBAL​::gzgetss($$;$) {}; *CORE​::GLOBAL​::gzgetss = sub {}' Prototype mismatch​: sub CORE​::GLOBAL​::gzgetss ($$;$) vs none at -e line 1.

ie %CORE​::GLOBAL​:: should either *be* pre-populated with references to all the built-ins\, or should act as if they are.

Should that be a new ticket?

I think there should be a warning if a subroutine is assigned to *CORE​::GLOBAL​::X where X is a builtin and the subroutine assigned doesn't have the matching prototype. I agree that should be a new ticket.

I want to defer considering the proposed mechanism for a bit until I respond to the inline issue.

3​: Consistency - if subroutine in %CORE​::GLOBAL​:: is one that inlines\, it Ā  should inline. [snip] We break consistency of the internals if the built in functions exposed by %CORE​:: aren't self-inlining. We break consistency of %CORE​::GLOBAL​:: if the parser treats values there differently if they came from %CORE​::

To do that seems to end up with a few possible ways to cope

1) any self-inlining function found in %CORE​::GLOBAL​:: doesn't inline Ā  Even end-user XS code written with the intent of overridden builtins 2) functions from %CORE​:: are specially magic\, and if found in Ā  %CORE​::GLOBAL​:: don't inline. That lets the core do something that XS Ā  can't. It's special case. It seems particularly undesirable. 3) functions referenced by %CORE​:: have an additional flag "don't self- Ā  inline when found in %CORE​::GLOBAL​::" Ā  This flag is not the default (XS writers generally happy\, as this now Ā  gives them the choice to write either sort of code)

I'm still preferring what I'd view as option 0

0) any self-inlining function found in %CORE​::GLOBAL​:: inlines. Ā  If you want to prevent this\, use a wrapper.

In this conversation\, I realized that even prior to 5.15\, you can assign a constant sub to *CORE​::GLOBAL​::X and hit the same problem. E.g.

  *CORE​::GLOBAL​::rand = sub() { 0 };

That exhibits the same problem with localization as the caller example.

On reflection\, I mostly agree with your "option 0" (for the reasons Zefram gives)\, with one caveat. I think attempting to localize *CORE​::GLOBAL​::XXX should warn if it is already assigned to an inline function. E..g

  *CORE​::GLOBAL​::rand = sub() { 0 };   local *CORE​::GLOBAL​::rand = \&better_rand; # should warn

I think that makes a lot of sense when localizing *CORE​::GLOBAL because *CORE​::GLOBAL has the semantic of turning an inline op into a dynamic subroutine and once an inline sub is assigned to it\, it won't work the way one expects. I think that's a useful warning because of the global nature of such assignments -- any code *anywhere* could assign an inline sub and your code would never know when trying to localize. (Is there even a way for non XS code to detect such a case?)

I don't think that should warn generally for localizing *Foo​::bar where *Foo​::bar{CODE} is an inline sub\, because someone might be localizing *Foo​::bar for the effects on other\, non CODE slots.

Then\, addressing the earlier question about the behavior of *CORE​::GLOBAL with respect to prototypes\, I'm not sure it makes sense to prepopulate %CORE​::GLOBAL​:: with refs to all the built-ins. That would break code that checks for *CORE​::GLOBAL​::XXX{CODE} to see if a global override is in effect.

On the other hand\, doing so would ensure the warning about localizing *CORE​::GLOBAL without prior assignment of non inline subroutine -- but I think the breakage is unacceptable. I do think there would be benefit to another type of warning. I.e. localizing *CORE​::GLOBAL​::caller when nothing has been assigned to *CORE​::GLOBAL​::caller should warn as well.

Summary​: I think localizing *CORE​::GLOBAL​::XXX should warn in two different cases​:

  - *CORE​::GLOBAL​::XXX{CODE} is an inline sub   - *CORE​::GLOBAL​::XXX{CODE} is not defined

It seems like that would sufficiently address the issue of surprising behavior without compromising the benefits of CORE subs as implemented or the ability to replace a built-in with an inline sub for efficiency.

It might also be good for some existing or new module to offer a C\<\< is_inline(\&fcn)>> function to allow things to detect and be smarter about such overrides.

-- David

p5pRT commented 12 years ago

From @cpansprout

On Fri Apr 20 07​:06​:10 2012\, nicholas wrote​:

ie %CORE​::GLOBAL​:: should either *be* pre-populated with references to all the built-ins\, or should act as if they are.

Should that be a new ticket?

Probably. (Now Iā€™m straying from the topic of this ticket​:) That answers a question I had not yet formulated. I was wondering how the CORE namespace would eventually be used for keyword lookup. What you suggest leads to something interesting. You seem to be implying that one could create new keywords by adding subs to CORE​::GLOBAL​::. Now *that* would be nice. But how would it interact with feature.pm?

(Maybe we should move this to a new ticket. :-)

--

Father Chrysostomos

p5pRT commented 12 years ago

From @cpansprout

On Fri Apr 20 10​:06​:41 2012\, xdaveg@​gmail.com wrote​:

I think attempting to localize *CORE​::GLOBAL​::XXX should warn if it is already assigned to an inline function. E..g

\*CORE&#8203;::GLOBAL&#8203;::rand = sub\(\) \{ 0 \};
local \*CORE&#8203;::GLOBAL&#8203;::rand = \\&better\_rand;  \# should warn

I donā€™t think there is any way to determine whether a subroutineā€™s call checker will inline it. A custom call checker might inline only on odd dates\, for instance. Or a call checker might inline a sub call to a *different* sub (I can think of real uses for that).

I do think there would be benefit to another type of warning. I.e. localizing *CORE​::GLOBAL​::caller when nothing has been assigned to *CORE​::GLOBAL​::caller should warn as well.

I donā€™t understand how that would help. Could you elaborate?

--

Father Chrysostomos

p5pRT commented 12 years ago

From @xdg

On Fri\, Apr 20\, 2012 at 4​:42 PM\, Father Chrysostomos via RT \perlbug\-followup@&#8203;perl\.org wrote​:

I do think there would be benefit to another type of warning. Ā I.e. localizing *CORE​::GLOBAL​::caller when nothing has been assigned to *CORE​::GLOBAL​::caller should warn as well.

I donā€™t understand how that would help. Ā Could you elaborate?

In "Foo.pm" -- note commented out line​:

  package Foo;

  # *CORE​::GLOBAL​::caller = sub { goto \&CORE​::caller };

  sub override {   my $sub = shift;   local *CORE​::GLOBAL​::caller = \&fake_caller;   $sub->();   }

  sub fake_caller {   return ("fake"\, "fake.pl"\, 1);   }

  1;

In "test.pl"

  use Test​::More;   use Foo;

  sub join_caller { join " - "\, caller; }

  is( Foo​::override( \&join_caller )\, "fake - fake.pl - 1" );

  done_testing;

With the initial C\<\< local *CORE​::GLOBAL​::caller = ... >> commented out\, test.pl compiles "caller" to the caller op instead of to the wrapper subroutine around "goto \&CORE​::caller". Thus\, calling C\<\<local *CORE​::GLOBAL​::caller = ... >> in Foo​::override() has no effect\, just as if *CORE​::GLOBAL​::caller had an inline sub assigned to it.

N.B. This is why Sub​::Uplevel has to assign a (pure perl) *CORE​::GLOBAL​::caller as early as possible -- it's a placeholder so that subsequent compilation gets the sub and not the op and thus can be localized later when uplevel is called.

Thus\, I think they are similar kinds of errors -- someone is trying to localize in a situation where it's almost certainly not going to work as expected.

There could be situations where localizing without a prior override is intentional for the purpose of temporarily overriding something compiled at runtime (e.g. via string eval()) but I think that's an even more unlikely/obscure case and someone can just turn off the warning if they're doing it on purpose.

-- David

p5pRT commented 12 years ago

From zefram@fysh.org

David Golden wrote​:

In other words\, it was an *explicit* mechanism for replacing an "inline" thing (an op) with a dynamic thing (a function).

No\, it's an explicit mechanism for replacing an op with *something else*. The ability to change that something else at runtime is an accident of implementation\, as is the general ability to replace functions at runtime. In both cases\, that ability is only occasionally used. If you want something that's reliably dynamic\, I see no problem with requiring you to write in the dynamism explicitly.

I think there should be a warning if a subroutine is assigned to *CORE​::GLOBAL​::X where X is a builtin and the subroutine assigned doesn't have the matching prototype.

I agree with this\, if it can be done cleanly.

It might also be good for some existing or new module to offer a C\<\< is_inline(\&fcn)>> function to allow things to detect and be smarter about such overrides.

Not possible in the general case. You could perhaps do a might_inline()\, but that can't be automatically determined\, it would have to be an explicit flag on the sub.

-zefram