Bleadperl v5.21.6-336-gfedf30e breaks TOKUHIROM/B-Tap-0.14.tar.gz #14316
Comments
From @andkbisect commit fedf30e Add OP_MULTIDEREF report http://www.cpantesters.org/cpan/report/5f687062-7e1f-11e4-a49a-1b5452b25338 also affected VPIT/autovivification-0.14.tar.gz perl -V Summary of my perl5 (revision 5 version 21 subversion 7) configuration: Characteristics of this binary (from libperl): |
From perl@profvince.comLe 08/12/2014 19:26, (Andreas J. Koenig) (via RT) a écrit :
autovivification.pm needs to be taught about this new op. I won't work Vincent |
|
The RT System itself - Status changed from 'new' to 'open' |
From @andk
One piece of innovation in this commit is the new warning "Use of http://www.cpantesters.org/cpan/report/c7ded7f8-7ed8-11e4-aaeb-9f7752b25338 I came to the conclusion[0]: https://metacpan.org/source/GWILLIAMS/RDF-Trine-1.011/lib/RDF/Trine/Store/Hexastore.pm#L308 The two relevant lines are 308 and 38 and the term NODEMAP->{ $_ } -- |
From @cpansproutOn Tue Dec 09 13:25:44 2014, andreas.koenig.7os6VVqR@franz.ak.mind.de wrote:
For once, it was not one of my commits. :-)
Confirmed: use constant NODEMAP $ pbpaste|./perl -Ilib -w -MO=Concise So it is happening at compile time, which seems wrong to me, since this was always a run-time warning. -- Father Chrysostomos |
From @iabynOn Tue, Dec 09, 2014 at 05:56:47PM -0800, Father Chrysostomos via RT wrote:
It's right that its now (also) a compile-time warning, since The error is that S_maybe_multideref() does the check as soon as it sees $_[ NODEMAP->{ subject } ] and thinks its actually seen $_[ NODEMAP ] and issues the warning. I'm working up a fix right now. -- |
From @iabynOn Wed, Dec 10, 2014 at 12:55:08PM +0000, Dave Mitchell wrote:
Now done: commit b48c4cb fix spurious 'Use of reference' warning -- |
From @cpansproutOn Wed Dec 10 05:03:17 2014, davem wrote:
For constant folding, we forego the optimisation if it would change when warnings occur. For multideref I think we should do something similar, but we don’t need to forego the warning altogether. How about relocating the constant to the pad, even under non-threaded builds, and treating it as a lexical variable? Another consideration is that an overloaded object may return a different value later on, or an object may lose or gain overloading by the time the code runs, especially if we have not reached the ‘use overload’ statement yet. -- Father Chrysostomos |
From @iabynOn Wed, Dec 10, 2014 at 05:31:52AM -0800, Father Chrysostomos via RT wrote:
I think the idea of evaluating a constant at compile-time is already Storing the constant in the pad is going to be slower than directly Comparing $r->[0] using bench.pl --raw $r->[0] $r->[$i] -- |
From @cpansproutOn Wed Dec 10 08:18:23 2014, davem wrote:
Never mind my overloading concern, then. :-)
I meant store it in the pad like a lexical only if it is a reference. If we ignore overloading, then we can do that only if we would have warned about it. That would make this optimisation consistent with constant folding. -- Father Chrysostomos |
From @cpansproutOn Wed Dec 10 08:49:28 2014, sprout wrote:
Do you have any objections to the attached patch? -- Father Chrysostomos |
From @cpansproutFrom 0f03985 Mon Sep 17 00:00:00 2001 The multideref optimisation was making it into a compile-time warn- In those cases where we would have (or might have) warned at compile Inline Patchdiff --git a/dump.c b/dump.c
index afa40cd..fbc09e5 100644
--- a/dump.c
+++ b/dump.c
@@ -2285,7 +2285,8 @@ S_append_padvar(pTHX_ PADOFFSET off, CV *cv, SV *out, int n,
if (paren)
sv_catpvs_nomg(out, "(");
for (i = 0; i < n; i++) {
- if (namepad && (sv = padnamelist_fetch(namepad, off + i)))
+ if (namepad && (sv = padnamelist_fetch(namepad, off + i))
+ && PadnameLEN(sv))
{
STRLEN cur = SvCUR(out);
Perl_sv_catpvf(aTHX_ out, "[%"PNf, PNfARG(sv));
diff --git a/lib/B/Deparse.pm b/lib/B/Deparse.pm
index 0535262..6e16387 100644
--- a/lib/B/Deparse.pm
+++ b/lib/B/Deparse.pm
@@ -3994,7 +3994,10 @@ sub pp_multideref {
}
}
elsif (($actions & MDEREF_INDEX_MASK) == MDEREF_INDEX_padsv) {
- $text .= $self->padname(shift @items);
+ my $pn = $self->padname_sv(my $off = shift @items);
+ $text .= $pn->LEN
+ ? $self->padname($off)
+ : $self->const($self->padval($off), $cx);
}
elsif (($actions & MDEREF_INDEX_MASK) == MDEREF_INDEX_gvsv) {
$text .= '$' . ($self->stash_variable_name('$', shift @items))[0];
diff --git a/op.c b/op.c
index cef99f2..30bb8a4 100644
--- a/op.c
+++ b/op.c
@@ -12398,11 +12398,22 @@ S_maybe_multideref(pTHX_ OP *start, OP *orig_o, UV orig_action, U8 hints)
else {
/* it's a constant array index */
SV *ix_sv = cSVOPo->op_sv;
- if (pass && UNLIKELY(SvROK(ix_sv) && !SvGAMAGIC(ix_sv)
- && ckWARN(WARN_MISC)))
- Perl_warner(aTHX_ packWARN(WARN_MISC),
- "Use of reference \"%"SVf"\" as array index",
- SVfARG(ix_sv));
+ if (UNLIKELY( SvROK(ix_sv) && !SvGAMAGIC(ix_sv)
+ && ( ckWARN(WARN_MISC)
+ || isLEXWARN_off)))
+ {
+ if (pass) {
+ PADOFFSET ix = arg->pad_offset =
+ pad_alloc(OP_CONST, SVf_READONLY);
+ SvREFCNT_dec(PAD_SVl(ix));
+ PAD_SETSV(ix, ix_sv);
+ cSVOPo->op_sv = NULL;
+ }
+ arg++;
+ index_type = MDEREF_INDEX_padsv;
+ o = o->op_next;
+ break;
+ }
iv = SvIV(ix_sv);
if ( action_count == 0
diff --git a/t/lib/warnings/pp_hot b/t/lib/warnings/pp_hot
index 702df08..65d2f1a 100644
--- a/t/lib/warnings/pp_hot
+++ b/t/lib/warnings/pp_hot
@@ -350,10 +350,15 @@ use constant FOO => { a => 1 };
EXPECT
########
-use warnings 'misc';
use constant FOO => {};
-() = $_[FOO];
+$^W=1;
+for(1..2){ () = $_[FOO]; }
+use warnings 'misc';
+for(1..2){ () = $_[FOO]; }
EXPECT
OPTION regex
Use of reference "HASH\(0x\w+\)" as array index at - line 3.
+Use of reference "HASH\(0x\w+\)" as array index at - line 3.
+Use of reference "HASH\(0x\w+\)" as array index at - line 5.
+Use of reference "HASH\(0x\w+\)" as array index at - line 5. |
From @iabynOn Sat, Dec 13, 2014 at 06:01:58AM -0800, Father Chrysostomos via RT wrote:
Sorry, but I don't like it at all. Its a tricksy special-case (storing If someone has some code that does use constant FOO => ....; then personally I think warning about a strange value for FOO at compile -- |
From @tonycozOn Wed Dec 10 05:28:04 2014, davem wrote:
With that the original problem with the ticket is fixed, but B::Tap is still I suspect it's just a case that the tests need to be updated to deal with OP_MULTIDEREF to build a valid op-tree. Tony |
From @iabynOn Tue, Apr 07, 2015 at 08:43:03PM -0700, Tony Cook via RT wrote:
B::Tap 0.15 passes its tests. Is there any reason this ticket can't be -- |
From @rjbsNew B-Tap. Closed! -- |
|
@rjbs - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#123390 (status was 'resolved')
Searchable as RT123390$
The text was updated successfully, but these errors were encountered: