-
Notifications
You must be signed in to change notification settings - Fork 529
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
Fast exit undef/empty strings in Data::Dumper::qquote()
#22070
Fast exit undef/empty strings in Data::Dumper::qquote()
#22070
Conversation
fb08490
to
1d91efe
Compare
Can you provide an example of how you are getting these uninitialized warnings? Thanks. |
I don't see any such warnings in the build, and Data::Dumper itself should never call qquote() with undef that I can see. |
Sorry, I totally cheated and called it from outside... (
Anyway, I know it's undocumented, and I'm okay if it breaks, but I figured I'd push my (trivial) optimization back upstream, just in case you were interested. «shrug» Cheers! :-D |
On Mon, 11 Mar 2024 at 17:37, DabeDotCom ***@***.***> wrote:
Can you provide an example of how you are getting these uninitialized
warnings? Thanks.
Sorry, I totally cheated and called it from outside... (Text::Quote
<https://metacpan.org/pod/Text::Quote> mentions taking "inspiration" from
Data::Dump, but I figured since Data::Dumper was a core module, I'd just
leverage *it* instead.)
prompt% perl -MData::Dumper -E 'say Data::Dumper::qquote()'
Use of uninitialized value $_ in substitution (s///) at /System/Library/Perl/5.34/darwin-thread-multi-2level/Data/Dumper.pm line 764.
Use of uninitialized value in numeric gt (>) at /System/Library/Perl/5.34/darwin-thread-multi-2level/Data/Dumper.pm line 775.
Use of uninitialized value $bytes in numeric gt (>) at /System/Library/Perl/5.34/darwin-thread-multi-2level/Data/Dumper.pm line 775.
Use of uninitialized value $_ in pattern match (m//) at /System/Library/Perl/5.34/darwin-thread-multi-2level/Data/Dumper.pm line 783.
Use of uninitialized value $_ in concatenation (.) or string at /System/Library/Perl/5.34/darwin-thread-multi-2level/Data/Dumper.pm line 783.
""
Anyway, I know it's undocumented, and I'm okay if it breaks, but I figured
I'd push my (trivial) optimization back upstream, just in case you were
interested. «shrug»
Cheers! :-D
IMO there is nothing wrong with hardening DD::qquote() to undef input. It
may technically be undocumented, in practice ive seen it used many times.
Cheers
Yves
…--
perl -Mre=debug -e "/just|another|perl|hacker/"
|
I think it could use a test or two (simple non-empty string, empty string, undef), otherwise it's good. |
@DabeDotCom, could you provide some unit tests for this p.r., as per @tonycoz's request? Thanks. |
c9ba999
to
d9e8c70
Compare
dist/Data-Dumper/t/qquote.t
Outdated
@@ -0,0 +1,80 @@ | |||
#!./perl -w | |||
# t/seen.t - Test Seen() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
old comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good otherwise
ade6b45
to
1838c7e
Compare
This silences a squajillion (okay, five) "uninitialized" warnings, and is more efficient, anyway...
(I thought that's why we didn't like Python...)
1838c7e
to
420ca2e
Compare
While I am in general in support of this ticket, I have to admit i find it
quite strange that qquote() would return the empty string for an undef
argument. It should return the string "undef" instead.
Sorry I missed this earlier. Is there a rationale for why it lies about its
argument?
Yves
…On Thu, 20 Jun 2024 at 20:04, Karl Williamson ***@***.***> wrote:
Merged #22070 <#22070> into blead.
—
Reply to this email directly, view it on GitHub
<#22070 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAZ5RYGGUHG47Z4WN3UPL3ZIMKUFAVCNFSM6AAAAABENP2CFSVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJTGIZTENZUHE4DOOI>
.
You are receiving this because you commented.Message ID:
***@***.***>
--
perl -Mre=debug -e "/just|another|perl|hacker/"
|
This silences a squajillion (okay, five) "uninitialized" warnings, and is more efficient, anyway...