Bug #117838 for Type-Tiny: Bug in coercions for parameterized types

Wed Sep 14 14:47:29 2016 lyle.kopnicky [...] grantstreet.com - Ticket created

CC:	Steve Grazzini <grazzini [...] grantstreet.com>
Subject:	Bug in coercions for parameterized types
Date:	Wed, 14 Sep 2016 11:46:55 -0700
To:	bug-Type-Tiny [...] rt.cpan.org
From:	Lyle Kopnicky <lyle.kopnicky [...] grantstreet.com>

Hi, I've run into some unexpected behavior involving coercions from parameterized types. The attached script demonstrates the problem. What I'm expecting: ArrayRef[Bullwinkle] should be able to use the coercion that was added to Bullwinkle. This works with line 22 commented out. But surprisingly, just calling ArrayRef[Bullwinkle] (by uncommenting line 22) seems to freeze the coercions as they were before the coercion was added, even though each time ArrayRef[Bullwinkle] is called, it creates a new object. In our code base we have interface definitions for an API, that make use of type constraints. The interface definitions are used by both the client and server side. Some of the coercions, however, can only be included in the server-side code, because they involve accessing server-side data. So we add coercions as needed on the server side. This was working fine with the MooseX::Types::Moose library, but we wanted to switch to Type::Tiny because it has much better error messages. And because you get the auto-generated coercions for parameterized types. So, is this something that you consider a bug? Can it be fixed? Or should we be taking a different approach? Thanks for your help. -- *Lyle Kopnicky, **Perl Software Developer* *Grant Street Group*Ph: (412) 391-5555, Ext. 6817

Message body is not shown because sender requested not to inline it.

Thu May 18 08:29:42 2017 perl [...] toby.ink - Correspondence added

On 2016-09-14T19:47:29+01:00, lyle.kopnicky@grantstreet.com wrote: Show quoted text

> even though each time ArrayRef[Bullwinkle] is called, it creates a new > object.

It does? Have you checked? It shouldn't. perl -MScalar::Util=refaddr -MTypes::Standard=:all -E'my ($x,$y) = map ArrayRef[Int], 1..2; say refaddr($_) for $x, $y' Will output the same address twice. -Toby

Thu May 18 08:29:43 2017 The RT System itself - Status changed from 'new' to 'open'

Thu May 18 08:43:41 2017 perl [...] toby.ink - Correspondence added

And yeah, basically the situation is this. When you first create an ArrayRef[Bullwinkle], Type::Tiny calculates how the coercions work on ArrayRef[Bullwinkle] based on the coercions for Bullwinkle. Making changes to the coercions on Bullwinkle after that would invalidate the coercions for ArrayRef[Bullwinkle]. So that's why the coercions on Bullwinkle get frozen. And that's why it's best to define your coercions as close as possible to where the type itself is defined. It might be smarter for Bullwinkle to keep track of things that are depending on it like ArrayRef[Bullwinkle], and when a new coercion is added, tell them to update their coercions. I will add that to the wishlist. For your particular case, instead of adding the coercions at the server side towards the end, and risking trying to add to frozen coercions, you could add them as close as possible to where the type is defined but use: # detect whether we're running on the server use constant RUNNING_ON_SERVER => ($ENV{HOSTNAME} eq 'myserver'); ...; my $Bullwinkle = class_type('Bullwinkle'); $Bullwinkle->coercion->add_type_coercions( Int, 'Bullwinkle->new(favourite_number=>$_)' ) if RUNNING_ON_SERVER;

Thu May 18 08:44:07 2017 perl [...] toby.ink - Severity Wishlist added

Mon May 29 07:03:42 2017 perl [...] toby.ink - Correspondence added

This is probably going to be a rejection. I don't think it can be changed without sacrificing inlining of coercions, which is one of the major benefits of Type::Tiny over Moose's type constraint system. If you created an attribute, say, with: has creatures => (is => 'rw', isa => ArrayRef[Bullwinkle], coerce => 1); This can (if your object system supports it… Moo does) inline the Perl code for coercing integers to Bullwinkles straight into the constructor and accessor. If you later add a coercion to Bullwinkle, this would require the constructor and accessor to be re-built, and Type::Tiny has no way to signal to the class builder that this must be done. So yeah, add coercions to types as close as possible to where you define the type. I'm leaving this on the wishlist for now, but it's likely to become a reject.

Mon May 29 07:03:43 2017 perl [...] toby.ink - Status changed from 'open' to 'stalled'

Mon Jun 12 18:44:07 2017 lyle.kopnicky [...] grantstreet.com - Correspondence added

