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
richardleach committed Nov 21, 2024
1 parent ccd591c commit 5835483
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 111 deletions.
8 changes: 2 additions & 6 deletions ext/B/t/optree_varinit.t
View file
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
View file
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
108 changes: 9 additions & 99 deletions pod/perldelta.pod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
[ this is a template for a new perldelta file. Any text flagged as XXX needs
to be processed before release. ]

perldelta - what is new for perl v5.41.6
perldelta - what is new for perl v5.41.7

=head1 DESCRIPTION

This document describes differences between the 5.41.5 release and the 5.41.6
This document describes differences between the 5.41.6 release and the 5.41.7
release.

If you are upgrading from an earlier release such as 5.41.4, first read
L<perl5415delta>, which describes differences between 5.41.4 and 5.41.5.
If you are upgrading from an earlier release such as 5.41.5, first read
L<perl5416delta>, which describes differences between 5.41.5 and 5.41.6.

=head1 Notice

Expand Down Expand Up @@ -91,23 +91,6 @@ There may well be none in a stable release.

XXX

=item *

Code that uses the C<indexed> function from the L<builtin> module to generate
a list of index/value pairs out of array which is then passed into a
two-variable C<foreach> list to unpack those again is now optimised to be more
efficient.

my @array = (...);

foreach my ($idx, $val) (builtin::indexed @array) {
...
}

In particular, a temporary list twice the size of the original
array is no longer generated. Instead, the list iterates down the array
in-place directly, in the same way that C<foreach (@array)> would do.

=back

=head1 Modules and Pragmata
Expand Down Expand Up @@ -146,31 +129,6 @@ L<XXX> has been upgraded from version A.xx to B.yy.

XXX If there was something important to note about this change, include that here.

=item *

L<IPC::Open3> has been upgraded from version 1.23 to 1.24.
L<IPC::Open2> has been upgraded from version 1.07 to 1.08.

There is now a way to pass lexical filehandles to child processes directly
(instead of having the module create a pipe internally). Previously, only
bareword filehandles could be used in "dup mode".

=item *

L<Math::Complex> has been upgraded from version 1.62 to 1.63.

The complex number parser for string inputs has been improved. In particular,
C<1+i>, C<123i>, C<inf>, C<-inf>, C<infi>, and C<-infi> are now handled
correctly.

=item *

L<B::Deparse> has been upgraded from version 1.78 to 1.79.

It now retains parens around logical negations on the left-hand side of
comparisons (such as C<(!$x) == $y>) because omitting them would trigger a
C<Possible precedence problem> warning since perl v5.41.4. [GH #22661]

=back

=head2 Removed Modules and Pragmata
Expand Down Expand Up @@ -372,19 +330,9 @@ L</Modules and Pragmata> section.

=over 4

=item arm64 DARWIN

Fix arm64 darwin hints when using use64bitall with Configure [GH #22672]

=item Android

Changes to perl_langinfo.h for Android [GH #22650] related to [GH #22627]

=item Cygwin

cygwin.c: fix several silly/terrible C errors [GH #22724]
=item XXX-some-platform

workaround DLL load address conflict [GH #22696]
XXX

=back

Expand All @@ -400,16 +348,7 @@ well.

=item *

Enable removing most of mathoms.c and stub functions [GH #22691] and [GH #22714]

=item *

reinstate apostrophe as package separator behind a default enabled feature,
which is disabled from feature bundle 5.41.

=item *

pp_reverse: don't COW buffer just to then un-COW it [GH #22729]
XXX

=back

Expand All @@ -427,20 +366,6 @@ manager will later use a regex to expand these into links.

=item *

The perl parser would erroneously parse like C<=cut> some other POD directives
whose names start with I<cut>, prematurely terminating an embedded POD section.
The following cases were affected: I<cut> followed by a digit (e.g.
C<=cut2studio>), I<cut> followed by an underscore (e.g. C<=cut_grass>), and in
string C<eval>, any identifier starting with I<cut> (e.g. C<=cute>).
[GH #22759]

=item *

Builds with C<-msse> and quadmath on 32-bit x86 systems would crash
with a misaligned access early in the build. [GH #22577]

=item *

Declaring a lexically scoped array or hash using C<state> within a subroutine
and then immediately returning no longer triggers a "Bizarre copy of HASH/ARRAY
in subroutine exit" error. [GH #18630]
Expand All @@ -459,22 +384,7 @@ platform specific bugs also go here.

=item *

C<cygwin> builds may fail during testing due to a conflict between the
load addresses of F<cygperl5_41_6.dll> and
F<olib/auto/I18N/Langinfo/Langinfo.dll>. This will also be visible
for anything that attempts to fork() with C<I18N::LangInfo> loaded.

This is known to fail for builds with options that increase the size
of the binary, such as C<-DDEBUGGING>, C<-Doptimize="-O0 -g"> or
C<-Doptimize="-O2 -g -march=x86-64-v2">.

This can be avoided by building perl with
C<-Astatic_ext=I18N/Langinfo>.

The base addresses are generated by the linker based on the names of
the DLLs, so this is expected to clear up for 5.41.7.

[github #22695]
XXX

=back

Expand All @@ -498,7 +408,7 @@ died, add a short obituary here.

XXX Generate this with:

perl Porting/acknowledgements.pl v5.41.5..HEAD
perl Porting/acknowledgements.pl v5.41.6..HEAD

=head1 Reporting Bugs

Expand Down
4 changes: 3 additions & 1 deletion t/op/state.t
View file
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 5835483

Please sign in to comment.