perl5 icon indicating copy to clipboard operation
perl5 copied to clipboard

Fast exit undef/empty strings in `Data::Dumper::qquote()`

Open DabeDotCom opened this issue 1 year ago • 5 comments

This silences a squajillion (okay, five) "uninitialized" warnings, and is more efficient, anyway...

DabeDotCom avatar Mar 08 '24 22:03 DabeDotCom

This silences a squajillion (okay, five) "uninitialized" warnings, and is more efficient, anyway...

Can you provide an example of how you are getting these uninitialized warnings? Thanks.

jkeenan avatar Mar 08 '24 23:03 jkeenan

I don't see any such warnings in the build, and Data::Dumper itself should never call qquote() with undef that I can see.

tonycoz avatar Mar 11 '24 03:03 tonycoz

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 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

DabeDotCom avatar Mar 11 '24 16:03 DabeDotCom

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/"

demerphq avatar Mar 11 '24 16:03 demerphq

I think it could use a test or two (simple non-empty string, empty string, undef), otherwise it's good.

tonycoz avatar Mar 11 '24 23:03 tonycoz

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.

jkeenan avatar Jun 13 '24 13:06 jkeenan

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 https://github.com/Perl/perl5/pull/22070 into blead.

— Reply to this email directly, view it on GitHub https://github.com/Perl/perl5/pull/22070#event-13232749879, 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/"

demerphq avatar Jun 21 '24 09:06 demerphq

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?

FTR — That's what it did before, and I didn't want to risk breaking anything... :-D

(Perhaps because it's "quoting" undef, which — it could be argued — is equivalent to an empty string? «shrug»)

DabeDotCom avatar Jun 26 '24 16:06 DabeDotCom