fix accessor bugs

[gitmo/Moo.git] / lib / Method / Generate / Accessor.pm

diff --git a/lib/Method/Generate/Accessor.pm b/lib/Method/Generate/Accessor.pm
index 372cba9..7524528 100644 (file)
--- a/lib/Method/Generate/Accessor.pm
+++ b/lib/Method/Generate/Accessor.pm
@@ -18,9 +18,15 @@ BEGIN {
   ;
 }
 
+sub _die_overwrite
+{
+  my ($pkg, $method, $type) = @_;
+  die "You cannot overwrite a locally defined method ($method) with @{[ $type || 'an accessor' ]}";
+}
+
 sub generate_method {
   my ($self, $into, $name, $spec, $quote_opts) = @_;
-  $name =~ s/^\+//;
+  $spec->{allow_overwrite}++ if $name =~ s/^\+//;
   die "Must have an is" unless my $is = $spec->{is};
   if ($is eq 'ro') {
     $spec->{reader} = $name unless exists $spec->{reader};
@@ -85,6 +91,8 @@ sub generate_method {
 
   my %methods;
   if (my $reader = $spec->{reader}) {
+    _die_overwrite($into, $reader, 'a reader')
+      if !$spec->{allow_overwrite} && *{_getglob("${into}::${reader}")}{CODE};
     if (our $CAN_HAZ_XS && $self->is_simple_get($name, $spec)) {
       $methods{$reader} = $self->_generate_xs(
         getters => $into, $reader, $name, $spec
@@ -100,6 +108,8 @@ sub generate_method {
     }
   }
   if (my $accessor = $spec->{accessor}) {
+    _die_overwrite($into, $accessor, 'an accessor')
+      if !$spec->{allow_overwrite} && *{_getglob("${into}::${accessor}")}{CODE};
     if (
       our $CAN_HAZ_XS
       && $self->is_simple_get($name, $spec)
@@ -118,6 +128,8 @@ sub generate_method {
     }
   }
   if (my $writer = $spec->{writer}) {
+    _die_overwrite($into, $writer, 'a writer')
+      if !$spec->{allow_overwrite} && *{_getglob("${into}::${writer}")}{CODE};
     if (
       our $CAN_HAZ_XS
       && $self->is_simple_set($name, $spec)
@@ -135,6 +147,8 @@ sub generate_method {
     }
   }
   if (my $pred = $spec->{predicate}) {
+    _die_overwrite($into, $pred, 'a predicate')
+      if !$spec->{allow_overwrite} && *{_getglob("${into}::${pred}")}{CODE};
     $methods{$pred} =
       quote_sub "${into}::${pred}" =>
         '    '.$self->_generate_simple_has('$_[0]', $name, $spec)."\n"
@@ -144,6 +158,8 @@ sub generate_method {
     _install_coderef( "${into}::$spec->{builder}" => $spec->{builder_sub} );
   }
   if (my $cl = $spec->{clearer}) {
+    _die_overwrite($into, $cl, 'a clearer')
+      if !$spec->{allow_overwrite} && *{_getglob("${into}::${cl}")}{CODE};
     $methods{$cl} =
       quote_sub "${into}::${cl}" => 
         $self->_generate_simple_clear('$_[0]', $name, $spec)."\n"
@@ -163,12 +179,11 @@ sub generate_method {
         die "You gave me a handles of ${hspec} and I have no idea why";
       }
     };
-    foreach my $spec (@specs) {
-      my ($proxy, $target, @args) = @$spec;
+    foreach my $delegation_spec (@specs) {
+      my ($proxy, $target, @args) = @$delegation_spec;
+      _die_overwrite($into, $proxy, 'a delegation')
+        if !$spec->{allow_overwrite} && *{_getglob("${into}::${proxy}")}{CODE};
       $self->{captures} = {};
-      if ( *{_getglob("${into}::${proxy}")}{CODE} ) {
-        die "You cannot overwrite a locally defined method ($proxy) with a delegation";
-      }
       $methods{$proxy} =
         quote_sub "${into}::${proxy}" =>
           $self->_generate_delegation($asserter, $target, \@args),
@@ -179,16 +194,9 @@ sub generate_method {
   if (my $asserter = $spec->{asserter}) {
     $self->{captures} = {};
 
-    my $code = "do {\n"
-               ."  my \$val = ".$self->_generate_get($name, $spec).";\n"
-               ."  unless (".$self->_generate_simple_has('$_[0]', $name).") {\n"
-               .qq!    die "Attempted to access '${name}' but it is not set";\n!
-               ."  }\n"
-               ."  \$val;\n"
-               ."}\n";
 
     $methods{$asserter} =
-      quote_sub "${into}::${asserter}" => $code,
+      quote_sub "${into}::${asserter}" => $self->_generate_asserter($name, $spec),
         delete $self->{captures}
       ;
   }
@@ -224,15 +232,10 @@ sub _generate_get {
   if ($self->is_simple_get($name, $spec)) {
     $simple;
   } else {
-    'do { '.$self->_generate_use_default(
+    $self->_generate_use_default(
       '$_[0]', $name, $spec,
       $self->_generate_simple_has('$_[0]', $name, $spec),
-    ).'; '
-    .($spec->{isa}
-      ?($self->_generate_isa_check($name, $simple, $spec->{isa}).'; ')
-      :''
-    )
-    .$simple.' }';
+    );
   }
 }
 
@@ -262,9 +265,14 @@ sub _generate_use_default {
       $spec->{coerce}
     )
   }
-  $self->_generate_simple_set(
-    $me, $name, $spec, $get_value
-  ).' unless '.$test;
+  $test." ? \n"
+  .$self->_generate_simple_get($me, $name, $spec)."\n:"
+  .($spec->{isa}
+    ? "    do {\n      my \$value = ".$get_value.";\n"
+      ."      ".$self->_generate_isa_check($name, '$value', $spec->{isa}).";\n"
+      ."      ".$self->_generate_simple_set($me, $name, $spec, '$value')."\n"
+      ."    }\n"
+    : '    '.$self->_generate_simple_set($me, $name, $spec, $get_value)."\n");
 }
 
 sub _generate_get_default {
@@ -481,11 +489,11 @@ sub _generate_core_set {
 sub _generate_simple_set {
   my ($self, $me, $name, $spec, $value) = @_;
   my $name_str = perlstring $name;
+  my $simple = $self->_generate_core_set($me, $name, $spec, $value);
 
   if ($spec->{weak_ref}) {
-    $value = '$preserve = '.$value;
-    my $simple = $self->_generate_core_set($me, $name, $spec, $value);
     require Scalar::Util;
+    my $get = $self->_generate_simple_get($me, $name, $spec);
 
     # Perl < 5.8.3 can't weaken refs to readonly vars
     # (e.g. string constants). This *can* be solved by:
@@ -496,12 +504,10 @@ sub _generate_simple_set {
     #
     # but requires XS and is just too damn crazy
     # so simply throw a better exception
-    my $weak_simple = "my \$preserve; Scalar::Util::weaken(${simple}); no warnings 'void'; \$preserve";
+    my $weak_simple = "do { Scalar::Util::weaken(${simple}); no warnings 'void'; $get }";
     Moo::_Utils::lt_5_8_3() ? <<"EOC" : $weak_simple;
-
-      my \$preserve;
       eval { Scalar::Util::weaken($simple); 1 }
-        ? do { no warnings 'void'; \$preserve; }
+        ? do { no warnings 'void'; $get }
         : do {
           if( \$@ =~ /Modification of a read-only value attempted/) {
             require Carp;
@@ -515,7 +521,7 @@ sub _generate_simple_set {
         }
 EOC
   } else {
-    $self->_generate_core_set($me, $name, $spec, $value);
+    $simple;
   }
 }
 
@@ -525,6 +531,17 @@ sub _generate_getset {
     ."\n      : ".$self->_generate_get($name, $spec)."\n    )";
 }
 
+sub _generate_asserter {
+  my ($self, $name, $spec) = @_;
+
+  "do {\n"
+   ."  my \$val = ".$self->_generate_get($name, $spec).";\n"
+   ."  unless (".$self->_generate_simple_has('$_[0]', $name, $spec).") {\n"
+   .qq!    die "Attempted to access '${name}' but it is not set";\n!
+   ."  }\n"
+   ."  \$val;\n"
+   ."}\n";
+}
 sub _generate_delegation {
   my ($self, $asserter, $target, $args) = @_;
   my $arg_string = do {