Skip to content
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

S_newONCEOP: Rejig the op_next padop (Closes #18630) #22764

Open
wants to merge 2 commits into
base: blead
Choose a base branch
from

Conversation

richardleach
Copy link
Contributor

Two related changes pertaining to the padop executed when an OP_ONCE is encountered for the nth time.

  • S_newONCEOP: correctly copy array/hash at subroutine exit
    Should a state array or hash be declared immediately prior to
    returning, the error Bizarre copy of HASH in subroutine exit error
    would be triggered. Now, the array or hash is returned without error.
    Closes Returning state hash-assign risks "Bizarre copy of HASH in subroutine exit" #18630

  • S_newONCEOP/Perl_scalarvoid: skip over padops uselss in void context
    Rejigs how the nth-time padop is handled in void context to avoid the
    warning "Useless use of private variable in void context". The OPf_STATE
    flag is removed and the op_next chain now explicitly skips the padop.


  • This set of changes requires a perldelta entry, and it is included.

@tonycoz
Copy link
Contributor

tonycoz commented Nov 21, 2024

An array in scalar context is normally the number of elements in the array, but that doesn't appear to be the case for the trailing state @foo:

$ ./perl -Ilib -E 'sub f { state @x = @_; } say f(3,2,1); say scalar f()'
321
1
$ ./perl -Ilib -E 'sub f { state @x = @_; } say scalar f(4,2,1); say scalar f()'
3
1

@richardleach
Copy link
Contributor Author

Thanks, I'll look into it.

I see that in:

./perl -Ilib -E 'sub f { state @x = @_; @x; } say f(3,2,1); say scalar f()'
321
3
$ perl -E 'sub f { my @tto =(3,2,1) } say f(3,2,1); say scalar f()'
321
3
$ perl -E 'sub f { my @tto =(3,2,1); @tto } say f(3,2,1); say scalar f()'
321
3

Though not in:

$ ./perl -Ilib -E 'sub f { (3,2,1) } say f(3,2,1); say scalar f()'
321
1

@richardleach
Copy link
Contributor Author

Looks like it comes down to clobbering (or not) OPf_WANT. I'll update the PR shortly.

@richardleach richardleach force-pushed the hydahy/18630_state branch 2 times, most recently from d88bcce to 5835483 Compare November 21, 2024 10:06
Should a _state_ array or hash be declared immediately prior to
returning, the error "Bizarre copy of HASH in subroutine exit" error
would be triggered. Now, the array or hash is returned without error.

Specifically:
 * Instead of always creating a PADSV, the appropriate padop is created.
 * The OPf_REF flag is switched off, allowing the return of the
   container variable's contents rather than the container SV itself.
 * The OPf_MOD & OPf_SPECIAL flags are switches off, as they are not
   relevant to this version of the padop.

Note: The op created has been renamed from "other" to "nextop", since
it is linked to the ONCE op via op_next, not op_other.

Fixes GH#18630
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.
@richardleach
Copy link
Contributor Author

Ok, I think it has the correct behaviour now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Returning state hash-assign risks "Bizarre copy of HASH in subroutine exit"
2 participants