Skip to content
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

Closed
p5pRT opened this issue Feb 26, 2009 · 16 comments
Closed

bizarre closure lossage #9665

p5pRT opened this issue Feb 26, 2009 · 16 comments

Comments

@p5pRT
Copy link

p5pRT commented Feb 26, 2009

Migrated from rt.perl.org#63540 (status was 'resolved')

Searchable as RT63540$

@p5pRT
Copy link
Author

p5pRT commented Feb 26, 2009

From zefram@fysh.org

Created by zefram@fysh.org

$ cat t0.pl
sub memoise {
  my($evaluate) = @​_;
  my $value;
  return sub () {
  #if(0) { $evaluate; }
  if(defined $evaluate) {
  $value = $evaluate->();
  $evaluate = undef;
  }
  return $value;
  };
}
my $a = memoise(sub { "ok" });
print $a->(), "\n";
$ perl5.6.0 t0.pl
ok
$ perl5.6.2 t0.pl
ok
$ perl5.8.0 t0.pl

$ perl5.8.9 t0.pl

$ perl5.10.0 t0.pl
CODE(0x8066ba8)
$

5.6 exhibits the expected behaviour. 5.8 and 5.10 are giving nonsense
results. If the "if(0) { $evaluate; }" line is uncommented, which
should be doubly a no-op, then all of these perl versions give the
correct result.

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl v5.8.8:

Configured by Debian Project at Fri Dec 19 00:43:54 EST 2008.

Summary of my perl5 (revision 5 version 8 subversion 8) configuration:
  Platform:
    osname=linux, osvers=2.6.18-6-686, archname=i486-linux-gnu-thread-multi
    uname='linux etch 2.6.18-6-686 #1 smp fri dec 12 16:48:28 utc 2008 i686 gnulinux '
    config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=i486-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.8 -Darchlib=/usr/lib/perl/5.8 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.8.8 -Dsitearch=/usr/local/lib/perl/5.8.8 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -Duseshrplib -Dlibperl=libperl.so.5.8.8 -Dd_dosuid -des'
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=define use5005threads=undef useithreads=define usemultiplicity=define
    useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
    use64bitint=undef use64bitall=undef uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include'
    ccversion='', gccversion='4.1.2 20061115 (prerelease) (Debian 4.1.1-21)', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
    perllibs=-ldl -lm -lpthread -lc -lcrypt
    libc=/lib/libc-2.3.6.so, so=so, useshrplib=true, libperl=libperl.so.5.8.8
    gnulibc_version='2.3.6'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:
    


@INC for perl v5.8.8:
    /etc/perl
    /usr/local/lib/perl/5.8.8
    /usr/local/share/perl/5.8.8
    /usr/lib/perl5
    /usr/share/perl5
    /usr/lib/perl/5.8
    /usr/share/perl/5.8
    /usr/local/lib/site_perl
    /usr/local/lib/perl/5.8.4
    /usr/local/share/perl/5.8.4
    .


Environment for perl v5.8.8:
    HOME=/home/zefram
    LANG (unset)
    LANGUAGE (unset)
    LC_CTYPE=en_GB
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/zefram/pub/i686-pc-linux-gnu/bin:/home/zefram/pub/common/bin:/usr/bin:/usr/X11R6/bin:/bin:/usr/local/bin:/usr/games
    PERL_BADLANG (unset)
    SHELL=/usr/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented Feb 26, 2009

From zefram@fysh.org

Additional​: Matt Trout spotted that omitting the () prototype from the
sub returned by memoise is another way to make it operate correctly on
all perl versions.

I've also tried it on 5.005 and 5.004, and they both operate correctly.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Mar 12, 2009

From p5p@spam.wizbit.be

On Thu Feb 26 12​:39​:26 2009, zefram@​fysh.org wrote​:

$ cat t0.pl
sub memoise {
my($evaluate) = @​_;
my $value;
return sub () {
#if(0) { $evaluate; }
if(defined $evaluate) {
$value = $evaluate->();
$evaluate = undef;
}
return $value;
};
}
my $a = memoise(sub { "ok" });
print $a->(), "\n";
$ perl5.6.0 t0.pl
ok
$ perl5.6.2 t0.pl
ok
$ perl5.8.0 t0.pl

$ perl5.8.9 t0.pl

$ perl5.10.0 t0.pl
CODE(0x8066ba8)
$

5.6 exhibits the expected behaviour. 5.8 and 5.10 are giving
nonsense
results. If the "if(0) { $evaluate; }" line is uncommented, which
should be doubly a no-op, then all of these perl versions give the
correct result.

