Bug #125659 for Moo: isa coderefs are able to alter the value they're passed

Sat Jun 23 04:52:48 2018 perl [...] toby.ink - Ticket created

Subject:

isa coderefs are able to alter the value they're passed

use v5.14; use strict; use warnings; package Foo { use Moo; has fff => (is => 'ro', isa => sub { $_[0]++ }); } my $foo = Foo->new(fff => 41); say $foo->fff; # says 42

Tue Jun 26 04:41:59 2018 perl [...] toby.ink - Correspondence added

use v5.14; use strict; use warnings; package Foo { use Moo; use Sub::Quote qw( quote_sub ); has fff => (is => 'ro', isa => quote_sub q{ $_[0]++ }); } my $foo = Foo->new(fff => 41); say $foo->fff; # says 41

Tue Jun 26 04:48:15 2018 perl [...] toby.ink - Correspondence added

use v5.14; use strict; use warnings; package Foo { use Moo; use Sub::Quote qw( quote_sub ); has fff => (is => 'ro', isa => quote_sub q{ my ($value) = @_; $value++ }); } my $foo = Foo->new(fff => 41); say $foo->fff; # says 42

Tue Jun 26 04:52:46 2018 perl [...] toby.ink - Correspondence added

Best solution might be just to document it. Something like: For efficiency, Moo will sometimes avoid copying a value before passing it to the isa check. This means the isa coderef can inadvertently modify the value that is used to construct the object. Do not rely on this behaviour as it is inconsistent and may change from version to version.

Tue Jun 26 06:32:00 2018 haarg [...] haarg.org - Correspondence added

On Tue Jun 26 04:52:46 2018, TOBYINK wrote: Show quoted text

> Best solution might be just to document it. Something like: > > For efficiency, Moo will sometimes avoid copying a value before > passing it to the isa check. This means the isa coderef can > inadvertently modify the value that is used to construct the object. > Do not rely on this behaviour as it is inconsistent and may change > from version to version.

I think that seems reasonable. It will always be possible to modify the value being checked in some cases (references) so I don't think there is a strong case for changing the current behavior. If someone does come up with a more concrete problem with this, it can always be revisited.

Tue Jun 26 06:32:02 2018 The RT System itself - Status changed from 'new' to 'open'

Mon Jul 02 05:41:16 2018 perl [...] toby.ink - Correspondence added

For reference, this is Moose's behaviour: Inlined type constraints *can* alter the value. Non-inlined type constraints can't. (Or at least, I haven't been able to trick them into changing the value.) perl -E'package Foo { use Moose; use Moose::Util::TypeConstraints; subtype "NonNeg", as "Int", where { ++$_[0] > 0 }, inline_as { "++$_[1] > 0" }; has foo => (is => "ro", isa => "NonNeg"); __PACKAGE__->meta->make_immutable}; say Foo->new(foo => 41)->foo'

Mon Jul 02 12:56:08 2018 karen [...] froods.org - Correspondence added

Subject:	Re: [rt.cpan.org #125659] isa coderefs are able to alter the value they're passed
Date:	Mon, 2 Jul 2018 09:55:56 -0700
To:	bug-Moo [...] rt.cpan.org
From:	Karen Etheridge <karen [...] froods.org>

Especially since whether a type is inlined should be invisible to the user, I would argue this is unintentional in Moose, and we should fix it there as well. On Mon, Jul 2, 2018 at 2:41 AM, Toby Inkster via RT <bug-Moo@rt.cpan.org> wrote: Show quoted text

> Queue: Moo > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=125659 > > > For reference, this is Moose's behaviour: > > Inlined type constraints *can* alter the value. > > Non-inlined type constraints can't. (Or at least, I haven't been able to > trick them into changing the value.) > > > perl -E'package Foo { use Moose; use Moose::Util::TypeConstraints; subtype > "NonNeg", as "Int", where { ++$_[0] > 0 }, inline_as { "++$_[1] > 0" }; has > foo => (is => "ro", isa => "NonNeg"); __PACKAGE__->meta->make_immutable}; > say Foo->new(foo => 41)->foo' >