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
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: