Skip to content

UCD.pm: Add check for parameter type #23301

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

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

Conversation

khwilliamson
Copy link
Contributor

@khwilliamson khwilliamson commented May 18, 2025

This needs to be a scalar reference; I got burned with it not doing the right thing when it wasn't.

  • I'm unsure if this set of changes requires a perldelta entry

This needs to be a scalar reference; I got burned with it not doing the
right thing when it wasn't.
@khwilliamson khwilliamson added the defer-next-dev This PR should not be merged yet, but await the next development cycle label May 18, 2025
@tonycoz
Copy link
Contributor

tonycoz commented May 19, 2025

This could use a test.

  • I'm unsure if this set of changes requires a perldelta entry, and it is included.

I don't see the perldelta entry.

jkeenan added a commit to jkeenan/perl5 that referenced this pull request May 19, 2025
In response to Tony's review in GH Perl#23301
@jkeenan
Copy link
Contributor

jkeenan commented May 19, 2025

This could use a test.

Provided one at khwilliamson#1

In response to Tony's review in GH Perl#23301
@tonycoz
Copy link
Contributor

tonycoz commented May 19, 2025

The test made obvious to me:

tony@venus:.../git/perl6$ ./perl -Ilib -MUnicode::UCD=num -e 'my $x; num("123", \$x)'
tony@venus:.../git/perl6$ ./perl -Ilib -MUnicode::UCD=num -e 'my $x=\1; num("123", \$x)'
Unicode::UCD::num: second parameter must be a scalar reference at -e line 1.

In both cases the parameter is a reference to $x, and $$x can be assigned to.

Another case would be:

tony@venus:.../git/perl6$ ./perl -Ilib -MUnicode::UCD=num -E 'my $x; num("123", bless \$x);'
Unicode::UCD::num: second parameter must be a scalar reference at -e line 1.

But I think that's more a case of "Doctor, it hurts when I do (something dumb)"

Alas builtin::reftype() also returns REF for the second case.

I'm not fond of this ref() behaviour (ref() -> REF describes the contents of the reference container, not the referenced container itself)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defer-next-dev This PR should not be merged yet, but await the next development cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants