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

Add tests for lvalue subs returning vec() or substr() and called twice in one expression #22386

Merged
merged 2 commits into from
Jul 23, 2024

Conversation

t-a-k
Copy link
Contributor

@t-a-k t-a-k commented Jul 9, 2024

I found that Perl prior to v5.20 (at least v5.16.3 bundled in CentOS Linux 7 and locally-built v5.18.4) fail to return correct value from subroutine returning vec() and substr() as lvalue but called multiple times in rvalue context of one expression:

$ perl -wle 'my $x = "\002"; sub myvec1 :lvalue { vec($x, $_[0], 1) }' \
    -e 'print myvec1(0); print myvec1(1); print myvec1(0) + myvec1(1)'
0
1
2
$

This apparently had been fixed in the commit 169504d (in 2013), but testcases for these symptoms doesn't seem to exist so I'd like to add ones to prevent accidental regression.

…) and called twice in one expression

Perl prior to v5.20 (at least v5.16.3, v5.18.4) fail to return correct
value from subroutine returning vec() and substr() as lvalue but called
multiple times in rvalue context of one expression:

    $ perl -wle 'my $x = "\002"; sub myvec1 :lvalue { vec($x, $_[0], 1) }' \
        -e 'print myvec1(0); print myvec1(1); print myvec1(0) + myvec1(1)'
    0
    1
    2
    $

This apparently had been fixed in the commit
169504d (in 2013), but testcases
for these symptoms doesn't seem to exist.
t/op/sub_lval.t Outdated Show resolved Hide resolved
Copy link
Contributor

@jkeenan jkeenan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I extracted the proposed changes to a program outside my checkout, then ran that program against perl-5.18.4 and perl-5.20.0, respectively.

$ perlbrew use perl-5.18.4
$ prove -v gh-22386.t 
gh-22386.t .. 
1..2
not ok 1 - wrap_vec
not ok 2 - wrap_substr

#   Failed test 'wrap_vec'
#   at gh-22386.t line 16.
#          got: 10
#     expected: 8

#   Failed test 'wrap_substr'
#   at gh-22386.t line 26.
#          got: 'kk'
#     expected: 'ok'
# Looks like you failed 2 tests of 2.
Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/2 subtests 

Test Summary Report
-------------------
gh-22386.t (Wstat: 512 Tests: 2 Failed: 2)
  Failed tests:  1-2
  Non-zero exit status: 2
Files=1, Tests=2,  0 wallclock secs ( 0.01 usr +  0.00 sys =  0.01 CPU)
Result: FAIL

$ perlbrew use perl-5.20.0
$ prove -v gh-22386.t 
gh-22386.t .. 
1..2
ok 1 - wrap_vec
ok 2 - wrap_substr
ok
All tests successful.
Files=1, Tests=2,  0 wallclock secs ( 0.01 usr +  0.00 sys =  0.01 CPU)
Result: PASS

So I think that once the grammar in the inline comment is corrected, this will be good to go into blead.

Copy link
Contributor

@tonycoz tonycoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use grammer to grammar in the second commit message (which a squash would likely fix)

Thanks to @jkeenan for pointing this out.
@t-a-k t-a-k force-pushed the add-lvalue-sub-tests branch from 853a6a9 to 6af24cc Compare July 17, 2024 15:10
@t-a-k
Copy link
Contributor Author

t-a-k commented Jul 17, 2024

Could use grammer to grammar in the second commit message (which a squash would likely fix)

Sorry for the typo. I've force-pushed the second commit.

@khwilliamson khwilliamson merged commit 14c6cf2 into Perl:blead Jul 23, 2024
33 checks passed
@t-a-k t-a-k deleted the add-lvalue-sub-tests branch August 2, 2024 13:19
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.

4 participants