%tiedhash in bool context doesn't check if hash is empty

[p5pRT]

Migrated from rt.perl.org#17718 (status was 'resolved')

Searchable as RT17718$

[p5pRT]

From @ysth

Created by @ysth

Checking for hash emptiness with code like C\< if (%x) > fails if %x is tied.

~ $perl -MTie​::Hash -wle'tie %x\,"Tie​::StdHash"; %x=1..2; print(%x?"ok"​:"not ok")' not ok

This comes from this code in pp_padhv and pp_rv2hv​: ~/bleadperl/perl $fgrep -A2 HvFILL pp*.c pp.c​: if (HvFILL((HV*)TARG)) pp.c- Perl_sv_setpvf(aTHX_ sv\, "%ld/%ld"\, pp.c​: (long)HvFILL((HV*)TARG)\, (long)HvMAX((HV*)TARG) + 1); pp.c- else pp.c- sv_setiv(sv\, 0); -- pp_hot.c​: if (HvFILL(hv)) pp_hot.c- Perl_sv_setpvf(aTHX_ TARG\, "%"IVdf"/%"IVdf\, pp_hot.c​: (IV)HvFILL(hv)\, (IV)HvMAX(hv) + 1); pp_hot.c- else pp_hot.c- sv_setiv(TARG\, 0);

where HvFILL doesn't seem to reflect whether the tied hash has any contents. There isn't any way to return valid bucket info if the hash is not empty--perhaps it should return "1/1"? But if the hash is empty it needs to return a false value regardless of HvFILL.

Perl Info

``` Flags: category=core severity=low Site configuration information for perl v5.8.0: Configured by sthoenna at Sun Aug 4 15:00:09 2002. Summary of my perl5 (revision 5.0 version 8 subversion 0) configuration: Platform: osname=cygwin, osvers=1.3.10(0.5132), archname=cygwin-64int uname='cygwin_98-4.10 yo 1.3.10(0.5132) 2002-02-25 11:14 i686 unknown ' config_args='-de -Dusedevel -Duse64bitint -Ui_ndbm -Ui_dbm -Dstatic_ext='IO Socket' -Doptimize=-g' hint=recommended, useposix=true, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=define use64bitall=undef uselongdouble=undef usemymalloc=y, bincompat5005=undef Compiler: cc='gcc', ccflags ='-DPERL_USE_SAFE_PUTENV -DDEBUGGING -fno-strict-aliasing -I/usr/local/include', optimize='-g', cppflags='-DPERL_USE_SAFE_PUTENV -DDEBUGGING -fno-strict-aliasing -I/usr/local/include' ccversion='', gccversion='2.95.3-5 (cygwin special)', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 ivtype='long long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=4 alignbytes=8, prototype=define Linker and Libraries: ld='ld2', ldflags =' -L/usr/local/lib' libpth=/usr/local/lib /usr/lib /lib libs=-lgdbm -lcrypt -lutil perllibs=-lcrypt -lutil libc=/usr/lib/libc.a, so=dll, useshrplib=true, libperl=libperl.a gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags=' -L/usr/local/lib' Locally applied patches: @INC for perl v5.8.0: /usr/local/lib/perl5/5.8.0/cygwin-64int /usr/local/lib/perl5/5.8.0 /usr/local/lib/perl5/site_perl/5.8.0/cygwin-64int /usr/local/lib/perl5/site_perl/5.8.0 /usr/local/lib/perl5/site_perl . Environment for perl v5.8.0: HOME=/cygdrive/c/home/sthoenna LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/local/bin:/usr/bin:/bin:/cygdrive/c/home/sthoenna/bin:/cygdrive/c/PROGRA~1/YARNW:/usr/bin:/USR/LOCAL/EMACS/EMACS-20.7/BIN:/cygdrive/c/WINDOWS:/cygdrive/c/WINDOWS/COMMAND PERL_BADLANG (unset) SHELL (unset) ```

[p5pRT]

From @jhi

"Ticket #17718 %tiedhash in bool context doesn't check if hash is empty"

http​://rt.perl.org/rt2/Ticket/Display.html?id=17718

-- Jarkko Hietaniemi \jhi@&#8203;iki\.fi http​://www.iki.fi/jhi/ "There is this special biologist word we use for 'stable'. It is 'dead'." -- Jack Cohen

