-
Notifications
You must be signed in to change notification settings - Fork 560
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bizarre closure lossage #9665
Comments
From zefram@fysh.orgCreated by zefram@fysh.org$ cat t0.pl $ perl5.8.9 t0.pl $ perl5.10.0 t0.pl 5.6 exhibits the expected behaviour. 5.8 and 5.10 are giving nonsense Perl Info
|
From zefram@fysh.orgAdditional: Matt Trout spotted that omitting the () prototype from the I've also tried it on 5.005 and 5.004, and they both operate correctly. -zefram |
From p5p@spam.wizbit.beOn Thu Feb 26 12:39:26 2009, zefram@fysh.org wrote:
Binary search: ----Program---- Change 1: ----EOF ($?='0')---- ----EOF ($?='0')---- http://public.activestate.com/cgi-bin/perlbrowse/p/7389 Subject: Re: Creating const subs for constants. http://public.activestate.com/cgi-bin/perlbrowse/p/7390 Add a testcase for #7389. Change 2: ----EOF ($?='0')---- ----EOF ($?='0')---- http://public.activestate.com/cgi-bin/perlbrowse/p/19637 Subject: [PATCH] jumbo closure fix |
The RT System itself - Status changed from 'new' to 'open' |
From @iabynOn Wed, Mar 11, 2009 at 05:50:54PM -0700, Bram via RT wrote:
[snip]
There is a hacky optimisation that looks for code like this: my $x; ie an anon sub with () prototype and whose body just returns an outer However, the code in Perl_op_const_sv() seems to be failing to correctly In particular, this fairly minimal example shows it returning a CONST sub: use Devel::Peek; and its the second call to Perl_op_const_sv() that's getting it wrong. -- |
From @iabynOn Mon, Mar 23, 2009 at 11:39:19AM -0500, David Nicol wrote:
What are you jabbering on about? And what is the diff below apropos to?
-- |
From @davidnicolOn Sat, Mar 21, 2009 at 8:44 AM, Dave Mitchell <davem@iabyn.com> wrote:
What? That's horrible! How are we supposed to write read-only --- /usr/lib/perl5/5.8/constant_original.pm 2009-03-23 + my $pkg = caller; # Normal constant name 1; +This is the feature that is lost by using string-eval rather than In the current implementation, scalar constants are actually |
From @nwc10On Sat, Mar 21, 2009 at 01:44:40PM +0000, Dave Mitchell wrote:
If anyone wants to beat Dave to it, the code in question starts here Nicholas Clark |
From @davidnicolOn Mon, Mar 23, 2009 at 1:43 PM, Dave Mitchell <davem@iabyn.com> wrote:
I rewrote constant.pm to create a code block to eval-string, instead After reading Nick's last, I see that the thing that needs to happen BEGIN { will then optimize pidconst but not getthing, which could be tested ok (63540 == const63540); Hmm. It isn't the reference counts, we're checking for that correctly Inline Patchdiff --git a/op.c b/op.c
index 78d9990..d0105ad 100644
--- a/op.c
+++ b/op.c
@@ -5460,6 +5460,7 @@ Perl_op_const_sv(pTHX_ const OP *o, CV *cv)
sv = PAD_BASE_SV(CvPADLIST(cv), o->op_targ);
/* the candidate should have 1 ref from this pad and 1 ref
* from the parent */
+ warn ("sv %s has refcount %i", SvPV_nolen(sv),
and by reordering things BEGIN { so that $x is set before creating the sub, and the other reference is Deferring the constification of the sub until the routine is used Document this as yet another thing that you just have to be aware of? -- |
From @nwc10On Tue, Mar 24, 2009 at 04:15:52PM -0500, David Nicol wrote:
I don't think that this is right.
I think that the correct fix is to make Perl_cv_const_sv() do what it's Making other changes would fix the symptoms, rather than the bug. Nicholas Clark |
From @iabynOn Tue, Mar 24, 2009 at 09:48:58PM +0000, Nicholas Clark wrote:
Exactly! -- |
From @iabynOn Tue, Mar 24, 2009 at 04:15:52PM -0500, David Nicol wrote:
I could see what the patch did. I just didn't know *why* you had submitted Because you didn't bother to say. So I have to waste my fucking time on it (when I know with 99% certainty PS - if you'd read my original post properly, you'd have seen that I'd -- |
From @davidnicolI have been asked what the patch I submitted on march 23 was supposed I had thought that the constant pragma created some strings and then The included patch is what it takes to switch C<use constant> from its |
From @cpansproutFixed by dbe92b0. |
@cpansprout - Status changed from 'open' to 'resolved' |
From @cpansproutOn Mon Nov 29 06:10:58 2010, sprout wrote:
But that fix papered over the inherent problem with the code: We are following op_next pointers without starting at the beginning of the chain. I was trying to understand the logic in op_const_sv this morning when it dawned on me that there wasn’t any. I’m about to refactor op_const_sv into 3 subs (it does three different things anyway). Then things will start to make sense and this type of bug will be less likely to recur. -- Father Chrysostomos |
Migrated from rt.perl.org#63540 (status was 'resolved')
Searchable as RT63540$
The text was updated successfully, but these errors were encountered: