Bleadperl v5.19.2-138-g137da2b breaks JKEGL/Marpa-R2-2.064000.tar.gz #13130
Comments
From @andkgit bisect 137da2b is the first bad commit diagnostics Not yet on cpantesters and too large to reproduce, please try yourself perl -V Summary of my perl5 (revision 5 version 19 subversion 3) configuration: Characteristics of this binary (from libperl): -- |
From @cpansproutOn Sat Jul 27 14:28:11 2013, andreas.koenig.7os6VVqR@franz.ak.mind.de wrote:
It is relying on a bug in perl. It uses a single lexical variable whose value keeps changing, and The cited commit fixes that bug, such that closures work properly. The attached patch makes Marpa::R2 use constant.pm to create constants. Unfortunately, it is more complicated than it ought to be, because -- Father Chrysostomos |
From @cpansproutInline Patchdiff -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 |
|
The RT System itself - Status changed from 'new' to 'open' |
From bug-Marpa-R2@rt.cpan.org<URL: https://rt.cpan.org/Ticket/Display.html?id=87384 > 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 |
From @demerphqOn 28 July 2013 21:57, Father Chrysostomos via RT
Does this mean one cannot use sub foo () { 1 } to create a constant anymore? If so, IMO this breaks a lot of code. Yves -- |
From @LeontOn Mon, Jul 29, 2013 at 7:26 PM, demerphq <demerphq@gmail.com> wrote:
If I understand this correctly, the problem is that this doesn't do my $foo = 1; Because $foo itself keeps changing value. Leon |
From bug-Marpa-R2@rt.cpan.org<URL: https://rt.cpan.org/Ticket/Display.html?id=87384 > 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. |
1 similar comment
From bug-Marpa-R2@rt.cpan.org<URL: https://rt.cpan.org/Ticket/Display.html?id=87384 > 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. |
From @cpansproutOn Mon Jul 29 10:27:30 2013, demerphq wrote:
No. $ git describe It only applies to sub foo() { $variable_declared_outside }. -- Father Chrysostomos |
From @demerphqOn 29 July 2013 19:47, Leon Timmermans <fawaka@gmail.com> wrote:
Ah yes, it should be: *$bar= eval "sub () { $foo }"; So that the $foo is frozen. Yves -- |
From @demerphqOn 30 July 2013 05:25, Father Chrysostomos via RT
Ah yes, thanks. Sorry for getting confused. Yves -- |
From bug-Marpa-R2@rt.cpan.org<URL: https://rt.cpan.org/Ticket/Display.html?id=87384 > On Mon Jul 29 13:17:32 2013, JKEGL wrote:
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’. |
|
@cpansprout - Status changed from 'open' to 'resolved' |
From bug-Marpa-R2@rt.cpan.org<URL: https://rt.cpan.org/Ticket/Display.html?id=87384 > 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:
|
1 similar comment
From bug-Marpa-R2@rt.cpan.org<URL: https://rt.cpan.org/Ticket/Display.html?id=87384 > 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:
|
From bug-Marpa-R2@rt.cpan.org<URL: https://rt.cpan.org/Ticket/Display.html?id=87384 > Fixed in Marpa-R2 2.066000. Thanks! |
From @eserteDana Sub 27. Srp 2013, 14:28:11, andreas.koenig.7os6VVqR@franz.ak.mind.de reče:
Also affected: |
From [Unknown Contact. See original ticket]Dana Sub 27. Srp 2013, 14:28:11, andreas.koenig.7os6VVqR@franz.ak.mind.de reče:
Also affected: |
Migrated from rt.perl.org#119047 (status was 'resolved')
Searchable as RT119047$
The text was updated successfully, but these errors were encountered: