Bug #123385 for Moose: Native trait initializing attribute

Thu Oct 26 02:20:45 2017 joseluis.martinez [...] capside.com - Ticket created

Subject:	Native trait initializing attribute
Date:	Wed, 25 Oct 2017 22:44:56 +0000
To:	"bug-Moose [...] rt.cpan.org" <bug-Moose [...] rt.cpan.org>
From:	Jose Luis Martínez Torres <joseluis.martinez [...] capside.com>

Hi, I've discovered this behaviour. I'm reporting it as a bug because it seemed strange: #!/usr/bin/env perl use Test::More; package Test { use Moose; has prop => (is => 'ro', isa => 'HashRef', traits => [ 'Hash' ], handles => { count_prop => 'count' }); } my $instance = Test->new; ok(not(defined $instance->prop), 'prop is undefined'); $instance->count_prop; ok(not(defined $instance->prop), 'prop is undefined'); prop gets initialized to a {} when count_prop is called, which is unexpected (it breaks later comparisions for "definedness") Best Regards, Jose Luis

Thu Oct 26 14:53:56 2017 DROLSKY [...] cpan.org - Correspondence added

On 2017-10-26 01:20:45, joseluis.martinez@capside.com wrote: Show quoted text

> Hi, > > > I've discovered this behaviour. I'm reporting it as a bug because it > seemed strange: > > > #!/usr/bin/env perl > use Test::More; > package Test { > use Moose; > has prop => (is => 'ro', isa => 'HashRef', traits => [ 'Hash' ], > handles => { count_prop => 'count' }); > } > > my $instance = Test->new; > ok(not(defined $instance->prop), 'prop is undefined'); > $instance->count_prop; > ok(not(defined $instance->prop), 'prop is undefined'); > > prop gets initialized to a {} when count_prop is called, which is > unexpected (it breaks later comparisions for "definedness")

This is definitely a real issue. The fix is to have to tweak our Hash (and Array too) native trait code generation to check that the slot is populated before doing any further operations. This shouldn't too hard, just somewhat tedious because the code is a bit complex. If you wanted to help then updating the test suite to test that the various native trait methods don't auto-vivify the underlying reference would be very helpful!

Thu Oct 26 14:53:57 2017 The RT System itself - Status changed from 'new' to 'open'

Thu Oct 26 17:47:43 2017 joseluis.martinez [...] capside.com - Correspondence added

Subject:	Re: [rt.cpan.org #123385] Native trait initializing attribute
Date:	Thu, 26 Oct 2017 21:47:12 +0000
To:	"bug-Moose [...] rt.cpan.org" <bug-Moose [...] rt.cpan.org>
From:	Jose Luis Martínez Torres <joseluis.martinez [...] capside.com>

Hi, I've written the test here: https://github.com/moose/Moose/pull/153. Curiously, I've found that undefined attributes with Array delegated methods die. IMHO they should work like the Hash delegated methods (not dying, and returning 0, undef, etc). Hope it helps 😊 Jose Luis Martinez Show quoted text

________________________________ De: Dave Rolsky via RT <bug-Moose@rt.cpan.org> Enviado: jueves, 26 de octubre de 2017 20:53 Para: Jose Luis Martínez Torres Asunto: [rt.cpan.org #123385] Native trait initializing attribute <URL: https://rt.cpan.org/Ticket/Display.html?id=123385 > On 2017-10-26 01:20:45, joseluis.martinez@capside.com wrote:

> Hi, > > > I've discovered this behaviour. I'm reporting it as a bug because it > seemed strange: > > > #!/usr/bin/env perl > use Test::More; > package Test { > use Moose; > has prop => (is => 'ro', isa => 'HashRef', traits => [ 'Hash' ], > handles => { count_prop => 'count' }); > } > > my $instance = Test->new; > ok(not(defined $instance->prop), 'prop is undefined'); > $instance->count_prop; > ok(not(defined $instance->prop), 'prop is undefined'); > > prop gets initialized to a {} when count_prop is called, which is > unexpected (it breaks later comparisions for "definedness")

This is definitely a real issue. The fix is to have to tweak our Hash (and Array too) native trait code generation to check that the slot is populated before doing any further operations. This shouldn't too hard, just somewhat tedious because the code is a bit complex. If you wanted to help then updating the test suite to test that the various native trait methods don't auto-vivify the underlying reference would be very helpful!

Sat Nov 11 22:19:21 2017 DROLSKY [...] cpan.org - Correspondence added

I started looking at this some more and realize that how native trait delegations should handle unset attributes is really poorly defined. For getters returning undef or an empty list may make sense, but for setter methods it seems like an explicit exception is preferable. But then I wonder if we should throw an exception for _all_ types of methods. I'm trying to start a discussion on the #moose-dev IRC channel to see if anyone has thoughts.

Mon Nov 13 02:49:06 2017 jlmartinez [...] capside.com - Correspondence added

On Sat Nov 11 22:19:21 2017, DROLSKY wrote: Show quoted text

> I started looking at this some more and realize that how native trait > delegations should handle unset attributes is really poorly defined.

Yep! I started suspecting that when seeing how the test cases behave different for Arrays and Hashes Show quoted text

> For getters returning undef or an empty list may make sense, but for > setter methods it seems like an explicit exception is preferable.

I'm more of the opinion that setters should autovivify the attribute, finally doing what they are told to do without imposing a special case on the consumer (after all he is already saying that the attribute is a Hashref, so "set this key to this value" setter should just create the new Hash and set the key, or push onto an Array should create the new array and push to it). Show quoted text

> I'm trying to start a discussion on the #moose-dev IRC > channel to see if anyone has thoughts.

Thanks for starting the discussion! Best Regards, Jose Luis