Skip to content

Commit

Permalink
S_newONCEOP/Perl_scalarvoid: skip over padops useless in void context
Browse files Browse the repository at this point in the history
Prior to this commit, padops associated with OP_ONCE might serve no
purpose in a void context. Since the context is not known at the point
of the OP_ONCE creation, the padops have retained the STATE flag, but
not the LVINTRO flag. This prevented Perl_scalarvoid from warning
"Useless use of private variable in void context".

In this commit, the STATE flag is removed and Perl_scalarvoid modified
such that when a OP_ONCE padop is found in void context, the op_next
pointer on the OP_ONCE is silently modified to skip over the padop.
  • Loading branch information
richardleach committed Nov 25, 2024
1 parent 2efb6d3 commit 1b81858
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 12 deletions.
8 changes: 2 additions & 6 deletions ext/B/t/optree_varinit.t
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,6 @@ checkOptree ( name => 'void context state',
# 3 <|> once(other->4)[$:2,3] vK/1
# 4 <$> const[IV 1] s
# 5 <1> padsv_store[$x:2,3] vKS/LVINTRO,STATE
# goto 6
# 8 <0> padsv[$x:2,3] v/STATE
# 6 <;> nextstate(main 3 -e:1) v:%,{,fea=15
# 7 <@> leave[1 ref] vKP/REFC
EOT_EOT
Expand All @@ -415,8 +413,6 @@ EOT_EOT
# 3 <|> once(other->4)[$:2,3] vK/1
# 4 <$> const(IV 1) s
# 5 <1> padsv_store[$x:2,3] vKS/LVINTRO,STATE
# goto 6
# 8 <0> padsv[$x:2,3] v/STATE
# 6 <;> nextstate(main 3 -e:1) v:%,{,fea=15
# 7 <@> leave[1 ref] vKP/REFC
EONT_EONT
Expand All @@ -433,7 +429,7 @@ checkOptree ( name => 'scalar context state',
# 5 <0> padsv[$x:2,3] sRM*/LVINTRO,STATE
# 6 <2> sassign sKS/2
# goto 7
# a <0> padsv[$x:2,3] s/STATE
# a <0> padsv[$x:2,3] s
# 7 <1> padsv_store[$y:2,3] vKS/LVINTRO
# 8 <;> nextstate(main 3 -e:1) v:%,{,fea=15
# 9 <@> leave[1 ref] vKP/REFC
Expand All @@ -445,7 +441,7 @@ EOT_EOT
# 5 <0> padsv[$x:2,3] sRM*/LVINTRO,STATE
# 6 <2> sassign sKS/2
# goto 7
# a <0> padsv[$x:2,3] s/STATE
# a <0> padsv[$x:2,3] s
# 7 <1> padsv_store[$y:2,3] vKS/LVINTRO
# 8 <;> nextstate(main 3 -e:1) v:%,{,fea=15
# 9 <@> leave[1 ref] vKP/REFC
Expand Down
20 changes: 15 additions & 5 deletions op.c
Original file line number Diff line number Diff line change
Expand Up @@ -2257,9 +2257,19 @@ Perl_scalarvoid(pTHX_ OP *arg)
case OP_ASLICE:
case OP_HELEM:
case OP_HSLICE:
if (!(o->op_private & (OPpLVAL_INTRO|OPpOUR_INTRO)))
/* Otherwise it's "Useless use of grep iterator" */
useless = OP_DESC(o);
if (!(o->op_private & (OPpLVAL_INTRO|OPpOUR_INTRO))) {
if ((op_parent(o)->op_type == OP_ONCE) &&
(op_parent(o)->op_next == o)
){
/* An already set "state" OP has been encounted
* and there's no point pushing anything to the
* stack in void context. */
op_parent(o)->op_next = o->op_next;
} else {
/* Otherwise it's "Useless use of grep iterator" */
useless = OP_DESC(o);
}
}
break;

case OP_SPLIT:
Expand Down Expand Up @@ -8597,8 +8607,8 @@ S_newONCEOP(pTHX_ OP *initop, OP *padop)
{
const PADOFFSET target = padop->op_targ;
OP *const nexop = newOP(padop->op_type,
(padop->op_flags & ~(OPf_REF|OPf_MOD|OPf_SPECIAL))
| ((padop->op_private & ~OPpLVAL_INTRO) << 8));
(padop->op_flags & ~(OPf_REF|OPf_MOD|OPf_SPECIAL|OPf_WANT))
| ((padop->op_private & ~(OPpLVAL_INTRO|OPpPAD_STATE)) << 8));
OP *const first = newOP(OP_NULL, 0);
OP *const nullop = newCONDOP(0, first, initop, nexop);
/* XXX targlex disabled for now; see ticket #124160
Expand Down
4 changes: 3 additions & 1 deletion t/op/state.t
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ BEGIN {

use strict;

plan tests => 164;
plan tests => 166;

# Before loading feature.pm, test it with CORE::
ok eval 'CORE::state $x = 1;', 'CORE::state outside of feature.pm scope';
Expand Down Expand Up @@ -509,10 +509,12 @@ for (1,2) {
sub gh_18630H {state %h=(a=>1)}
my $res = join '', gh_18630H, gh_18630H;
is($res, "a1a1", 'HASH copied successfully in subroutine exit');
is(scalar gh_18630H, 1, 'gh_18630H scalar call returns key count');

sub gh_18630A {state @a = qw(b 2)}
$res = join '', gh_18630A , gh_18630A;
is($res, "b2b2", 'ARRAY copied successfully in subroutine exit');
is(scalar gh_18630A, 2, 'gh_18630A scalar call returns element count');
}

__DATA__
Expand Down

0 comments on commit 1b81858

Please sign in to comment.