From 1b81858b3f393b37f2ee28aa7caaa5ee380894a4 Mon Sep 17 00:00:00 2001 From: Richard Leach Date: Mon, 18 Nov 2024 23:26:16 +0000 Subject: [PATCH] S_newONCEOP/Perl_scalarvoid: skip over padops useless in void context 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. --- ext/B/t/optree_varinit.t | 8 ++------ op.c | 20 +++++++++++++++----- t/op/state.t | 4 +++- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/ext/B/t/optree_varinit.t b/ext/B/t/optree_varinit.t index 0d0dc72e435d..25ab49452c17 100644 --- a/ext/B/t/optree_varinit.t +++ b/ext/B/t/optree_varinit.t @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/op.c b/op.c index 996d3439da07..94ccc3080c6b 100644 --- a/op.c +++ b/op.c @@ -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: @@ -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 diff --git a/t/op/state.t b/t/op/state.t index 99b4a49b65bf..1c0b0a81fecd 100644 --- a/t/op/state.t +++ b/t/op/state.t @@ -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'; @@ -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__