Binary search​:

----Program----
sub memoise {
  my($evaluate) = @​_;
  my $value;
  return sub () {
  #if(0) { $evaluate; }
  if(defined $evaluate) {
  $value = $evaluate->();
  $evaluate = undef;
  }
  return $value;
  };
}
my $a = memoise(sub { "ok" });
print $a->(), "\n";
__END__

Change 1​:
----Output of ...l/p1rFte9/perl-5.7.0@​7386/bin/perl----
ok

----EOF ($?='0')----
----Output of ...l/ppS7IQi/perl-5.7.0@​7389/bin/perl----

----EOF ($?='0')----

http​://public.activestate.com/cgi-bin/perlbrowse/p/7389
Change 7389 by jhi@​alpha on 2000/10/21 14​:26​:45

  Subject​: Re​: Creating const subs for constants.
  From​: John Tobey <jtobey@​john-edwin-tobey.org>
  Date​: Fri, 20 Oct 2000 22​:03​:27 -0400 (EDT)
  Message-Id​: <m13mo0N-000FObC@​feynman.localnet>

http​://public.activestate.com/cgi-bin/perlbrowse/p/7390
Change 7390 by jhi@​alpha on 2000/10/21 14​:35​:57

  Add a testcase for #7389.

Change 2​:
----Output of .../pz1Wttf/perl-5.8.0@​19635/bin/perl----

----EOF ($?='0')----
----Output of .../pMqzSN2/perl-5.8.0@​19637/bin/perl----
CODE(0x8141478)

----EOF ($?='0')----
Need a perl between 19635 and 19637

http​://public.activestate.com/cgi-bin/perlbrowse/p/19637
Change 19637 by rgs@​rgs-home on 2003/05/29 18​:47​:40

  Subject​: [PATCH] jumbo closure fix
  From​: Dave Mitchell <davem@​fdgroup.com>
  Date​: Wed, 26 Feb 2003 14​:49​:47 +0000
  Message-ID​: <20030226144947.A14444@​fdgroup.com>

@p5pRT
Copy link
Author

p5pRT commented Mar 12, 2009

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

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2009

From @iabyn

On Wed, Mar 11, 2009 at 05​:50​:54PM -0700, Bram via RT wrote​:

On Thu Feb 26 12​:39​:26 2009, zefram@​fysh.org wrote​:

$ cat t0.pl
sub memoise {
my($evaluate) = @​_;
my $value;
return sub () {
#if(0) { $evaluate; }
if(defined $evaluate) {
$value = $evaluate->();
$evaluate = undef;
}
return $value;
};
}
my $a = memoise(sub { "ok" });
print $a->(), "\n";
$ perl5.6.0 t0.pl
ok
$ perl5.6.2 t0.pl
ok
$ perl5.8.0 t0.pl

$ perl5.8.9 t0.pl

$ perl5.10.0 t0.pl
CODE(0x8066ba8)
$

5.6 exhibits the expected behaviour. 5.8 and 5.10 are giving
nonsense
results. If the "if(0) { $evaluate; }" line is uncommented, which
should be doubly a no-op, then all of these perl versions give the
correct result.

Binary search​:

[snip]

http​://public.activestate.com/cgi-bin/perlbrowse/p/19637
Change 19637 by rgs@​rgs-home on 2003/05/29 18​:47​:40

Subject&#8203;: \[PATCH\] jumbo closure fix
From&#8203;: Dave Mitchell \<davem@&#8203;fdgroup\.com>
Date&#8203;: Wed\, 26 Feb 2003 14&#8203;:49&#8203;:47 \+0000
Message\-ID&#8203;: \<20030226144947\.A14444@&#8203;fdgroup\.com>

There is a hacky optimisation that looks for code like this​:

  my $x;
  ...
  sub () {$x}

ie an anon sub with () prototype and whose body just returns an outer
lexical, and if so, optimises it into a CONST sub who's value is the
current $x. This is needed to support 'use constant'.

However, the code in Perl_op_const_sv() seems to be failing to correctly
detect 'just a lexical' in the sub's body.

In particular, this fairly minimal example shows it returning a CONST sub​:

  use Devel​::Peek;
  my $x;
  Dump sub () {
  if ($x) { }
  return $x;
  };

and its the second call to Perl_op_const_sv() that's getting it wrong.
One day when I have more time, I'll try to fix this.

--
Thank God I'm an atheist.....

