Make sure weak attributes remain weak when cloning (Moose 2.0007)

diff --git a/Changes b/Changes
index acc6363..39a75a2 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,6 +1,10 @@
 Revision history for Mouse
 
-0.92
+0.93 2011-05-17 00:22:12
+    [BUG FIXES]
+    * Make sure weak attributes remain weak when cloning (Moose 2.0007)
+
+0.92 2011-4-14 23:37
     [BUG FIXES]
     * Replace C++-style comments (//) with C89-style comments(/* */)
       (RT #67412)

diff --git a/MANIFEST.SKIP b/MANIFEST.SKIP
index e3786a4..36b2576 100644 (file)
--- a/MANIFEST.SKIP
+++ b/MANIFEST.SKIP
@@ -61,4 +61,4 @@ ppport\.h$
 MYMETA\.yml$
 Moose-t/
 xshelper\.h$
-
+\.swp$

diff --git a/lib/Mouse/Meta/Method/Constructor.pm b/lib/Mouse/Meta/Method/Constructor.pm
index fe32939..610e98a 100644 (file)
--- a/lib/Mouse/Meta/Method/Constructor.pm
+++ b/lib/Mouse/Meta/Method/Constructor.pm
@@ -87,10 +87,6 @@ sub _generate_initialize_object {
             $post_process .= "\$checks[$index]->($instance_slot)\n";
             $post_process .= "  or $attr_var->_throw_type_constraint_error($instance_slot, $constraint_var);\n";
         }
-        if($is_weak_ref){
-            $post_process  = "Scalar::Util::weaken($instance_slot) "
-                             . "if ref $instance_slot;\n";
-        }
 
         # build cde for an attribute
         if (defined $init_arg) {
@@ -151,6 +147,11 @@ sub _generate_initialize_object {
 
         $code .= "}\n" if defined $init_arg;
 
+        if($is_weak_ref){
+            $code .= "Scalar::Util::weaken($instance_slot) "
+                   . "if ref $instance_slot;\n";
+        }
+
         push @res, $code;
     }
 

diff --git a/t/020_attributes/036_clone_weak.t b/t/020_attributes/036_clone_weak.t
new file mode 100644 (file)
index 0000000..db42d7e
--- /dev/null
+++ b/t/020_attributes/036_clone_weak.t
@@ -0,0 +1,181 @@
+use strict;
+use warnings;
+use Test::More;
+
+{
+    package Foo;
+    use Mouse;
+
+    has bar => (
+        is       => 'ro',
+        weak_ref => 1,
+    );
+}
+
+{
+    package MyScopeGuard;
+
+    sub new {
+        my ($class, $cb) = @_;
+        bless { cb => $cb }, $class;
+    }
+
+    sub DESTROY { shift->{cb}->() }
+}
+
+{
+    my $destroyed = 0;
+
+    my $foo = do {
+        my $bar = MyScopeGuard->new(sub { $destroyed++ });
+        my $foo = Foo->new({ bar => $bar });
+        my $clone = $foo->meta->clone_object($foo);
+
+        is $destroyed, 0;
+
+        $clone;
+    };
+
+    isa_ok($foo, 'Foo');
+    is $foo->bar, undef;
+    is $destroyed, 1;
+}
+
+{
+    my $clone;
+    {
+        my $anon = Mouse::Meta::Class->create_anon_class;
+
+        my $foo = $anon->new_object;
+        isa_ok($foo, $anon->name);
+        ok(Mouse::Util::class_of($foo), "has a metaclass");
+
+        $clone = $anon->clone_object($foo);
+        isa_ok($clone, $anon->name);
+        ok(Mouse::Util::class_of($clone), "has a metaclass");
+    }
+
+    ok(Mouse::Util::class_of($clone), "still has a metaclass");
+}
+
+=pod
+
+{
+    package Foo::Meta::Attr::Trait;
+    use Mouse::Role;
+
+    has value_slot => (
+        is      => 'ro',
+        isa     => 'Str',
+        lazy    => 1,
+        default => sub { shift->name },
+    );
+
+    has count_slot => (
+        is      => 'ro',
+        isa     => 'Str',
+        lazy    => 1,
+        default => sub { '<<COUNT>>' . shift->name },
+    );
+
+    sub slots {
+        my $self = shift;
+        return ($self->value_slot, $self->count_slot);
+    }
+
+    sub _set_count {
+        my $self = shift;
+        my ($instance) = @_;
+        my $mi = $self->associated_class->get_meta_instance;
+        $mi->set_slot_value(
+            $instance,
+            $self->count_slot,
+            ($mi->get_slot_value($instance, $self->count_slot) || 0) + 1,
+        );
+    }
+
+    sub _clear_count {
+        my $self = shift;
+        my ($instance) = @_;
+        $self->associated_class->get_meta_instance->deinitialize_slot(
+            $instance, $self->count_slot
+        );
+    }
+
+    sub has_count {
+        my $self = shift;
+        my ($instance) = @_;
+        $self->associated_class->get_meta_instance->has_slot_value(
+            $instance, $self->count_slot
+        );
+    }
+
+    sub count {
+        my $self = shift;
+        my ($instance) = @_;
+        $self->associated_class->get_meta_instance->get_slot_value(
+            $instance, $self->count_slot
+        );
+    }
+
+    after set_initial_value => sub {
+        shift->_set_count(@_);
+    };
+
+    after set_value => sub {
+        shift->_set_count(@_);
+    };
+
+    around _inline_instance_set => sub {
+        my $orig = shift;
+        my $self = shift;
+        my ($instance) = @_;
+
+        my $mi = $self->associated_class->get_meta_instance;
+
+        return 'do { '
+                 . $mi->inline_set_slot_value(
+                       $instance,
+                       $self->count_slot,
+                       $mi->inline_get_slot_value(
+                           $instance, $self->count_slot
+                       ) . ' + 1'
+                   ) . ';'
+                 . $self->$orig(@_)
+             . '}';
+    };
+
+    after clear_value => sub {
+        shift->_clear_count(@_);
+    };
+}
+
+{
+    package Bar;
+    use Mouse;
+    Mouse::Util::MetaRole::apply_metaroles(
+        for => __PACKAGE__,
+        class_metaroles => {
+            attribute => ['Foo::Meta::Attr::Trait'],
+        },
+    );
+
+    has baz => ( is => 'rw' );
+}
+
+{
+    my $attr = Bar->meta->find_attribute_by_name('baz');
+
+    my $bar = Bar->new(baz => 1);
+    is($attr->count($bar), 1, "right count");
+
+    $bar->baz(2);
+    is($attr->count($bar), 2, "right count");
+
+    my $clone = $bar->meta->clone_object($bar);
+    is($attr->count($clone), $attr->count($bar), "right count");
+}
+
+=cut
+
+done_testing;

diff --git a/xs-src/Mouse.xs b/xs-src/Mouse.xs
index 10f6c09..1b124c7 100644 (file)
--- a/xs-src/Mouse.xs
+++ b/xs-src/Mouse.xs
@@ -315,7 +315,7 @@ mouse_class_initialize_object(pTHX_ SV* const meta, SV* const object, HV* const
                 value = mouse_xa_apply_type_constraint(aTHX_ xa, value, flags);
             }
             value = set_slot(object, slot, value);
-            if(SvROK(value) && flags & MOUSEf_ATTR_IS_WEAK_REF){
+            if(flags & MOUSEf_ATTR_IS_WEAK_REF && SvROK(value)){
                 weaken_slot(object, slot);
             }
             if(flags & MOUSEf_ATTR_HAS_TRIGGER){
@@ -337,8 +337,16 @@ mouse_class_initialize_object(pTHX_ SV* const meta, SV* const object, HV* const
                     mouse_xa_set_default(aTHX_ xa, object);
                 }
             }
-            /* don't check while cloning (or reblesseing) */
-            else if(!is_cloning && flags & MOUSEf_ATTR_IS_REQUIRED) {
+            else if(is_cloning) {
+                if(flags & MOUSEf_ATTR_IS_WEAK_REF){
+                    SV* const value = get_slot(object, slot);
+                    if(SvROK(value)) {
+                        weaken_slot(object, slot);
+                    }
+                }
+            }
+            /* don't check "required" while cloning (or reblesseing) */
+            else if(flags & MOUSEf_ATTR_IS_REQUIRED) {
                 mouse_throw_error(attr, NULL, "Attribute (%"SVf") is required", slot);
             }
         }