Subject:	Re: [rt.cpan.org #117838] Bug in coercions for parameterized types
Date:	Mon, 12 Jun 2017 15:38:15 -0700
To:	bug-Type-Tiny [...] rt.cpan.org
From:	Lyle Kopnicky <lyle.kopnicky [...] grantstreet.com>

It seems to me to be a matter of caching/performance vs. correctness. When the constraint ArrayRef[Bullwinkle] is constructed, it must be building a table of what types it can coerce from. It could instead dynamically ask the Bullwinkle class. So when a value is to be checked for coercibility, and it sees it's an array, but an item in the array isn't a Bullwinkle, it can ask the Bullwinkle type whether it can coerce that value, and if so, have it do so. On Mon, May 29, 2017 at 4:03 AM, Toby Inkster via RT < bug-Type-Tiny@rt.cpan.org> wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=117838 > > > This is probably going to be a rejection. > > I don't think it can be changed without sacrificing inlining of coercions, > which is one of the major benefits of Type::Tiny over Moose's type > constraint system. > > If you created an attribute, say, with: > > has creatures => (is => 'rw', isa => ArrayRef[Bullwinkle], coerce => 1); > > This can (if your object system supports it… Moo does) inline the Perl > code for coercing integers to Bullwinkles straight into the constructor and > accessor. > > If you later add a coercion to Bullwinkle, this would require the > constructor and accessor to be re-built, and Type::Tiny has no way to > signal to the class builder that this must be done. > > So yeah, add coercions to types as close as possible to where you define > the type. > > I'm leaving this on the wishlist for now, but it's likely to become a > reject. >

-- *Lyle Kopnicky, **Perl Software Developer* *Grant Street Group*Ph: (412) 391-5555, Ext. 6817

Mon Jun 12 18:44:08 2017 The RT System itself - Status changed from 'stalled' to 'open'

Sun Jun 18 08:47:58 2017 perl [...] toby.ink - Correspondence added

Yeah, it doesn't do that because that's hella slow. If you're coercing an arrayref of numbers into integers, it's a lot faster to do this: my $new = [ map int($_), @$old ]; Than this: my $new = [ map Int->coerce($_), @$old ]; Because the latter involves sub calls and method lookups and stuff. This is one of the reasons Type::Tiny is so much faster than Moose's type constraint system, especially for coercions. The cost of that is that you can't add and remove coercions later on, but doing that is something I'd argue is bad form anyway — it causes action-at-a-distance. For example, one class defines a coercion from Str to FileHandle by treating the string as a filename and opening it. Another class expects coercion from Str to FileHandle to be via IO::String. There's already stuff in the manual about this: https://metacpan.org/pod/distribution/Type-Tiny/lib/Type/Tiny/Manual/Coercions.pod#The-(Lack-of)-Zen-of-Coercions This is something I'd rather make it harder for people to do than slow down the library so that people can have bullets to aim at their feet.

Thu Jun 29 15:32:24 2017 lyle.kopnicky [...] grantstreet.com - Correspondence added

Subject:	Re: [rt.cpan.org #117838] Bug in coercions for parameterized types
Date:	Thu, 29 Jun 2017 12:24:35 -0700
To:	bug-Type-Tiny [...] rt.cpan.org
From:	Lyle Kopnicky <lyle.kopnicky [...] grantstreet.com>

Sounds like a reasonable tradeoff. Thanks for taking a look. On Sun, Jun 18, 2017 at 5:47 AM, Toby Inkster via RT < bug-Type-Tiny@rt.cpan.org> wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=117838 > > > Yeah, it doesn't do that because that's hella slow. > > If you're coercing an arrayref of numbers into integers, it's a lot faster > to do this: > > my $new = [ map int($_), @$old ]; > > Than this: > > my $new = [ map Int->coerce($_), @$old ]; > > Because the latter involves sub calls and method lookups and stuff. > > This is one of the reasons Type::Tiny is so much faster than Moose's type > constraint system, especially for coercions. > > The cost of that is that you can't add and remove coercions later on, but > doing that is something I'd argue is bad form anyway — it causes > action-at-a-distance. For example, one class defines a coercion from Str to > FileHandle by treating the string as a filename and opening it. Another > class expects coercion from Str to FileHandle to be via IO::String. > > There's already stuff in the manual about this: > > https://metacpan.org/pod/distribution/Type-Tiny/lib/ > Type/Tiny/Manual/Coercions.pod#The-(Lack-of)-Zen-of-Coercions > > This is something I'd rather make it harder for people to do than slow > down the library so that people can have bullets to aim at their feet. >

-- *Lyle Kopnicky, **Perl Software Developer* *Grant Street Group*Ph: (412) 391-5555, Ext. 6817

Thu Jul 05 05:57:57 2018 perl [...] toby.ink - Status changed from 'open' to 'rejected'