@p5pRT
Copy link
Author

p5pRT commented Mar 23, 2009

From @iabyn

On Mon, Mar 23, 2009 at 11​:39​:19AM -0500, David Nicol wrote​:

On Sat, Mar 21, 2009 at 8​:44 AM, Dave Mitchell <davem@​iabyn.com> wrote​:

There is a hacky optimisation that looks for code like this​:

   my $x;
   ...
   sub () {$x}

ie an anon sub with () prototype and whose body just returns an outer
lexical, and if so, optimises it into a CONST sub who's value is the
current $x. This is needed to support 'use constant'.

What? That's horrible! How are we supposed to write read-only
accessors then? Oh. The hack is needed for
http​://search.cpan.org/~saper/constant-1.17/lib/constant.pm#Magic_constants
to work. And read-only accessors will still work as methods, because
methods aren't inlined.

What are you jabbering on about? And what is the diff below apropos to?

--- /usr/lib/perl5/5.8/constant_original.pm 2009-03-23
10​:51​:16.288169400 -0500
+++ /usr/lib/perl5/5.8/constant.pm 2009-03-23 11​:33​:18.066527600 -0500
@​@​ -41,12 +41,14 @​@​
$constants{+shift} = undef;
}

+ my $pkg = caller;
+ my @​EvalMe = ("package $pkg;");
+
foreach my $name ( keys %constants ) {
unless (defined $name) {
require Carp;
Carp​::croak("Can't use undef as constant name");
}
- my $pkg = caller;

    \# Normal constant name
    if \($name =~ /^\_?\[^\\W\_0\-9\]\\w\*\\z/ and \!$forbidden\{$name\}\) \{

@​@​ -94,22 +96,37 @​@​
no strict 'refs';
my $full_name = "${pkg}​::$name";
$declared{$full_name}++;
+ my $body;
if ($multiple) {
my $scalar = $constants{$name};
- *$full_name = sub () { $scalar };
+ $scalar =~ /^(\d*\.?\d+)\z/;
+ $body = ( $1 ? $1 : qq{"\Q$scalar\E"} );
} else {
if (@​_ == 1) {
my $scalar = $_[0];
- *$full_name = sub () { $scalar };
+ $scalar =~ /^(\d*\.?\d+)\z/;
+ $body = ( $1 ? $1 : qq{"\Q$scalar\E"} );
} elsif (@​_) {
my @​list = @​_;
- *$full_name = sub () { @​list };
+ $body = join ',', map {
+ /^(\d*\.?\d+)\z/
+ ? $1 : qq/"\Q$_\E"/
+ } @​list;
} else {
- *$full_name = sub () { };
+ $body = '';
}
- }
+ };
+ push @​EvalMe, <<"THISPIECE";
+sub $name () {
+ $body
+}
+THISPIECE
}
}
+ eval join "\n",@​EvalMe,"1;" or
+ die join "\n",
+ "ERROR IN CODE CONSTRUCTION WITH",
+ @​EvalMe,"END CODE",$@​;
}

1;
@​@​ -270,6 +287,9 @​@​
value. References to tied variables, however, can be used as
constants without any problems.

+This is the feature that is lost by using string-eval rather than
+magical, constantifying read-only accessors.
+
=head1 TECHNICAL NOTES

In the current implementation, scalar constants are actually

--
SCO - a train crash in slow motion

@p5pRT
Copy link
Author

p5pRT commented Mar 23, 2009

From @davidnicol

On Sat, Mar 21, 2009 at 8​:44 AM, Dave Mitchell <davem@​iabyn.com> wrote​:

There is a hacky optimisation that looks for code like this​:

   my $x;
   ...
   sub () {$x}

ie an anon sub with () prototype and whose body just returns an outer
lexical, and if so, optimises it into a CONST sub who's value is the
current $x. This is needed to support 'use constant'.

What? That's horrible! How are we supposed to write read-only
accessors then? Oh. The hack is needed for
http​://search.cpan.org/~saper/constant-1.17/lib/constant.pm#Magic_constants
to work. And read-only accessors will still work as methods, because
methods aren't inlined.

--- /usr/lib/perl5/5.8/constant_original.pm 2009-03-23
10​:51​:16.288169400 -0500
+++ /usr/lib/perl5/5.8/constant.pm 2009-03-23 11​:33​:18.066527600 -0500
@​@​ -41,12 +41,14 @​@​
  $constants{+shift} = undef;
  }