[p5pRT]

From @ysth

On 5 May 2003 19​:17​:21 -0000\, perlbug-followup@​perl.org wrote​:

"Ticket #17718 %tiedhash in bool context doesn't check if hash is empty"

http​://rt.perl.org/rt2/Ticket/Display.html?id=17718

Not so low hanging. See the unanswered questions under [perl #18186].

It would be easy enough to call mg_size when formatting the bucket info and make magic_sizepack (currently never used for hashes) call FETCHSIZE only for arrays and BUCKETS for hashes\, but that might get in the way of a future HASHSIZE (or FETCHSIZE??) method for efficient scalar(keys(%tiedhash)). I guess magic_sizepack could could check if the current op was pp_keys or not...

Any comments\, anybody?

[p5pRT]

From whatever@davidnicol.com

On Mon\, 2003-05-05 at 21​:06\, Yitzchak Scott-Thoennes wrote​:

It would be easy enough to call mg_size when formatting the bucket info and make magic_sizepack (currently never used for hashes) call FETCHSIZE only for arrays and BUCKETS for hashes\, but that might get in the way of a future HASHSIZE (or FETCHSIZE??) method for efficient scalar(keys(%tiedhash)). I guess magic_sizepack could could check if the current op was pp_keys or not...

Any comments\, anybody?

it appears to this chronic suggestor of tie extensions that the whole exercise is a waste of time unless one is willing to implement your extension\, not just to suggest how cool it would be.

That's my comment\, (and I have been up all night at this point.)

-- David Nicol\, independent consultant and contractor have a nice day\, really. http​://www.funnytimes.com

[p5pRT]

From @ysth

On 2 Oct 2002 02​:21​:30 -0000\, perl5-porters@​perl.org wrote​:

Checking for hash emptiness with code like C\< if (%x) > fails if %x is tied.

~ $perl -MTie​::Hash -wle'tie %x\,"Tie​::StdHash"; %x=1..2; print(%x?"ok"​:"not ok")' not ok

There is no way to check a tied hash for emptiness without affecting the iterator\, so I'm going to suggest that it croak for now (instead of returning bad data) and we can add an experimental BUCKETS (or some such) in the devel branch later.

Note that scalar(%tiedhash) was used erroneously in op/magic.t (though the value was never actually used.)

Inline Patch

```diff --- perl/pp_hot.c.orig Thu May 1 05:26:22 2003 +++ perl/pp_hot.c Tue May 6 14:35:10 2003 @@ -795,6 +795,7 @@ { dSP; dTOPss; HV *hv; + I32 gimme = GIMME_V; if (SvROK(sv)) { wasref: @@ -808,7 +809,7 @@ RETURN; } else if (LVRET) { - if (GIMME == G_SCALAR) + if (gimme != G_ARRAY) Perl_croak(aTHX_ "Can't return hash to lvalue scalar context"); SETs((SV*)hv); RETURN; @@ -825,7 +826,7 @@ RETURN; } else if (LVRET) { - if (GIMME == G_SCALAR) + if (gimme != G_ARRAY) Perl_croak(aTHX_ "Can't return hash to lvalue" " scalar context"); SETs((SV*)hv); @@ -850,7 +851,7 @@ DIE(aTHX_ PL_no_usym, "a HASH"); if (ckWARN(WARN_UNINITIALIZED)) report_uninit(); - if (GIMME == G_ARRAY) { + if (gimme == G_ARRAY) { SP--; RETURN; } @@ -885,7 +886,7 @@ RETURN; } else if (LVRET) { - if (GIMME == G_SCALAR) + if (gimme != G_ARRAY) Perl_croak(aTHX_ "Can't return hash to lvalue" " scalar context"); SETs((SV*)hv); @@ -894,12 +895,15 @@ } } - if (GIMME == G_ARRAY) { /* array wanted */ + if (gimme == G_ARRAY) { /* array wanted */ *PL_stack_sp = (SV*)hv; return do_kv(); } - else { + else if (gimme == G_SCALAR) { dTARGET; + if (SvRMAGICAL(hv) && mg_find((SV *)hv, PERL_MAGIC_tied)) + Perl_croak(aTHX_ "Can't provide tied hash usage; " + "use keys(%%hash) to test if empty"); if (HvFILL(hv)) Perl_sv_setpvf(aTHX_ TARG, "%"IVdf"/%"IVdf, (IV)HvFILL(hv), (IV)HvMAX(hv) + 1); @@ -907,8 +911,8 @@ sv_setiv(TARG, 0); SETTARG; - RETURN; } + RETURN; } STATIC void --- perl/pp.c.orig Wed Apr 16 14:25:38 2003 +++ perl/pp.c Tue May 6 14:38:42 2003 @@ -107,6 +107,9 @@ } else if (gimme == G_SCALAR) { SV* sv = sv_newmortal(); + if (SvRMAGICAL(TARG) && mg_find(TARG, PERL_MAGIC_tied)) + Perl_croak(aTHX_ "Can't provide tied hash usage; " + "use keys(%%hash) to test if empty"); if (HvFILL((HV*)TARG)) Perl_sv_setpvf(aTHX_ sv, "%ld/%ld", (long)HvFILL((HV*)TARG), (long)HvMAX((HV*)TARG) + 1); --- perl/pod/perldata.pod.orig Sat Apr 26 17:07:08 2003 +++ perl/pod/perldata.pod Tue May 6 14:32:14 2003 @@ -257,7 +257,9 @@ set. For example, you stick 10,000 things in a hash, but evaluating %HASH in scalar context reveals C<"1/16">, which means only one out of sixteen buckets has been touched, and presumably contains all -10,000 of your items. This isn't supposed to happen. +10,000 of your items. This isn't supposed to happen. If a tied hash +is evaluated in scalar context, a fatal error will result, since this +bucket usage information is currently not available for tied hashes. You can preallocate space for a hash by assigning to the keys() function. This rounds up the allocated buckets to the next power of two: --- perl/pod/perldiag.pod.orig Sun Apr 27 00:17:16 2003 +++ perl/pod/perldiag.pod Tue May 6 14:38:22 2003 @@ -918,6 +918,14 @@ (F) The script you specified can't be opened for the indicated reason. +=item Can't provide tied hash usage; use keys(%hash) to test if empty + +(F) When a hash is evaluated in scalar context, bucket usage is +returned if the hash is populated, and false is returned if the hash +is empty. Buckage usage is not currently available for tied hashes. +To test if a hash is empty or populated, use keys(%hash) in scalar +context instead. + =item Can't read CRTL environ (S) A warning peculiar to VMS. Perl tried to read an element of %ENV --- perl/pod/perltie.pod.orig Tue Nov 26 13:53:46 2002 +++ perl/pod/perltie.pod Tue May 6 14:58:00 2003 @@ -1076,6 +1076,14 @@ Tied filehandles are still incomplete. sysopen(), truncate(), flock(), fcntl(), stat() and -X can't currently be trapped. +The bucket usage information provided by C is not +available. If C<%hash> is tied, this will currently result in a +fatal error. + +Counting the number of entries in a hash via C or +C) is inefficient since it needs to iterate +through all the entries with FIRSTKEY/NEXTKEY. + =head1 AUTHOR Tom Christiansen --- perl/t/op/magic.t.orig Mon Mar 10 22:31:54 2003 +++ perl/t/op/magic.t Wed May 7 00:27:50 2003 @@ -316,16 +316,20 @@ skip('no caseless %ENV support') for 1..4; } +{ + no warnings 'void'; + # Make sure Errno hasn't been prematurely autoloaded -ok !defined %Errno::; + ok !defined %Errno::; # Test auto-loading of Errno when %! is used -ok scalar eval q{ - my $errs = %!; - defined %Errno::; -}, $@; + ok scalar eval q{ + %!; + defined %Errno::; + }, $@; +} # Make sure that Errno loading doesn't clobber $! ```

[p5pRT]

From @jhi

Thanks\, applied (change #19452). Even though this introduces a new croak\, I might even take it to 5.8.1\, since the croak stops people from using a construct that doesn't do what they think it is doing.

-- Jarkko Hietaniemi \jhi@&#8203;iki\.fi http​://www.iki.fi/jhi/ "There is this special biologist word we use for 'stable'. It is 'dead'." -- Jack Cohen

[p5pRT]

@cwest - Status changed from 'new' to 'resolved'