refaliasing breaks refcounting of field variable

[leonerd]

(copied from email)

Combining the experimental core class system with experimental refaliasing, here's an attempt to replace a field variable:

$ perl5.37.9 -Mexperimental=refaliasing,class -lwe 'class Foo { field $aa; method bb { print \$aa; } method cc { print \$aa; \$aa = \(my$x); print \$aa; } } $x = Foo->new; $x->bb; $x->cc; $x->bb;'
SCALAR(0x56013fda95f0)
SCALAR(0x56013fda95f0)
SCALAR(0x56013febfd80)
UNKNOWN(0x56013fda95f0)
zsh: segmentation fault  perl5.37.9 -Mexperimental=refaliasing,class -lwe 

How this ought to behave is up for debate. Maybe the replacement operation should croak, maybe it should affect the pad of the running method without affecting the field variable, and maybe it should replace the field variable in the object representation. But it certainly shouldn't do the above, prematurely freeing an SV and then segfaulting. It looks to me like this is doing a replacement in the pad but getting confused about the refcounting of the pad entry.

---

https://www.nntp.perl.org/group/perl.perl5.porters/2023/03/msg266060.html

[leonerd]

But it certainly shouldn't do the above, prematurely freeing an SV and then segfaulting.

Agree; the current behaviour is definitely a bug.

How this ought to behave is up for debate. Maybe the replacement operation should croak, maybe it should affect the pad of the running method without affecting the field variable, and maybe it should replace the field variable in the object representation.

I think there are possibly arguments for and against any of these situations. The first thought that comes to mind is that I don't think there are currently any other situations where a refalias operation will actually croak, so doing so here feels weird. I wonder if the best thing to do is to aim at as similar behaviour as would happen on a refalias of a regular (blessed) hash element:

$ perl -Mexperimental=refaliasing -E \
  'my $self = bless { x => 123 };
  sub aaa { \$self->{x} = \456; } 
  sub bbb { say $self->{x} }

  aaa(); bbb();'
456

I.e. that the actual field storage in the actual object should be replaced.

Annoyingly it would be hard to arrange for that to happen, because during OP_METHSTART the pad entries of the method CV just get aliased from the object fields, so knowing to set new values in there would be nontrivial.

In any case, the actual SV in the pad doesn't look any different, so the refalising operation would have to inspect PadnameIsFIELD() of the corresponding pad name in order to know whether to take special measures here.

Exactly what special measures are, as you say, up for debate.

Opinions anyone?

[leonerd]

Actually, another thought says that if these field variables behave more like captured lexicals, then perhaps the change is only visible during the lifetime of a single invocation of the method:

$ perl -Mexperimental=refaliasing -E \
  'my $field = 123;
   sub aaa { \$field = \456; say "$field in aaa" }
   sub bbb { say "$field in bbb" }

   aaa(); bbb();'
456 in aaa
123 in bbb

[aquanight]

Actually, another thought says that if these field variables behave more like captured lexicals, then perhaps the change is only visible during the lifetime of a single invocation of the method:

$ perl -Mexperimental=refaliasing -E \
  'my $field = 123;
   sub aaa { \$field = \456; say "$field in aaa" }
   sub bbb { say "$field in bbb" }

   aaa(); bbb();'
456 in aaa
123 in bbb

Actually it's a bit more complicated than that. If you want to treat it like a captured lexical, the refalias replaces that variable permanently for the entire life of that method (asterisk):

sub foo {
    my $x = 1;
    return sub {
        \$x = \(my $z = $x * 2);
        return \$x;
    }
}
my $c = foo;
my @x;
push @x, $c->() for 1..7;
my $sum = List::Util::sum0 map {$$_} @x;
# Which is this: 14, 254, or 896?
# - 14 would mean \$x reverted to the closed-over $x (containing 1) with each invocation of the anonsub.
# - 896 (or any other multiple of 7 really) would mean SOMEHOW the inner sub kept returning references to the same variable which shouldn't be possible at all
# - What you get is 254 because the anonsub is permanently replacing its closed over variable with the new one each time.

Asterisk: of course it's even worse. In reality it would need to replace the "field", for only that method, and for only that instance, and for the entire life of that particular instance. Because conceptually you'd be viewing the method as closed over that specific instance's fields, which means there's a conceptually-separate closure of the method for each instance (in reality they're just one method that rebinds to the fields with each invocation).