exists() on tied hash generates call to FETCH

[p5pRT]

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

Searchable as RT1189$

[p5pRT]

From @mjdominus

If %h is a tied hash, then

  exists $h{key}

calls the tied EXISTS method and then the tied FETCH method.
It should not call FETCH.

Sample​:

---------------- BEGIN SAMPLE PROGRAM ------------

tie %h => T;
$z = exists $h{'q'};
exit 0;

package T;

sub TIEHASH {
  my $self = {};
  bless $self => T;
}

sub EXISTS { print "EX\n"; return }

sub FETCH { print "FETCH\n"; return }

---------------- END SAMPLE PROGRAM ------------

This program prints out

  EX
  FETCH

even though no fetch should have been done.

This problem occurs under at least the following​: 5.005_02, _56, and _57.

Perl Info

This perlbug was built using Perl 5.00502 - Sun Oct 18 04:25:09 CDT 1998
It is being executed now by  Perl 5.00556 - Wed May 19 22:26:34 EDT 1999.

Site configuration information for perl 5.00502:

Configured by root at Sun Oct 18 04:25:09 CDT 1998.

Summary of my perl5 (5.0 patchlevel 5 subversion 2) configuration:
  Platform:
    osname=linux, osvers=2.0.35, archname=i586-linux
    uname='linux darkstar 2.0.35 #10 tue oct 13 18:04:13 cdt 1998 i586 unknown '
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=undef useperlio=undef d_sfio=undef
  Compiler:
    cc='cc', optimize='-O2', gccversion=2.7.2.3
    cppflags='-Dbool=char -DHAS_BOOL -I/usr/local/include'
    ccflags ='-Dbool=char -DHAS_BOOL -I/usr/local/include'
    stdchar='char', d_stdstdio=define, usevfork=false
    intsize=4, longsize=4, ptrsize=4, doublesize=8
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    alignbytes=4, usemymalloc=n, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -L/usr/local/lib'
    libpth=/usr/local/lib /shlib /lib /usr/lib
    libs=-lndbm -lgdbm -ldbm -ldb -ldl -lm -lc
    libc=/lib/libc.so.5.4.46, so=so, useshrplib=false, libperl=libperl.a
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic'
    cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:
    


@INC for perl 5.00502:
    /usr/lib/perl5/5.00502/i586-linux
    /usr/lib/perl5/5.00502
    /usr/lib/perl5/site_perl/5.005/i586-linux
    /usr/lib/perl5/site_perl/5.005
    .


Environment for perl 5.00502:
    HOME=/home/mjd
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH=/lib:/usr/lib:/usr/X11R6/lib
    LOGDIR (unset)
    PATH=/home/mjd/bin:/usr/local/bin:/bin:/usr/bin:/usr/X11/bin:/usr/games:/sbin:/usr/sbin:/usr/local/bin/X11:/usr/local/bin/mh:/data/mysql/bin:/home/mjd/TPI/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

[p5pRT]

From [Unknown Contact. See original ticket]

On Sun, 08 Aug 1999 at 13​:19​:01 -0000, mjd@​plover.com wrote​:

This is a bug report for perl from mjd@​plover.com,
generated with the help of perlbug 1.26 running under perl 5.00502.

If %h is a tied hash, then

exists $h\{key\}

calls the tied EXISTS method and then the tied FETCH method.
It should not call FETCH.

sub EXISTS { print "EX\n"; return }

This appears only to be true if EXISTS returns 'undef', as your example
does via its unqualified 'return'. If it returns true (1) or false (''),
no call to FETCH is made.

I construe this as a feature; EXISTS returning 'undef' means "don't
know" and the tie tries again with FETCH. I didn't find the code that
does this, though, and it's not documented...

Ian

[p5pRT]

From @mjdominus

If %h is a tied hash, then

exists $h\{key\}

calls the tied EXISTS method and then the tied FETCH method.
It should not call FETCH.

sub EXISTS { print "EX\n"; return }

This appears only to be true if EXISTS returns 'undef', as your example
does via its unqualified 'return'.

Ho! Thank you!

I construe this as a feature; EXISTS returning 'undef' means "don't know"

If so, it is a mighty weird feature, because it is the only place in
Perl where a boolean function can return undef and have it mean
anything *other* than false. That is why it did not occur to me to
try `return 0' or `return ""'.

I also don't see it as a very useful feature. Suppose the EXISTS
method doesn't know, and needs to consult FETCH to find out. It is
much more straightforward, and just as effective, for EXISTS to call
FETCH directly than to have it rely on bizarre magic like this.

Thanks again for telling me how to fix this.

[p5pRT]

From @mjdominus

Created by @mjdominus

If %h is a tied hash, then

  exists $h{key}

calls the tied EXISTS method and then the tied FETCH method.
It should not call FETCH.

Sample​:

---------------- BEGIN SAMPLE PROGRAM ------------

tie %h => T;
$z = exists $h{'q'};
exit 0;

package T;

sub TIEHASH {
  my $self = {};
  bless $self => T;
}

sub EXISTS { print "EX\n"; return }

sub FETCH { print "FETCH\n"; return }

---------------- END SAMPLE PROGRAM ------------

This program prints out

  EX
  FETCH

even though no fetch should have been done.

This problem occurs under at least the following​: 5.005_02, _56, and
_57, 5.6.0.

Ian Phillipps points out that this occurs only when EXISTS returns
undef; if it returns 0 or "" then the FETCH method is not called.

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl v5.6.0:

Configured by mjd at Wed Sep 20 15:34:29 EDT 2000.

Summary of my perl5 (revision 5.0 version 6 subversion 0) configuration:
  Platform:
    osname=linux, osvers=2.2.16, archname=i586-linux
    uname='linux plover 2.2.16 #97 fri jun 16 19:45:30 pdt 2000 i586 unknown '
    config_args='-Doptimize=-g -des'
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef
    useperlio=undef d_sfio=undef uselargefiles=define 
    use64bitint=undef use64bitall=undef uselongdouble=undef usesocks=undef
  Compiler:
    cc='cc', optimize='-g', gccversion=egcs-2.91.66 19990314/Linux (egcs-1.1.2 release)
    cppflags='-DDEBUGGING -fno-strict-aliasing -I/usr/local/include'
    ccflags ='-DDEBUGGING -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
    stdchar='char', d_stdstdio=define, usevfork=false
    intsize=4, longsize=4, ptrsize=4, doublesize=8
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=4, usemymalloc=n, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lnsl -lndbm -lgdbm -ldb -ldl -lm -lc -lposix -lcrypt
    libc=/lib/libc-2.1.3.so, so=so, useshrplib=false, libperl=libperl.a
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic'
    cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:
    


@INC for perl v5.6.0:
    /usr/local/lib/perl5/5.6.0/i586-linux
    /usr/local/lib/perl5/5.6.0
    /usr/local/lib/perl5/site_perl/5.6.0/i586-linux
    /usr/local/lib/perl5/site_perl/5.6.0
    /usr/local/lib/perl5/site_perl/5.00557/i586-linux
    /usr/local/lib/perl5/site_perl/5.00557
    /usr/local/lib/perl5/site_perl
    .


Environment for perl v5.6.0:
    HOME=/home/mjd
    LANG (unset)
    LANGUAGE (unset)
    LC_ALL=POSIX
    LD_LIBRARY_PATH=/lib:/usr/lib:/usr/X11R6/lib
    LOGDIR (unset)
    PATH=/home/mjd/bin:/usr/local/bin:/bin:/usr/bin:/usr/X11/bin:/usr/games:/sbin:/usr/sbin:/usr/local/bin/X11:/usr/local/bin/mh:/data/mysql/bin:/usr/local/bin/pbm:/usr/local/bin/ezmlm:/home/mjd/TPI/bin:/usr/local/mysql/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

[p5pRT]

From @rspier

Searching the BugDB for easy ones to close. A quick summary is that
Mark discovered that exists() will trigger a FETCH if the EXISTS method
returns undef. It was noted that this isn't documented anywhere. The
patch below solves this issue.

http​://bugs.perl.org/perlbug.cgi?req=bidmids&bidmids=19990808.001&range=23873&format=H

(If you choose to apply this patch, the bug can be easily closed by
BCCing close_19990808.001@​bugs.perl.org)

Inline Patch

--- perltie.pod.orig    Fri Mar  9 02:03:59 2001
+++ perltie.pod Fri Mar  9 02:09:41 2001
@@ -725,6 +725,10 @@
        return exists $self->{LIST}->{$dot};
     }
 
+If your method returns undef (as opposed to a true or false value)
+perl will attempt to satisfy the exists() function by triggering a C<FETCH>.
+If C<FETCH> returns a true value, exists() will be true.
+
 =item FIRSTKEY this
 
 This method will be triggered when the user is going

[p5pRT]

From @mjdominus

The patch below solves this issue.

I respectfully disagree. I think this is a bug, not a feature.

At least, it is not normal for Perl to treat 'undef' in this special
way; every other place where Perl expects a boolean value, undefined
is taken to mean false. And I cannot remember that last time this
came up anybody advanced any reason why the existing behavior was
desirable. If the author of the EXISTS method wanted to call FETCH
and return that value, they would be free to do so.

I think the correct approach is to fix it, or at least to leave it as
an undocumented oddity that might be fixed in the future. Documenting
it requires that it remain forever.

[p5pRT]

From @rspier

What you're saying makes sense.

So, I'll start following it down the other path​:

A little bit of GDB walking and here's what I see​:

FETCH is being called inside the
hv.c​:934 return SvTRUE(sv);

SvTRUE is a scary macro which calls sv_2bool
sv_2bool on a magical variable calls mg_get()

and mg_get calls the get entry in the vtable for the magic, which for
a hash is Perl_magic_getpack, which does the FETCH method
invocation. *kaboom*

I think the problem is that the sv passed to
  magic_existspack(sv, mg_find(sv, 'p'));
is Magical, because it'a copy of the magical HV.

I think the patch below may (actually) solve this problem (instead of
documenting it firmly in place). It creates a new SV to hold the
return value from magic_existspack, instead of using the magical
pointer.

All tests in the test suite pass for me, (except for op/qu.t, which is
very unrelated.)

Inline Patch

--- hv.c.orig   Fri Mar  9 03:16:20 2001
+++ hv.c        Fri Mar  9 03:20:34 2001
@@ -927,11 +927,12 @@
 
     if (SvRMAGICAL(hv)) {
        if (mg_find((SV*)hv,'P')) {
+           SV* svret = sv_newmortal();
            sv = sv_newmortal();
            keysv = sv_2mortal(newSVsv(keysv));
            mg_copy((SV*)hv, sv, (char*)keysv, HEf_SVKEY);
-           magic_existspack(sv, mg_find(sv, 'p'));
-           return SvTRUE(sv);
+           magic_existspack(svret, mg_find(sv, 'p'));
+           return SvTRUE(svret);
        }
 #ifdef ENV_IS_CASELESS
        else if (mg_find((SV*)hv,'E')) {

-R

>>>>> "MD" == Mark-Jason Dominus writes:

The patch below solves this issue.

MD> I respectfully disagree. I think this is a bug, not a feature.

MD> At least, it is not normal for Perl to treat 'undef' in this
MD> special way; every other place where Perl expects a boolean value,
MD> undefined is taken to mean false. And I cannot remember that last
MD> time this came up anybody advanced any reason why the existing
MD> behavior was desirable. If the author of the EXISTS method wanted
MD> to call FETCH and return that value, they would be free to do so.

MD> I think the correct approach is to fix it, or at least to leave it
MD> as an undocumented oddity that might be fixed in the future.
MD> Documenting it requires that it remain forever.

[p5pRT]

From @jhi

On Fri, Mar 09, 2001 at 03​:30​:20AM -0500, Robert Spier wrote​:

What you're saying makes sense.

So, I'll start following it down the other path​:

A little bit of GDB walking and here's what I see​:

FETCH is being called inside the
hv.c​:934 return SvTRUE(sv);

SvTRUE is a scary macro which calls sv_2bool
sv_2bool on a magical variable calls mg_get()

and mg_get calls the get entry in the vtable for the magic, which for
a hash is Perl_magic_getpack, which does the FETCH method
invocation. *kaboom*

I think the problem is that the sv passed to
magic_existspack(sv, mg_find(sv, 'p'));
is Magical, because it'a copy of the magical HV.

I think the patch below may (actually) solve this problem (instead of
documenting it firmly in place). It creates a new SV to hold the
return value from magic_existspack, instead of using the magical
pointer.

All tests in the test suite pass for me, (except for op/qu.t, which is

Thanks, applied.

rm t/op/qu.t

[p5pRT]

From The RT System itself

patch included in change @​9090. Waiting a few days for fallout before closing bug.

[p5pRT]

From [Unknown Contact. See original ticket]

dupe