+ my $pkg = caller;
+ my @​EvalMe = ("package $pkg;");
+
  foreach my $name ( keys %constants ) {
  unless (defined $name) {
  require Carp;
  Carp​::croak("Can't use undef as constant name");
  }
- my $pkg = caller;

  # Normal constant name
  if ($name =~ /^_?[^\W_0-9]\w*\z/ and !$forbidden{$name}) {
@​@​ -94,22 +96,37 @​@​
  no strict 'refs';
  my $full_name = "${pkg}​::$name";
  $declared{$full_name}++;
+ my $body;
  if ($multiple) {
  my $scalar = $constants{$name};
- *$full_name = sub () { $scalar };
+ $scalar =~ /^(\d*\.?\d+)\z/;
+ $body = ( $1 ? $1 : qq{"\Q$scalar\E"} );
  } else {
  if (@​_ == 1) {
  my $scalar = $_[0];
- *$full_name = sub () { $scalar };
+ $scalar =~ /^(\d*\.?\d+)\z/;
+ $body = ( $1 ? $1 : qq{"\Q$scalar\E"} );
  } elsif (@​_) {
  my @​list = @​_;
- *$full_name = sub () { @​list };
+ $body = join ',', map {
+ /^(\d*\.?\d+)\z/
+ ? $1 : qq/"\Q$_\E"/
+ } @​list;
  } else {
- *$full_name = sub () { };
+ $body = '';
  }
- }
+ };
+ push @​EvalMe, <<"THISPIECE";
+sub $name () {
+ $body
+}
+THISPIECE
  }
  }
+ eval join "\n",@​EvalMe,"1;" or
+ die join "\n",
+ "ERROR IN CODE CONSTRUCTION WITH",
+ @​EvalMe,"END CODE",$@​;
}

1;
@​@​ -270,6 +287,9 @​@​
value. References to tied variables, however, can be used as
constants without any problems.

+This is the feature that is lost by using string-eval rather than
+magical, constantifying read-only accessors.
+
=head1 TECHNICAL NOTES

In the current implementation, scalar constants are actually

@p5pRT
Copy link
Author

p5pRT commented Mar 23, 2009

From @nwc10

On Sat, Mar 21, 2009 at 01​:44​:40PM +0000, Dave Mitchell wrote​:

There is a hacky optimisation that looks for code like this​:

my $x;
\.\.\.
sub \(\) \{$x\}

ie an anon sub with () prototype and whose body just returns an outer
lexical, and if so, optimises it into a CONST sub who's value is the
current $x. This is needed to support 'use constant'.

However, the code in Perl_op_const_sv() seems to be failing to correctly
detect 'just a lexical' in the sub's body.

In particular, this fairly minimal example shows it returning a CONST sub​:

use Devel&#8203;::Peek;
my $x;
Dump sub \(\) \{
    if \($x\) \{ \}
    return $x;
\};

and its the second call to Perl_op_const_sv() that's getting it wrong.
One day when I have more time, I'll try to fix this.

If anyone wants to beat Dave to it, the code in question starts here
http​://perl5.git.perl.org/perl.git/blob/HEAD​:/op.c#l5401

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Mar 24, 2009

From @davidnicol

On Mon, Mar 23, 2009 at 1​:43 PM, Dave Mitchell <davem@​iabyn.com> wrote​:

What are you jabbering on about? And what is the diff below apropos to?

I rewrote constant.pm to create a code block to eval-string, instead
of assigning anonsubs to globs.

After reading Nick's last, I see that the thing that needs to happen
is that the constify hack should only happen when there are only so
many references to the lexical, so

  BEGIN {
  my ($x, $y, $z) = ($$, "undefined",$$);
  *const63540 = sub () { $x };
  *pidconst63540 = sub () { $z };
  *getthing63540 = sub () { $y };
  *setthing63540 = sub ($) { $y = shift };
  $x = 63540
  }

will then optimize pidconst but not getthing, which could be tested
with the above and then

  ok (63540 == const63540);
  ok ($$ == pidconst63540);
  setthing63540 "tintinabulation of the bells bells bells";
  ok (getthing63540 eq "tintinabulation of the bells bells bells");

Hmm. It isn't the reference counts, we're checking for that correctly

Inline Patch
diff --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),
SvREFCNT(sv)); /* dln */   if \(\!sv || SvREFCNT\(sv\) \!= 2\)   return NULL;   sv = newSVsv\(sv\);

