Add todo-test for GH 13307 (hash elem ref in @_ disappears)

[zb226]

This PR adds a todo-test for #13307 where a changed behavior involving @_ from perl 5.19.4 on was noted. Current blead perl still shows the same behavior.

---

• This set of changes does not require a perldelta entry.

[jkeenan]

Our Continuous Integration system has reported one failing test in your pull request. We need you to add your name (preferably something more than a github ID) and email address to AUTHORS. Please run this program from the top of your git checkout:

perl Porting/updateAUTHORS.pl 

When you're done make sure you call:

$ git add AUTHORS
$ git commit
$ make test_porting && make test_harness

... and then re-push your branch. Thanks.

[tonycoz]

A build with -Accflags=-DPERL_RC_STACK returns oooo for every line, which seems like the most reasonable result, even though that differs from what 5.18.1 produced.

blead default:

tony@venus:.../git/perl6$ ./perl -Ilib -V:ccflags
ccflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2';
tony@venus:.../git/perl6$ ./perl ../13307.pl 
undef
oooo
undef
oooo

blead with -Accflags=-DPERL_RC_STACK:

tony@venus:.../git/perl6$ ./perl -Ilib -V:ccflags
ccflags='-DPERL_RC_STACK -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2';
tony@venus:.../git/perl6$ ./perl ../13307.pl 
oooo
oooo
oooo
oooo

[zb226]

Hi, does that mean this issue will be handled elsewhere or should the test be added still?

[tonycoz]

After some thought, please make it check for the output I suggested and make the $::TODO conditional on "$Config{cc} $Config{ccflags} $Config{optimize}" !~ /-DPERL_RC_STACK\b/ - so it's a TODO test when -DPERL_RC_STACK is not found in those config variables.

[khwilliamson]

And that means we want this test to still be added; and thank you for contributing

[tonycoz]

I was thinking more:

my $is_rc_stack = "$Config{cc} $Config{ccflags} $Config{optimize}" =~ /-DPERL_RC_STACK\b/;
local $::TODO = $is_rc_stack ? undef : "GH 13307";

so we don't skip the test, but run the test non-TODO for PERL_RC_STACK and TODO for non-PERL_RC_STACK.

Also:

#   Failed test 'Is authors_file 'AUTHORS' up to date?'
#   at Porting/updateAUTHORS.pl line 130.
#   File 'AUTHORS' changes:
#     would add: zb226                          <zebster@spr.at>
# 
# Files need updating! You probably just need to run
# 
#    Porting/updateAUTHORS.pl
# 
# and commit the results.
# Looks like you failed 1 test of 5.
porting/authors.t ......... 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/5 subtests 

[zb226]

Thanks for the guidance (and patience). I've added my real name to the AUTHORS file, as that seems to be preferred. Since my commits have my github ID, it required a second run of updateAUTHORS.pl to add an entry in .mailmap to pass test_porting - I hope it's OK this way.

[zb226]

Is this looking OK?

[jkeenan]

I built from the branch in this pull request (rebased on blead) in 4 different ways and got slight, perhaps significant results in the output. (As I repeated the process, my commands got a bit more precisely focused.)

• GH 23661 "regular" build using gcc-13 on Ubuntu Linux 24.04 LTS ("regular" means simply -des -Dusedevel.)

$ cd t;./perl harness -v run/todo.t | ack '13307'; cd -
not ok 8 - Hashref element reference in @_ disappeared; GH 13307 # TODO GH 13307

• GH 23661 "regular" build using clang-18 on Ubuntu Linux 24.04 LTS

$ cd t;./perl harness -v run/todo.t | ack '(^Result:|13307)'; cd -
not ok 8 - Hashref element reference in @_ disappeared; GH 13307 # TODO GH 13307
# Failed test 8 - Hashref element reference in @_ disappeared; GH 13307 at run/todo.t line 228
Result: PASS

• GH 23661 "-DPERL_RC_STACK" build using gcc-13 on Ubuntu Linux 24.04 LTS

$ sh ./Configure -des -Dusedevel -Dcc=gcc -Accflags=-DPERL_RC_STACK && make test_prep
...
$ cd t;./perl harness -v run/todo.t | ack '(^Result:|13307)'; cd -
ok 8 - Hashref element reference in @_ disappeared; GH 13307
Result: PASS

• GH 23661 "-DPERL_RC_STACK" build using clang-18 on Ubuntu Linux 24.04 LTS

$ sh ./Configure -des -Dusedevel -Dcc=clang -Accflags=-DPERL_RC_STACK 1>/dev/null && make test_prep 1>/dev/null
pp_hot.c:3147:21: warning: variable 'i' set but not used [-Wunused-but-set-variable]
 3147 |             SSize_t i;
      |                     ^
1 warning generated.
Warning (mostly harmless): No library found for -lposix
Warning (mostly harmless): No library found for -lcposix
...
$ ok 8 - Hashref element reference in @_ disappeared; GH 13307
Result: PASS

Note the build-time warning in the 4th invocation. I spent several hours puzzling over this last night. AFAICT it is only emitted with a (recent?) clang build and only with -Accflags=-DPERL_RC_STACK. I tried to eliminate the warning with this combination of compiler and config options, but was unsuccessful, probably because I don't have much experience with the implications of #ifdef, #ifndef and #endif are for scoping of variables in C.

Note further that the way @tonycoz has suggested checking for -DPERL_RC_STACK differs from the only other instance of checking for that config option I could find underneath t/, that being in t/refcount.t:

 22 BEGIN {
 23     chdir 't' if -d 't';
 24     require './test.pl';
 25     skip_all('not built with PERL_RC_STACK')
 26         unless defined &Internals::stack_refcounted
 27             && (Internals::stack_refcounted() & 1);
 28     set_up_inc( qw(. ../lib) );
 29 }

[iabyn]

On Fri, Sep 05, 2025 at 07:00:55AM -0700, James E Keenan wrote: I built from the branch in this pull request (rebased on blead) in 4 different ways and got slight, perhaps significant results in the output. (As I repeated the process, my commands got a bit more precisely focused.)

IIUC, your results show that the PR is sound.

pp_hot.c:3147:21: warning: variable 'i' set but not used [-Wunused-but-set-variable]

This issue is unrelated to current ticket, but I've addressed it via PR #23689.

Note further that the way @tonycoz has suggested checking for `-DPERL_RC_STACK` differs from the only other instance of checking for that config option I could find underneath `t/`, that being in `t/refcount.t`: ``` 22 BEGIN { 23 chdir 't' if -d 't'; 24 require './test.pl'; 25 skip_all('not built with PERL_RC_STACK') 26 unless defined &Internals::stack_refcounted 27 && (Internals::stack_refcounted() & 1); 28 set_up_inc( qw(. ../lib) ); 29 }

Either way will work. The way in refcount.t is "my" way of doing it, which I think is more elegant.

…

-- The Enterprise successfully ferries an alien VIP from one place to another without serious incident. -- Things That Never Happen in "Star Trek" #7

[khwilliamson]

@zb226 before we can merge this, the commits need to be squashed down to 2: adding your name to the authors list; and everything else. git rebase -i is the way I do this myself. I can give you explicit instructions if you want.

[jkeenan]

@zb226 before we can merge this, the commits need to be squashed down to 2: adding your name to the authors list; and everything else. git rebase -i is the way I do this myself. I can give you explicit instructions if you want.

Yes, we'll need that squashing, since 3 of the commits on their own failed to pass our CI system (for whatever reason). Each commit on its own should PASS ./Configure, make and make test_harness.