Bug #87384 for Marpa-R2: [perl #119047] Bleadperl v5.19.2-138-g137da2b breaks JKEGL/Marpa-R2-2.064000.tar.gz

Sun Jul 28 15:58:01 2013 perlbug-followup [...] perl.org - Ticket created

CC:	perl5-porters [...] perl.org, bug-Marpa-R2 [...] rt.cpan.org
Subject:	[perl #119047] Bleadperl v5.19.2-138-g137da2b breaks JKEGL/Marpa-R2-2.064000.tar.gz
Date:	Sun, 28 Jul 2013 12:57:48 -0700
To:	"OtherRecipients of perl Ticket #119047":;
From:	"Father Chrysostomos via RT" <perlbug-followup [...] perl.org>

On Sat Jul 27 14:28:11 2013, andreas.koenig.7os6VVqR@franz.ak.mind.de wrote: Show quoted text

> git bisect > ---------- > 137da2b05b4b7628115049f343163bdaf2c30dbb is the first bad commit > commit 137da2b05b4b7628115049f343163bdaf2c30dbb > Author: Father Chrysostomos <sprout@cpan.org> > Date: Sun Jun 30 20:26:34 2013 -0700 > > [perl #79908] Stop sub inlining from breaking closures

It is relying on a bug in perl. It uses a single lexical variable whose value keeps changing, and creates closures that are not really closures but snapshots of the current value of the variable. The cited commit fixes that bug, such that closures work properly. (What it was relying on was an awful hack that, at the time it was added, was *supposed* to be transparent, but got implemented in a way that was not.) The attached patch makes Marpa::R2 use constant.pm to create constants. Unfortunately, it is more complicated than it ought to be, because constant.pm does not accept fully-qualified constant names. Is there any reason it shouldn’t? -- Father Chrysostomos --- via perlbug: queue: perl5 status: new https://rt.perl.org:443/rt3/Ticket/Display.html?id=119047

diff -rup Marpa-R2-2.064000-EIBDuD-orig/lib/Marpa/R2/Internal.pm Marpa-R2-2.064000-EIBDuD/lib/Marpa/R2/Internal.pm --- Marpa-R2-2.064000-EIBDuD-orig/lib/Marpa/R2/Internal.pm 2013-07-11 09:25:42.000000000 -0700 +++ Marpa-R2-2.064000-EIBDuD/lib/Marpa/R2/Internal.pm 2013-07-28 12:55:22.000000000 -0700 @@ -19,6 +19,7 @@ use 5.010; use strict; use warnings; use Carp; +use constant; use vars qw($VERSION $STRING_VERSION); $VERSION = '2.064000'; @@ -84,8 +85,10 @@ sub Marpa::R2::offset { Marpa::R2::exception("Unacceptable field name: $field") if $field =~ /[^A-Z0-9_]/xms; - my $field_name = $prefix . $field; - *{$field_name} = sub () {$offset}; + local *Marpa::R2::Internal::_temp:: = $prefix; + package Marpa::R2::Internal::_temp; + no warnings; + constant->import($field => $offset); } ## end for my $field (@fields) return 1; } ## end sub Marpa::R2::offset

Mon Jul 29 13:17:32 2013 jkegl [...] cpan.org - Correspondence added

RT-Send-CC:

sprout [...] cpan.org

Thanks, Father Chrysostomos, for the fix, which seems to work on *MOST* platforms. I incorporated it into Marpa-R2-2.065_003, which reports 4 failures on cpantesters. One seems unrelated to the fix, but the messages in the other three are of the kind that I'd expect if the constants are not being defined. As an example of one: Bareword "Marpa::R2::Internal::Grammar::TRACE_RULES" not allowed while "strict subs" in use at /usr/home/cpan/pit/64bit/conf/perl-5.14.1/.cpanplus/5.14.1/build/Marpa-R2-2.065_003/blib/lib/Marpa/R2/Grammar.pm line 148. This seems to indicate that the fix did not actually succeed in adding TRACE_RULES to the namespace Marpa::R2::Internal::Grammar. The reports are from 2 BSD variants and Linux, but all are for 5.14.x: http://www.cpantesters.org/cpan/report/5198f65e-f85c-11e2-8358-7df630b64f85 http://www.cpantesters.org/cpan/report/40725a58-f859-11e2-8f75-553d14be7dd6 http://www.cpantesters.org/cpan/report/ca977e96-f84c-11e2-8f75-553d14be7dd6

Mon Jul 29 13:17:33 2013 The RT System itself - Status changed from 'new' to 'open'

Mon Jul 29 19:39:35 2013 jkegl [...] cpan.org - Correspondence added

RT-Send-CC:

perl5-porters [...] perl.org, sprout [...] cpan.org

FYI anyone else working on this. I decided to work around the issue. These constants can be set at distribution build time, so I've decided to auto-generate a .pm file. This is easier all around from the point of view of getting Marpa working on all distributions.

Wed Jul 31 00:53:31 2013 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Mon Jul 29 13:17:32 2013, JKEGL wrote: Show quoted text

> Thanks, Father Chrysostomos, for the fix, which seems to work on > *MOST* platforms. > > I incorporated it into Marpa-R2-2.065_003, which reports 4 failures on > cpantesters. One seems unrelated to the fix, but the messages in the > other three are of the kind that I'd expect if the constants are not > being defined. As an example of one: > > Bareword "Marpa::R2::Internal::Grammar::TRACE_RULES" not allowed while > "strict subs" in use at /usr/home/cpan/pit/64bit/conf/perl- > 5.14.1/.cpanplus/5.14.1/build/Marpa-R2- > 2.065_003/blib/lib/Marpa/R2/Grammar.pm line 148. > > This seems to indicate that the fix did not actually succeed in adding > TRACE_RULES to the namespace Marpa::R2::Internal::Grammar.

I just had to get to the bottom of this.... It seems you omitted the ‘use constant’ part of my patch. On most perl versions it seems to be loaded by something else. Not so under 5.14. Sorry about the extra tickets. I forwarded the patch to this bug tracker from within perl’s bug tracker, and several responses to it were sent via ‘Reply All’.

Wed Jul 31 01:08:38 2013 jkegl [...] cpan.org - Correspondence added

RT-Send-CC:

perl5-porters [...] perl.org, sprout [...] cpan.org

Thanks for your patience with this issue, and with me as a slow learner. In retrospect the need for "use constant" is pretty obvious, and it explains the strange distribution of the symptoms -- where something else had required the "constant" pragma, it worked, and where nothing else had, it failed. Side effect city. As mentioned, I decided to back out the run-time calculation of constants in Marpa::R2. The values of the constants I needed were integers, known at thetime that the distribution was created. So my original solution was over-engineered and my new "backed out" solution is in fact better code -- simpler, and more efficient. It's implemented in my latest developer's releases. I am happy to learn my problem with your patch was my error and not a Perl issue. It's a bit of Perl infra-structure I may need someday, so it's a relief to know it's there and it works. On Wed Jul 31 00:53:31 2013, SPROUT wrote: Show quoted text

> On Mon Jul 29 13:17:32 2013, JKEGL wrote:

> > Thanks, Father Chrysostomos, for the fix, which seems to work on > > *MOST* platforms. > > > > I incorporated it into Marpa-R2-2.065_003, which reports 4 failures > > on > > cpantesters. One seems unrelated to the fix, but the messages in the > > other three are of the kind that I'd expect if the constants are not > > being defined. As an example of one: > > > > Bareword "Marpa::R2::Internal::Grammar::TRACE_RULES" not allowed > > while > > "strict subs" in use at /usr/home/cpan/pit/64bit/conf/perl- > > 5.14.1/.cpanplus/5.14.1/build/Marpa-R2- > > 2.065_003/blib/lib/Marpa/R2/Grammar.pm line 148. > > > > This seems to indicate that the fix did not actually succeed in > > adding > > TRACE_RULES to the namespace Marpa::R2::Internal::Grammar.

> > I just had to get to the bottom of this.... > > It seems you omitted the ‘use constant’ part of my patch. On most > perl versions it seems to be loaded by something else. Not so under > 5.14. > > Sorry about the extra tickets. I forwarded the patch to this bug > tracker from within perl’s bug tracker, and several responses to it > were sent via ‘Reply All’.

Sat Aug 03 13:18:49 2013 jkegl [...] cpan.org - Correspondence added

RT-Send-CC:

sprout [...] cpan.org

Fixed in Marpa-R2 2.066000. Thanks!

Sat Aug 03 13:18:50 2013 jkegl [...] cpan.org - Status changed from 'open' to 'resolved'

Sat Aug 03 13:18:50 2013 jkegl [...] cpan.org - Fixed in 2.065_006 added

Sat Aug 03 13:18:50 2013 jkegl [...] cpan.org - Fixed in 2.066000 added