Bleadperl v5.21.6-89-gd648ffc breaks autobox #14423
Comments
From @cpansproutcommit d648ffc Remove op_const_class; just use the name on the stack See <https://rt.cpan.org/Ticket/Display.html?id=100819>. I suspect we have a blead regression that affects pure-Perl code, and not just autobox, and that the simplest fix will fix autobox as well. This definitely needs to be looked into before 5.22. -- Father Chrysostomos |
From @iabynOn Sat, Jan 17, 2015 at 10:41:04PM -0800, Father Chrysostomos wrote:
Autobox has XS code that changes PL_check[OP_ENTERSUB] to a function I would expect in the first instance that it is autobox which needs -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @LeontOn Sun, Jan 18, 2015 at 7:41 AM, Father Chrysostomos <
Suggested fix attached. Leon |
From @Leont0001-perl-123619-Only-make-stringy-classnames-shared.patchFrom 119e4602d2867d8ca7d154fd7f6820a18b78ed45 Mon Sep 17 00:00:00 2001
From: Leon Timmermans <fawaka@gmail.com>
Date: Wed, 21 Jan 2015 23:43:36 +0100
Subject: [PATCH] [perl #123619] Only make stringy classnames shared
This caused failures in autobox that expect their invocant to be something
other than a string (e.g. a number). See also [cpan #123619].
---
op.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/op.c b/op.c
index d2ce7b9..e01e58b 100644
--- a/op.c
+++ b/op.c
@@ -11894,7 +11894,7 @@ Perl_ck_subr(pTHX_ OP *o)
}
/* make class name a shared cow string to speedup method calls */
/* constant string might be replaced with object, f.e. bigint */
- if (const_class && !SvROK(*const_class)) {
+ if (const_class && SvPOK(*const_class)) {
STRLEN len;
const char* str = SvPV(*const_class, len);
if (len) {
--
2.2.0-369-g3b010e3
|
From @cpansproutOn Wed Jan 21 04:53:22 2015, davem wrote:
It’s not just autobox that is affected: $ perl5.20.1 -e 'use constant H => do { my $fh = do { local *F }; *$fh = *STDOUT{IO} }; H->autoflush(1)' -- Father Chrysostomos |
From @cpansproutOn Wed Jan 21 14:53:55 2015, LeonT wrote:
I think that would work, but I need to translate the 2-line one-liner from the previous message into a test, or someone needs to beat me to it. -- Father Chrysostomos |
From @kentfredricop/gv.t seemed like the right place for it. |
From @kentfredric0002-Add-test-for-glob-constants-accepting-method-calls.patchFrom 63f243c0d6085b2bb57f3fbef97a86e2613bd321 Mon Sep 17 00:00:00 2001
From: Kent Fredric <kentfredric@gmail.com>
Date: Thu, 22 Jan 2015 23:15:51 +1300
Subject: [PATCH] Add test for glob constants accepting method calls
This specific case tripped up autobox.pm.
---
t/op/gv.t | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/t/op/gv.t b/t/op/gv.t
index 081d280..97d0f3f 100644
--- a/t/op/gv.t
+++ b/t/op/gv.t
@@ -12,7 +12,7 @@ BEGIN {
use warnings;
-plan( tests => 271 );
+plan( tests => 273 );
# type coercion on assignment
$foo = 'foo';
@@ -1073,6 +1073,14 @@ package glob_constant_test {
::is eval { bar->() }, eval { &{+bar} },
'glob_constant->() is not mangled at compile time';
::is "$@", "", 'no error from eval { &{+glob_constant} }';
+ use constant quux => do {
+ local *F;
+ my $f = *F;
+ *$f = *STDOUT{IO};
+ };
+ ::is eval { quux->autoflush; 420 }, 420,
+ 'glob_constant->method() works';
+ ::is "$@", "", 'no error from eval { glob_constant->method() }';
}
{
--
2.2.1
|
From @kentfredricHaving applied these patches, autobox now installs, however, a subsequent error appears in Moose::Autobox with string handling. Namely, ("Hello\n")->chomp Is expected to die due to readonly modification. However, it doesn't. This: for my $x (q(Hello\n)) { Does still die however. |
From @kentfredric#!/usr/bin/env perl use strict; use Moose::Autobox; { ok( !$return->{rval}, "Constant from a loop is readonly" ); ok( !$return->{rval}, "In place constant is readonly" ); __END__ __OPTRACE_OUTPUT__ __CONCISE_OUTPUT__ perl -MO=Concise,-exec -Ilib /tmp/test.pl |
From @cpansproutBoth patches applied. Thank you to both of you. commit 7131132 Add test for glob constants accepting method calls commit c18e654 [perl #123619] Only make stringy classnames shared On Thu Jan 22 05:21:54 2015, kentfredric wrote:
Hmmm. That affects pure-Perl cases, too: $ ./perl -Ilib -e 'sub UNIVERSAL::undef { undef $_[0] } "Hello"->undef' Can we just make the new scalar read-only? What about referential identity? I think we need to modify the scalar in place. But it is going to be marked read-only already at this point. I think if it is a regular COW it is safe to convert it into a shared-hash COW, which would cover the vast majority of code that benefits from this optimisation. -- Father Chrysostomos |
From @jkeenanOn Thu Jan 22 22:23:42 2015, sprout wrote:
autobox now appears to be PASSing on blead. See, e.g., http://matrix.cpantesters.org/?dist=autobox+2.83 Does this make this ticket closable? Or, if there are issues other than whether autobox is passing or not, can we remove it from the list of tickets blocking 5.22? Thank you very much. -- |
From @karenetheridgeOn Fri Mar 06 18:54:36 2015, jkeenan wrote:
Moose::Autobox still has problems on 5.21.9: https://rt.perl.org/Public/Bug/Display.html?id=123619#txn-1327901 |
From @iabynOn Sat, Mar 07, 2015 at 12:42:44AM -0800, Karen Etheridge via RT wrote:
Now fixed with commit b99dfd9 keep FOO read-only in 'FOO'->f() -- |
|
@iabyn - Status changed from 'open' to 'pending release' |
From @khwilliamsonThanks for submitting this ticket The issue should be resolved with the release today of Perl v5.22, available at http://www.perl.org/get.html -- |
|
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#123619 (status was 'resolved')
Searchable as RT123619$
The text was updated successfully, but these errors were encountered: