Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

method freeze(Pair:D:) changes object identity #6442

Closed
p6rt opened this issue Aug 12, 2017 · 5 comments
Closed

method freeze(Pair:D:) changes object identity #6442

p6rt opened this issue Aug 12, 2017 · 5 comments

Comments

@p6rt
Copy link

p6rt commented Aug 12, 2017

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

Searchable as RT131887$

@p6rt
Copy link
Author

p6rt commented Aug 12, 2017

From p.dumarchie@gmail.com

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 <unit> at <unknown file> 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.

@p6rt
Copy link
Author

p6rt commented Aug 13, 2017

From @lizmat

Fixed with rakudo/rakudo@c229022cb0 , tests needed

On 12 Aug 2017, at 14​:36, Peter du Marchie van Voorthuysen (via RT) <perl6-bugs-followup@​perl.org> 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.
# <URL​: https://rt-archive.perl.org/perl6/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 <unit> at <unknown file> 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.

@p6rt
Copy link
Author

p6rt commented Aug 13, 2017

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

@p6rt
Copy link
Author

p6rt commented Mar 13, 2018

From @dogbert17

On Sun, 13 Aug 2017 15​:14​:41 -0700, elizabeth wrote​:

Fixed with rakudo/rakudo@c229022cb0 , tests
needed

On 12 Aug 2017, at 14​:36, Peter du Marchie van Voorthuysen (via RT)
<perl6-bugs-followup@​perl.org> 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.
# <URL​: https://rt-archive.perl.org/perl6/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 <unit> at <unknown file> 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 Raku/roast@d7d42d6c37

@p6rt p6rt closed this as completed Mar 13, 2018
@p6rt
Copy link
Author

p6rt commented Mar 13, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant