overload and rebless

[p5pRT]

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

Searchable as RT34925$

[p5pRT]

From @nwc10

Created by @nwc10

clkao actually found this bug, but he's having trouble reporting it.

reblessing a object in a overloaded class doesn't clear the magic on some
condition.
Compare delete with delete_with_self in the test case.

Even if ->delete is called from within delete_with_self, it still doesn't
work.

#!/usr/bin/perl -w
use strict;
package Foo;

use overload
  bool => sub { shift->is_cool };

sub is_cool {
  $_[0]->{name} eq 'cool';
}

sub delete {
  undef %{$_[0]};
  bless $_[0], 'Something​::Else';
  return 1;
}

sub delete_with_self {
  my $self = shift;
  undef %$self;
  bless $self, 'Something​::Else';
  return 1;
}

package Something​::Else;

1;

package main;
use Test​::More tests => 2;

my $obj;
$obj = bless {name => 'cool'}, 'Foo';
print "ya\n" if $obj;
$obj->delete;
ok (eval {if ($obj) {1}; 1}, $@​ || 'reblessed into nonexist namespace');

$obj = bless {name => 'cool'}, 'Foo';
print "ya\n" if $obj;
$obj->delete_with_self;
ok (eval {if ($obj) {1}; 1}, $@​);
__END__
1..2
ya
ok 1 - reblessed into nonexist namespace
ya
not ok 2 - Operation `bool'​: no method found, argument in overloaded package Something​::Else at overload.pl line 41.
#
# Failed test (overload.pl at line 41)
# Looks like you failed 1 test of 2.

Nicholas Clark

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl v5.9.3:

Configured by nick at Mon Apr 11 12:23:37 BST 2005.

Summary of my perl5 (revision 5 version 9 subversion 3 patch 24148) configuration:
  Platform:
    osname=darwin, osvers=7.8.0, archname=darwin-2level
    uname='darwin ship-in-a-bottle 7.8.0 darwin kernel version 7.8.0: wed dec 22 14:26:17 pst 2004; root:xnuxnu-517.11.1.obj~1release_ppc power macintosh powerpc '
    config_args='-Dusedevel=y -Dcc=ccache gcc -Dld=gcc -Ubincompat5005 -Uinstallusrbinperl -Dcf_email=nick@ccl4.org -Dperladmin=nick@ccl4.org -Dinc_version_list=5.8.7 5.8.8 -Dinc_version_list_init=0 -Doptimize=-g -Dusethreads=n -Uuse64bitint -Duselargefiles -Dprefix=~/Sandpit/blead24227 -de'
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=undef useithreads=undef usemultiplicity=undef
    useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
    use64bitint=undef use64bitall=undef uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='ccache gcc', ccflags ='-fno-common -DPERL_DARWIN -no-cpp-precomp -DDEBUGGING -fno-strict-aliasing -pipe',
    optimize='-g',
    cppflags='-no-cpp-precomp -fno-common -DPERL_DARWIN -no-cpp-precomp -DDEBUGGING -fno-strict-aliasing -pipe'
    ccversion='', gccversion='3.3 20030304 (Apple Computer, Inc. build 1666)', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=4321
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=8
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc', ldflags =' -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib
    libs=-ldbm -ldl -lm -lc
    perllibs=-ldl -lm -lc
    libc=/usr/lib/libc.dylib, so=dylib, useshrplib=false, libperl=libperl.a
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dyld.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib'

Locally applied patches:
    


@INC for perl v5.9.3:
    lib
    /sw/lib/perl5
    /sw/lib/perl5/darwin
    /Users/nick/Sandpit/blead24227/lib/perl5/5.9.3/darwin-2level
    /Users/nick/Sandpit/blead24227/lib/perl5/5.9.3
    /Users/nick/Sandpit/blead24227/lib/perl5/site_perl/5.9.3/darwin-2level
    /Users/nick/Sandpit/blead24227/lib/perl5/site_perl/5.9.3
    /Users/nick/Sandpit/blead24227/lib/perl5/site_perl
    .


Environment for perl v5.9.3:
    DYLD_LIBRARY_PATH (unset)
    HOME=/Users/nick
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/Users/nick/bin:/sw/bin:/sw/sbin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/X11R6/bin:/usr/local/sbin:/sbin:/usr/sbin
    PERL5LIB=/sw/lib/perl5:/sw/lib/perl5/darwin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

[p5pRT]

From @nwc10

On Tue, Apr 12, 2005 at 01​:24​:58PM -0000, Nicholas Clark wrote​:

reblessing a object in a overloaded class doesn't clear the magic on some
condition.
Compare delete with delete_with_self in the test case.

It looks like it's a more fundamental problem of where the overloading flag
is stored​:

#!/usr/bin/perl -w
use strict;
package Foo;

use overload
  bool => sub { 0 };

sub quack {
  print "quacks like a Foo\n"
}
package main;

my %hash;

my $o1 = \%hash;
my $o2 = \%hash;
bless $o1, 'Foo';

print $o1 ? "o1 true\n" : "o1 false\n";
print $o2 ? "o2 true\n" : "o2 false\n";
$o2->quack();
__END__
o1 false
o2 true
quacks like a Foo

To make things work, it needs to be on the referent, not the reference.

Nicholas Clark

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From tony@kasei.com

On Tue, Apr 12, 2005 at 01​:24​:58PM -0000, Nicholas Clark wrote​:

reblessing a object in a overloaded class doesn't clear the magic on some
condition.

This isn't just an abstract bug; this causes problems for Class​::DBI
users on a regular basis...

Tony

[p5pRT]

From @nwc10

On Tue, Apr 12, 2005 at 08​:17​:20PM +0100, Tony Bowden wrote​:

On Tue, Apr 12, 2005 at 01​:24​:58PM -0000, Nicholas Clark wrote​:

reblessing a object in a overloaded class doesn't clear the magic on some
condition.

This isn't just an abstract bug; this causes problems for Class​::DBI
users on a regular basis...

Which is how clkao found it. (should have said)

Nicholas Clark

[p5pRT]

From @ysth

On Tue, Apr 12, 2005 at 03​:55​:40PM +0100, Nicholas Clark wrote​:

It looks like it's a more fundamental problem of where the overloading flag
is stored​:

To make things work, it needs to be on the referent, not the reference.

I'm not sure there's a flag available on every type of referent, which
may be why Ilya did it this way. Why does Class​::DBI need to do this?
Does it re-bless temporarily or permanently?

The existing implementation should work for almost every purpose so
long as you only bless via the actual RV you intend to use.

There will be a slight slowdown if you succeed in moving the flag to
the referent, especially on overloadable ops that are normally take RV
operands.

[p5pRT]

From @nwc10

On Wed, Apr 13, 2005 at 11​:03​:00AM -0700, Yitzchak Scott-Thoennes wrote​:

On Tue, Apr 12, 2005 at 03​:55​:40PM +0100, Nicholas Clark wrote​:

It looks like it's a more fundamental problem of where the overloading flag
is stored​:

To make things work, it needs to be on the referent, not the reference.

I'm not sure there's a flag available on every type of referent, which
may be why Ilya did it this way. Why does Class​::DBI need to do this?

I think that 0x10000000 was available on all SV types back then.

This would have to move​:

#define SVphv_REHASH 0x10000000 /* HV is recalculating hash values */

but as that's an internal implementation flag, anyone outside the core would
have to "deal with it"

Does it re-bless temporarily or permanently?

The existing implementation should work for almost every purpose so
long as you only bless via the actual RV you intend to use.

There will be a slight slowdown if you succeed in moving the flag to
the referent, especially on overloadable ops that are normally take RV
operands.

I believe that there would only be a slight slowdown for all cases when the
arguments were references and something somewhere is using overloading. And
no change for regular arguments. The initial check would be on SvROK()
rather than SvAMAGIC(), so it's still 1 flag bit.

Nicholas Clark

[p5pRT]

From @ysth

On Thu, Apr 14, 2005 at 10​:52​:36AM +0100, Nicholas Clark wrote​:

On Wed, Apr 13, 2005 at 11​:03​:00AM -0700, Yitzchak Scott-Thoennes wrote​:

On Tue, Apr 12, 2005 at 03​:55​:40PM +0100, Nicholas Clark wrote​:

It looks like it's a more fundamental problem of where the overloading flag
is stored​:

To make things work, it needs to be on the referent, not the reference.

I'm not sure there's a flag available on every type of referent, which
may be why Ilya did it this way. Why does Class​::DBI need to do this?

I think that 0x10000000 was available on all SV types back then.

This would have to move​:

#define SVphv_REHASH 0x10000000 /* HV is recalculating hash values */

but as that's an internal implementation flag, anyone outside the core would
have to "deal with it"

Does it re-bless temporarily or permanently?

The existing implementation should work for almost every purpose so
long as you only bless via the actual RV you intend to use.

There will be a slight slowdown if you succeed in moving the flag to
the referent, especially on overloadable ops that are normally take RV
operands.

I believe that there would only be a slight slowdown for all cases when the
arguments were references and something somewhere is using overloading. And
no change for regular arguments. The initial check would be on SvROK()
rather than SvAMAGIC(), so it's still 1 flag bit.

I was thinking principally about rv2av and rv2hv, common ops where ROK
is usually true, so checking ROK first doesn't help. But since those
dereference the RV anyway, there's no extra cost needed to check the
referent; just requires rewriting tryAMAGICunDEREF, probablly to take
the result of the SvRV as a parameter.

[p5pRT]

From @nwc10

On Thu, Apr 14, 2005 at 10​:52​:36AM +0100, Nicholas Clark wrote​:

On Wed, Apr 13, 2005 at 11​:03​:00AM -0700, Yitzchak Scott-Thoennes wrote​:

On Tue, Apr 12, 2005 at 03​:55​:40PM +0100, Nicholas Clark wrote​:

It looks like it's a more fundamental problem of where the overloading flag
is stored​:

To make things work, it needs to be on the referent, not the reference.

I'm not sure there's a flag available on every type of referent, which
may be why Ilya did it this way. Why does Class​::DBI need to do this?

I think that 0x10000000 was available on all SV types back then.

This would have to move​:

#define SVphv_REHASH 0x10000000 /* HV is recalculating hash values */

but as that's an internal implementation flag, anyone outside the core would
have to "deal with it"

I can see 3 possible solutions​:

1​: Move SVphv_REHASH somewhere else, and change the overloading code so that
  SVf_AMAGIC is stored on the referent rather than the reference. (And only
  on the referent)

This is source compatible but not binary compatible. The only thing I can
find on CPAN that references SVphv_REHASH is mod_perl 2, but I'm also not
sure what the implied change would be to existing XS code that is checking
for overloading. So I don't think that it can be done for maint, except as
a non-default conditional compile.

But it seems sane for blead and therefore 5.10

2​: Dave said something about one of the other flags possibly being able to be
  freed up. I have no idea which, but my hunch is that it's one of the pad
  related flags. I'm not sure if this helps.

3​: The problem is really only on rebless, isn't it? If so, a correct solution
  would be on *re*bless, and only where overloading changes, to brute force
  search for all the other references to the referent, and fix them up.

This would be binary compatible.

Nicholas Clark

[p5pRT]

From @JohnPeacock

Nicholas Clark wrote​:

This is source compatible but not binary compatible. The only thing I can
find on CPAN that references SVphv_REHASH is mod_perl 2, but I'm also not
sure what the implied change would be to existing XS code that is checking
for overloading. So I don't think that it can be done for maint, except as
a non-default conditional compile.

AFAIK I am the only person crazy enough to deal with overloading and XS
(in the version compatibility module), even though there has been code
in xsubpp to support overloading in XS since 5.8.1 (I think, my
Perforce-to-Subversion mirror is broken right now).

John

--
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4501 Forbes Boulevard
Suite H
Lanham, MD 20706
301-459-3366 x.5010
fax 301-429-5748

[p5pRT]

From @ysth

On Tue, Mar 14, 2006 at 05​:13​:42PM +0000, Nicholas Clark wrote​:

3​: The problem is really only on rebless, isn't it? If so, a correct solution
would be on *re*bless, and only where overloading changes, to brute force
search for all the other references to the referent, and fix them up.

No.

  $a = $b = {};
  bless $b, OverloadedClass;

leaves $a not marked as overloaded.

A brute force search could be quite lengthy. Better just to document the
issue, I think.

[p5pRT]

From @nwc10

On Tue, Mar 14, 2006 at 09​:30​:41AM -0800, Yitzchak Scott-Thoennes wrote​:

On Tue, Mar 14, 2006 at 05​:13​:42PM +0000, Nicholas Clark wrote​:

3​: The problem is really only on rebless, isn't it? If so, a correct solution
would be on *re*bless, and only where overloading changes, to brute force
search for all the other references to the referent, and fix them up.

No.

$a = $b = {};
bless $b, OverloadedClass;

leaves $a not marked as overloaded.

My impression is that that construction is rare, in that I don't know of
any idioms that require a non-object to be blessed into an object, outside
of constructors.

Whereas reblessing existing objects does seem to be a commonly used idiom.

A brute force search could be quite lengthy. Better just to document the
issue, I think.

I'm happy to document the example you give as not working, because I suspect
it can be worked round easily. I still think that it's worth investigating
the time for a scan (for the specific case of reblessing an object from
a class with overloading to a class without). Given that without a solution
Class​::DBI can never work 100% correctly.

Nicholas Clark

[p5pRT]

From @nwc10

On Tue, Mar 14, 2006 at 05​:13​:42PM +0000, Nicholas Clark wrote​:

On Thu, Apr 14, 2005 at 10​:52​:36AM +0100, Nicholas Clark wrote​:

On Wed, Apr 13, 2005 at 11​:03​:00AM -0700, Yitzchak Scott-Thoennes wrote​:

On Tue, Apr 12, 2005 at 03​:55​:40PM +0100, Nicholas Clark wrote​:

It looks like it's a more fundamental problem of where the overloading flag
is stored​:

To make things work, it needs to be on the referent, not the reference.

I'm not sure there's a flag available on every type of referent, which
may be why Ilya did it this way. Why does Class​::DBI need to do this?

I think that 0x10000000 was available on all SV types back then.

This would have to move​:

#define SVphv_REHASH 0x10000000 /* HV is recalculating hash values */

but as that's an internal implementation flag, anyone outside the core would
have to "deal with it"

I can see 3 possible solutions​:

1​: Move SVphv_REHASH somewhere else, and change the overloading code so that
SVf_AMAGIC is stored on the referent rather than the reference. (And only
on the referent)

This is source compatible but not binary compatible. The only thing I can
find on CPAN that references SVphv_REHASH is mod_perl 2, but I'm also not
sure what the implied change would be to existing XS code that is checking
for overloading. So I don't think that it can be done for maint, except as
a non-default conditional compile.

But it seems sane for blead and therefore 5.10

I have applied this. All tests pass, including new tests for reblessing.

Perlbench reports​:

  A B C D E F
  ---- ---- ---- ---- ---- ----
arith/mixed 100 100 100 100 100 100
arith/trig 100 100 100 100 100 100
array/copy 100 104 100 104 100 104
array/foreach 100 99 100 98 100 100
array/index 100 99 100 99 100 99
array/pop 100 108 100 109 100 108
array/shift 100 104 100 104 100 104
array/sort-num 100 103 100 103 100 103
array/sort 100 102 100 103 100 102
call/0arg 100 101 100 101 100 101
call/1arg 100 102 100 102 100 102
call/2arg 100 104 100 104 100 104
call/9arg 100 103 100 103 100 103
call/empty 100 102 100 102 100 102
call/fib 100 101 100 101 100 101
call/method 100 101 100 100 100 100
call/wantarray 100 90 100 90 100 90
hash/copy 100 95 100 95 100 95
hash/each 100 97 100 97 100 97
hash/foreach-sort 100 98 100 98 100 98
hash/foreach 100 101 100 101 100 101
hash/get 100 101 100 101 100 101
hash/set 100 97 100 97 100 97
loop/for-c 100 103 100 103 100 103
loop/for-range-const 100 99 100 100 100 99
loop/for-range 100 96 100 96 100 96
loop/getline 100 148 100 148 99 148
loop/while-my 100 102 100 102 100 103
loop/while 100 107 100 107 99 107
re/const 100 111 100 112 100 112
re/w 100 98 100 98 100 98
startup/fewmod 100 101 100 101 100 101
startup/lotsofsub 100 97 100 98 100 98
startup/noprog 100 100 100 100 100 100
string/base64 100 83 100 83 100 83
string/htmlparser 100 97 100 97 100 97
string/index-const 100 101 100 101 101 101
string/index-var 100 106 100 106 101 105
string/ipol 100 93 100 93 100 93
string/tr 100 97 101 98 100 97

AVERAGE 100 101 100 101 100 101

(A, C, E are without, B, D, F are with. I guess that this much randomness
means I'm ending up with alignment issues that in turn cause cache level
effects. Lies, damn lies, statistics ... benchmarks ...)

Nicholas Clark

[p5pRT]

From @nwc10

On Tue, Mar 14, 2006 at 05​:53​:08PM +0000, Nicholas Clark wrote​:

On Tue, Mar 14, 2006 at 09​:30​:41AM -0800, Yitzchak Scott-Thoennes wrote​:

On Tue, Mar 14, 2006 at 05​:13​:42PM +0000, Nicholas Clark wrote​:

3​: The problem is really only on rebless, isn't it? If so, a correct solution
would be on *re*bless, and only where overloading changes, to brute force
search for all the other references to the referent, and fix them up.

No.

$a = $b = {};
bless $b, OverloadedClass;

leaves $a not marked as overloaded.

My impression is that that construction is rare, in that I don't know of
any idioms that require a non-object to be blessed into an object, outside
of constructors.

Whereas reblessing existing objects does seem to be a commonly used idiom.

A brute force search could be quite lengthy. Better just to document the
issue, I think.

I'm happy to document the example you give as not working, because I suspect
it can be worked round easily. I still think that it's worth investigating
the time for a scan (for the specific case of reblessing an object from
a class with overloading to a class without). Given that without a solution
Class​::DBI can never work 100% correctly.

Well, it turned out to be easier to scan for both bless and rebless, rather
than just for bless.

Change 27512 makes maint scan for other references. The scan is conditional
on the overloading state actually changing, and there actually being other
references to find, and the scan quits as soon as it's found them all.

I haven't actually timed this.

Nicholas Clark

[p5pRT]

From nick@ing-simmons.net

John Peacock <jpeacock@​rowman.com> writes​:

Nicholas Clark wrote​:

This is source compatible but not binary compatible. The only thing I can
find on CPAN that references SVphv_REHASH is mod_perl 2, but I'm also not
sure what the implied change would be to existing XS code that is checking
for overloading. So I don't think that it can be done for maint, except as
a non-default conditional compile.

AFAIK I am the only person crazy enough to deal with overloading and XS
(in the version compatibility module), even though there has been code
in xsubpp to support overloading in XS since 5.8.1 (I think, my
Perforce-to-Subversion mirror is broken right now).

Audio​::Data does overloading with XS but doesn't rebless or check
for overload.

John

[p5pRT]

@rgs - Status changed from 'open' to 'resolved'