and by reordering things

  BEGIN {
  my ($x, $y, $z) = ($$, "undefined",$$);
  *setthing63540 = sub ($) { $y = shift };
  $x = 63540;
  *const63540 = sub () { $x };
  *pidconst63540 = sub () { $z };
  *getthing63540 = sub () { $y };
  }

so that $x is set before creating the sub, and the other reference is
made before creating the sub, the tests pass.

Deferring the constification of the sub until the routine is used
instead of when it is assigned to the glob (if possible) would fix
both cases, as by then $x would be set and $y would have a higher
refcount.

Document this as yet another thing that you just have to be aware of?
Define setters before getters to avoid triggering constanting?

--
"I lived in Hannibal fifteen and a half years, altogether, then ran
away, according to the custom of persons who are intending to become
celebrated." -- Samuel Clemens

@p5pRT
Copy link
Author

p5pRT commented Mar 24, 2009

From @nwc10

On Tue, Mar 24, 2009 at 04​:15​:52PM -0500, David Nicol wrote​:

After reading Nick's last, I see that the thing that needs to happen
is that the constify hack should only happen when there are only so
many references to the lexical, so

I don't think that this is right.

Deferring the constification of the sub until the routine is used
instead of when it is assigned to the glob (if possible) would fix
both cases, as by then $x would be set and $y would have a higher
refcount.

Document this as yet another thing that you just have to be aware of?
Define setters before getters to avoid triggering constanting?

I think that the correct fix is to make Perl_cv_const_sv() do what it's
intended to do, rather than changing any of the code in Perl space. I like
the intended behaviour - that a closure with a prototype of () that returns
only a constant behaves like a subroutine with a prototype of () that returns
only a constant - both being marked as inlinable by the tokeniser.

Making other changes would fix the symptoms, rather than the bug.

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Mar 24, 2009

From @iabyn

On Tue, Mar 24, 2009 at 09​:48​:58PM +0000, Nicholas Clark wrote​:

On Tue, Mar 24, 2009 at 04​:15​:52PM -0500, David Nicol wrote​:
I think that the correct fix is to make Perl_cv_const_sv() do what it's
intended to do, rather than changing any of the code in Perl space. I like
the intended behaviour - that a closure with a prototype of () that returns
only a constant behaves like a subroutine with a prototype of () that returns
only a constant - both being marked as inlinable by the tokeniser.

Making other changes would fix the symptoms, rather than the bug.

Exactly!

--
"You may not work around any technical limitations in the software"
  -- Windows Vista license

@p5pRT
Copy link
Author

p5pRT commented Mar 24, 2009

From @iabyn

On Tue, Mar 24, 2009 at 04​:15​:52PM -0500, David Nicol wrote​:

On Mon, Mar 23, 2009 at 1​:43 PM, Dave Mitchell <davem@​iabyn.com> wrote​:

What are you jabbering on about? And what is the diff below apropos to?

I rewrote constant.pm to create a code block to eval-string, instead
of assigning anonsubs to globs.

I could see what the patch did. I just didn't know *why* you had submitted
the patch. I didn't know know whether it was intended to be a serious
proposal for a fix to be committed, or a proof of concept to illustrate
something, or an irrelevant "ooh look at me, aren't I clever" post, or
whatever.

Because you didn't bother to say.

So I have to waste my fucking time on it (when I know with 99% certainty
that in the end I'll discover it was bollocks anyway).

PS - if you'd read my original post properly, you'd have seen that I'd
already diagnosed the problem and even identified the function at fault,
and in what way it was at fault.

--
In England there is a special word which means the last sunshine
of the summer. That word is "spring".

@p5pRT
Copy link
Author

p5pRT commented Mar 30, 2009

From @davidnicol

I have been asked what the patch I submitted on march 23 was supposed
to be or do, so here is an explanation.

I had thought that the constant pragma created some strings and then
evaluated them, which as I understood things was the userland way of
creating a SvCONST subroutine. It is, except that magic variables,
which are supported by use constant, are not supported by that method.

The included patch is what it takes to switch C<use constant> from its
current method to a method involving evaluating constructed strings.
It was intended for discussion, not inclusion.

@p5pRT
Copy link
Author

p5pRT commented Nov 29, 2010

From @cpansprout

Fixed by dbe92b0.

@p5pRT
Copy link
Author

p5pRT commented Nov 29, 2010

@cpansprout - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this as completed Nov 29, 2010
@p5pRT
Copy link
Author

p5pRT commented Oct 30, 2014

From @cpansprout

On Mon Nov 29 06​:10​:58 2010, sprout wrote​:

Fixed by dbe92b0.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant