[perl #131887] [BUG] method freeze(Pair:D:) changes object identity

2018-03-13 Thread Jan-Olof Hendig via RT
On Sun, 13 Aug 2017 15:14:41 -0700, elizabeth wrote:
> Fixed with https://github.com/rakudo/rakudo/commit/c229022cb0 , tests
> needed
> 
> > On 12 Aug 2017, at 14:36, Peter du Marchie van Voorthuysen (via RT)
> >  wrote:
> >
> > # New Ticket Created by  Peter du Marchie van Voorthuysen
> > # Please include the string:  [perl #131887]
> > # in the subject line of all future correspondence about this issue.
> > # https://rt.perl.org/Ticket/Display.html?id=131887 >
> >
> >
> > This is Rakudo version 2017.07 built on MoarVM version 2017.07
> > implementing Perl 6.c.
> >
> > If the value of a Pair is a Scalar container, then the Pair can be
> > modified, e.g.
> >
> >> my $value = 0;
> > 0
> >> my $pair = number => $value;
> > number => 0
> >> $pair.value = 1; $pair;
> > number => 1
> >
> > Method freeze make the value of the Pair read-only, by removing it
> > from its
> > Scalar container, and returns the value.
> >
> >> $pair.freeze;
> > 1
> >> $pair.value = 2;
> > Cannot modify an immutable Int (1)
> >   in block  at  line 1
> >
> > The problem is that freeze does more than that. It changes the object
> > identity as returned by WHICH as well:
> >
> >> $pair = number => $value;
> > number => 1
> >> $pair.WHICH;
> > Pair|127791728
> >> $pair.freeze;
> > 1
> >> $pair.WHICH;
> > Pair|Str|number|Int|1
> >
> > Now by itself having a 2-tuple that is identified by its two elements
> > is a
> > nice feature (if it would be documented). But _changing_ the object
> > identity is not consistent with the behavior of other built-in Perl 6
> > classes and actually breaks the implementation of some of these
> > classes.
> >
> > For example, a SetHash represents a mutable set. The Set method
> > returns a
> > _new_ object that is immutable:
> >
> >> $pair = number => $value;
> > number => 1
> >> my $set = SetHash.new($pair);
> > SetHash.new(number => 1)
> >> my $set2 = $set.Set;
> > set(number => 1)
> >> $set.WHICH;
> > SetHash|136539408
> >> $set2.WHICH;
> > Set|0EC3BFFD57719F5C6A3EE91A5EFAA5AEFE273964
> >
> > But because freezing a Pair changes the identity of the _original_
> > object
> > it's possible to add a second instance of the _same_ Pair to the
> > SetHash,
> > causing it to violate its contract:
> >
> >> $pair.freeze;
> > 1
> >> $set{$pair} = True;
> > True
> >> my ($a, $b) = $set.keys;
> > (number => 1 number => 1)
> >> $a === $b;
> > True
> >
> > I think it's clear that changing the identity of the original object
> > is not
> > correct. So I propose to remove the undocumented behavior of the
> > freeze
> > method that it changes the object identity.
> >
> > Now I can imagine that at some implementation level there are
> > benefits to
> > having a kind of Pair that is identified by its key _and_ value. I
> > also
> > think it could be generally useful to have a class implementing a
> > truly
> > immutable (2-)tuple that is identified by its elements. But that
> > should be
> > a separate class and the Pair class should provide a method to create
> > a
> > _new_ object of this class from a Pair object.

Test added with commit https://github.com/perl6/roast/commit/d7d42d6c37


Re: [perl #131887] [BUG] method freeze(Pair:D:) changes object identity

2017-08-13 Thread Elizabeth Mattijsen
Fixed with https://github.com/rakudo/rakudo/commit/c229022cb0 , tests needed

> On 12 Aug 2017, at 14:36, Peter du Marchie van Voorthuysen (via RT) 
>  wrote:
> 
> # New Ticket Created by  Peter du Marchie van Voorthuysen 
> # Please include the string:  [perl #131887]
> # in the subject line of all future correspondence about this issue. 
> # https://rt.perl.org/Ticket/Display.html?id=131887 >
> 
> 
> This is Rakudo version 2017.07 built on MoarVM version 2017.07
> implementing Perl 6.c.
> 
> If the value of a Pair is a Scalar container, then the Pair can be
> modified, e.g.
> 
>> my $value = 0;
>0
>> my $pair = number => $value;
>number => 0
>> $pair.value = 1; $pair;
>number => 1
> 
> Method freeze make the value of the Pair read-only, by removing it from its
> Scalar container, and returns the value.
> 
>> $pair.freeze;
>1
>> $pair.value = 2;
>Cannot modify an immutable Int (1)
>  in block  at  line 1
> 
> The problem is that freeze does more than that. It changes the object
> identity as returned by WHICH as well:
> 
>> $pair = number => $value;
>number => 1
>> $pair.WHICH;
>Pair|127791728
>> $pair.freeze;
>1
>> $pair.WHICH;
>Pair|Str|number|Int|1
> 
> Now by itself having a 2-tuple that is identified by its two elements is a
> nice feature (if it would be documented). But _changing_ the object
> identity is not consistent with the behavior of other built-in Perl 6
> classes and actually breaks the implementation of some of these classes.
> 
> For example, a SetHash represents a mutable set. The Set method returns a
> _new_ object that is immutable:
> 
>> $pair = number => $value;
>number => 1
>> my $set = SetHash.new($pair);
>SetHash.new(number => 1)
>> my $set2 = $set.Set;
>set(number => 1)
>> $set.WHICH;
>SetHash|136539408
>> $set2.WHICH;
>Set|0EC3BFFD57719F5C6A3EE91A5EFAA5AEFE273964
> 
> But because freezing a Pair changes the identity of the _original_ object
> it's possible to add a second instance of the _same_ Pair to the SetHash,
> causing it to violate its contract:
> 
>> $pair.freeze;
>1
>> $set{$pair} = True;
>True
>> my ($a, $b) = $set.keys;
>(number => 1 number => 1)
>> $a === $b;
>True
> 
> I think it's clear that changing the identity of the original object is not
> correct. So I propose to remove the undocumented behavior of the freeze
> method that it changes the object identity.
> 
> Now I can imagine that at some implementation level there are benefits to
> having a kind of Pair that is identified by its key _and_ value. I also
> think it could be generally useful to have a class implementing a truly
> immutable (2-)tuple that is identified by its elements. But that should be
> a separate class and the Pair class should provide a method to create a
> _new_ object of this class from a Pair object.


Re: [perl #131887] [BUG] method freeze(Pair:D:) changes object identity

2017-08-13 Thread Elizabeth Mattijsen via RT
Fixed with https://github.com/rakudo/rakudo/commit/c229022cb0 , tests needed

> On 12 Aug 2017, at 14:36, Peter du Marchie van Voorthuysen (via RT) 
>  wrote:
> 
> # New Ticket Created by  Peter du Marchie van Voorthuysen 
> # Please include the string:  [perl #131887]
> # in the subject line of all future correspondence about this issue. 
> # https://rt.perl.org/Ticket/Display.html?id=131887 >
> 
> 
> This is Rakudo version 2017.07 built on MoarVM version 2017.07
> implementing Perl 6.c.
> 
> If the value of a Pair is a Scalar container, then the Pair can be
> modified, e.g.
> 
>> my $value = 0;
>0
>> my $pair = number => $value;
>number => 0
>> $pair.value = 1; $pair;
>number => 1
> 
> Method freeze make the value of the Pair read-only, by removing it from its
> Scalar container, and returns the value.
> 
>> $pair.freeze;
>1
>> $pair.value = 2;
>Cannot modify an immutable Int (1)
>  in block  at  line 1
> 
> The problem is that freeze does more than that. It changes the object
> identity as returned by WHICH as well:
> 
>> $pair = number => $value;
>number => 1
>> $pair.WHICH;
>Pair|127791728
>> $pair.freeze;
>1
>> $pair.WHICH;
>Pair|Str|number|Int|1
> 
> Now by itself having a 2-tuple that is identified by its two elements is a
> nice feature (if it would be documented). But _changing_ the object
> identity is not consistent with the behavior of other built-in Perl 6
> classes and actually breaks the implementation of some of these classes.
> 
> For example, a SetHash represents a mutable set. The Set method returns a
> _new_ object that is immutable:
> 
>> $pair = number => $value;
>number => 1
>> my $set = SetHash.new($pair);
>SetHash.new(number => 1)
>> my $set2 = $set.Set;
>set(number => 1)
>> $set.WHICH;
>SetHash|136539408
>> $set2.WHICH;
>Set|0EC3BFFD57719F5C6A3EE91A5EFAA5AEFE273964
> 
> But because freezing a Pair changes the identity of the _original_ object
> it's possible to add a second instance of the _same_ Pair to the SetHash,
> causing it to violate its contract:
> 
>> $pair.freeze;
>1
>> $set{$pair} = True;
>True
>> my ($a, $b) = $set.keys;
>(number => 1 number => 1)
>> $a === $b;
>True
> 
> I think it's clear that changing the identity of the original object is not
> correct. So I propose to remove the undocumented behavior of the freeze
> method that it changes the object identity.
> 
> Now I can imagine that at some implementation level there are benefits to
> having a kind of Pair that is identified by its key _and_ value. I also
> think it could be generally useful to have a class implementing a truly
> immutable (2-)tuple that is identified by its elements. But that should be
> a separate class and the Pair class should provide a method to create a
> _new_ object of this class from a Pair object.


[perl #131887] [BUG] method freeze(Pair:D:) changes object identity

2017-08-12 Thread via RT
# New Ticket Created by  Peter du Marchie van Voorthuysen 
# Please include the string:  [perl #131887]
# in the subject line of all future correspondence about this issue. 
# https://rt.perl.org/Ticket/Display.html?id=131887 >


This is Rakudo version 2017.07 built on MoarVM version 2017.07
implementing Perl 6.c.

If the value of a Pair is a Scalar container, then the Pair can be
modified, e.g.

> my $value = 0;
0
> my $pair = number => $value;
number => 0
> $pair.value = 1; $pair;
number => 1

Method freeze make the value of the Pair read-only, by removing it from its
Scalar container, and returns the value.

> $pair.freeze;
1
> $pair.value = 2;
Cannot modify an immutable Int (1)
  in block  at  line 1

The problem is that freeze does more than that. It changes the object
identity as returned by WHICH as well:

> $pair = number => $value;
number => 1
> $pair.WHICH;
Pair|127791728
> $pair.freeze;
1
> $pair.WHICH;
Pair|Str|number|Int|1

Now by itself having a 2-tuple that is identified by its two elements is a
nice feature (if it would be documented). But _changing_ the object
identity is not consistent with the behavior of other built-in Perl 6
classes and actually breaks the implementation of some of these classes.

For example, a SetHash represents a mutable set. The Set method returns a
_new_ object that is immutable:

> $pair = number => $value;
number => 1
> my $set = SetHash.new($pair);
SetHash.new(number => 1)
> my $set2 = $set.Set;
set(number => 1)
> $set.WHICH;
SetHash|136539408
> $set2.WHICH;
Set|0EC3BFFD57719F5C6A3EE91A5EFAA5AEFE273964

But because freezing a Pair changes the identity of the _original_ object
it's possible to add a second instance of the _same_ Pair to the SetHash,
causing it to violate its contract:

> $pair.freeze;
1
> $set{$pair} = True;
True
> my ($a, $b) = $set.keys;
(number => 1 number => 1)
> $a === $b;
True

I think it's clear that changing the identity of the original object is not
correct. So I propose to remove the undocumented behavior of the freeze
method that it changes the object identity.

Now I can imagine that at some implementation level there are benefits to
having a kind of Pair that is identified by its key _and_ value. I also
think it could be generally useful to have a class implementing a truly
immutable (2-)tuple that is identified by its elements. But that should be
a separate class and the Pair class should provide a method to create a
_new_ object of this class from a Pair object.