[PATCH] overload: nomethod(..., '!') return value inverted #9784

p5pRT · 2009-07-02T06:34:48Z

Migrated from rt.perl.org#67156 (status was 'resolved')

Searchable as RT67156$

p5pRT · 2009-07-02T06:34:51Z

From perl@mbreen.com

use overload nomethod => sub { print "nomethod('$_[3]')"; 1; };
print " returns ", ! bless([], 'main') ? "true\n" : "false!\n";
# 5.8.9 outputs: nomethod('!') returns false!
# correct: nomethod('!') returns true

This relates to bug 41546, the fix for which corrected the
same error specifically for the comparison operators.

Patch attached is a general fix for the problem.

Flags:
category=core
severity=medium

perl -V omitted (tested and patched against perl-current -
not installed version)

--

p5pRT · 2009-07-02T06:34:51Z

From perl@mbreen.com

nomethod_not.patch

--- perl-current/lib/overload.t	2009-07-02 05:55:46.000000000 +0100
+++ perl-current_nomethod_not/lib/overload.t	2009-07-02 05:26:24.000000000 +0100
@@ -47,7 +47,7 @@ sub numify { 0 + "${$_[0]}" }	# Not need
 package main;
 
 $| = 1;
-use Test::More tests => 577;
+use Test::More tests => 579;
 
 
 $a = new Oscalar "087";
@@ -1335,7 +1335,7 @@ foreach my $op (qw(<=> == != < <= > >=))
 }
 
 {
-    # comparison operators with nomethod
+    # comparison operators with nomethod (bug 41546)
     my $warning = "";
     my $method;
 
@@ -1382,6 +1382,21 @@ foreach my $op (qw(<=> == != < <= > >=))
 }
 
 {
+    # nomethod called for '!' after attempted fallback
+    my $nomethod_called = 0;
+
+    package nomethod_not;
+    use overload nomethod => sub { $nomethod_called = 'yes'; };
+
+    package main;
+    my $o = bless [], 'nomethod_not';
+    my $res = ! $o;
+
+    is($nomethod_called, 'yes', "nomethod() is called for '!'");
+    is($res, 'yes', "nomethod(..., '!') return value propagates");
+}
+
+{
     # Subtle bug pre 5.10, as a side effect of the overloading flag being
     # stored on the reference rather than the referent. Despite the fact that
     # objects can only be accessed via references (even internally), the
--- perl-current/gv.c	2009-07-02 05:56:12.000000000 +0100
+++ perl-current_nomethod_not/gv.c	2009-07-02 06:26:22.000000000 +0100
@@ -16,7 +16,7 @@
  * history of Middle-earth and Over-heaven and of the Sundering Seas,'
  * laughed Pippin.
  *
- *     [p.599 of _The Lord of the Rings_, III/xi: "The Palant�r"]
+ *     [p.599 of _The Lord of the Rings_, III/xi: "The Palantir"]
  */
 
 /*
@@ -1930,7 +1930,8 @@ Perl_amagic_call(pTHX_ SV *left, SV *rig
            (void)((cv = cvp[off=bool__amg])
                   || (cv = cvp[off=numer_amg])
                   || (cv = cvp[off=string_amg]));
-           postpr = 1;
+           if (cv)
+               postpr = 1;
            break;
 	 case copy_amg:
 	   {
@@ -2034,35 +2035,24 @@ Perl_amagic_call(pTHX_ SV *left, SV *rig
 	 case ge_amg:
 	 case eq_amg:
 	 case ne_amg:
-	   postpr = 1; off=ncmp_amg; break;
+             off = ncmp_amg;
+             break;
 	 case slt_amg:
 	 case sle_amg:
 	 case sgt_amg:
 	 case sge_amg:
 	 case seq_amg:
 	 case sne_amg:
-	   postpr = 1; off=scmp_amg; break;
+             off = scmp_amg;
+             break;
 	 }
-      if (off != -1) cv = cvp[off];
-      if (!cv) {
-	goto not_found;
-      }
+      if ((off != -1) && (cv = cvp[off]))
+          postpr = 1;
+      else
+          goto not_found;
     } else {
     not_found:			/* No method found, either report or croak */
       switch (method) {
-	 case lt_amg:
-	 case le_amg:
-	 case gt_amg:
-	 case ge_amg:
-	 case eq_amg:
-	 case ne_amg:
-	 case slt_amg:
-	 case sle_amg:
-	 case sgt_amg:
-	 case sge_amg:
-	 case seq_amg:
-	 case sne_amg:
-	   postpr = 0; break;
 	 case to_sv_amg:
 	 case to_av_amg:
 	 case to_hv_amg:

p5pRT · 2009-11-11T10:27:17Z

sgc294@internode.on.net - Status changed from 'new' to 'open'

p5pRT · 2009-12-11T18:33:21Z

From perl@mbreen.com

Updated patch attached, in git format.

This is just one of several problems in overload I found around
the same time. I would be willing to write reports and patches
for them, time permitting, after (if) this patch is applied (the
patches would be progressive; and if nobody has time for this
one, the simplest, ...)

p5pRT · 2009-12-11T18:33:21Z

From perl@mbreen.com

0001-fix-bug-67156-overload-nomethod.patch

From 7e3e0667bb24d769118981af2a6b045ba92a11b0 Mon Sep 17 00:00:00 2001
From: Michael Breen <perl@mbreen.com>
Date: Fri, 11 Dec 2009 17:48:51 +0000
Subject: [PATCH] fix bug 67156: overload: nomethod(..., '!') return value inverted

---
 gv.c           |   32 +++++++++++---------------------
 lib/overload.t |   19 +++++++++++++++++--
 2 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/gv.c b/gv.c
index 932b2b8..b969ce2 100644
--- a/gv.c
+++ b/gv.c
@@ -16,7 +16,7 @@
  * history of Middle-earth and Over-heaven and of the Sundering Seas,'
  * laughed Pippin.
  *
- *     [p.599 of _The Lord of the Rings_, III/xi: "The Palant�r"]
+ *     [p.599 of _The Lord of the Rings_, III/xi: "The Palantir"]
  */
 
 /*
@@ -1902,7 +1902,8 @@ Perl_amagic_call(pTHX_ SV *left, SV *right, int method, int flags)
            (void)((cv = cvp[off=bool__amg])
                   || (cv = cvp[off=numer_amg])
                   || (cv = cvp[off=string_amg]));
-           postpr = 1;
+           if (cv)
+               postpr = 1;
            break;
 	 case copy_amg:
 	   {
@@ -2007,35 +2008,24 @@ Perl_amagic_call(pTHX_ SV *left, SV *right, int method, int flags)
 	 case ge_amg:
 	 case eq_amg:
 	 case ne_amg:
-	   postpr = 1; off=ncmp_amg; break;
+             off = ncmp_amg;
+             break;
 	 case slt_amg:
 	 case sle_amg:
 	 case sgt_amg:
 	 case sge_amg:
 	 case seq_amg:
 	 case sne_amg:
-	   postpr = 1; off=scmp_amg; break;
+             off = scmp_amg;
+             break;
 	 }
-      if (off != -1) cv = cvp[off];
-      if (!cv) {
-	goto not_found;
-      }
+      if ((off != -1) && (cv = cvp[off]))
+          postpr = 1;
+      else
+          goto not_found;
     } else {
     not_found:			/* No method found, either report or croak */
       switch (method) {
-	 case lt_amg:
-	 case le_amg:
-	 case gt_amg:
-	 case ge_amg:
-	 case eq_amg:
-	 case ne_amg:
-	 case slt_amg:
-	 case sle_amg:
-	 case sgt_amg:
-	 case sge_amg:
-	 case seq_amg:
-	 case sne_amg:
-	   postpr = 0; break;
 	 case to_sv_amg:
 	 case to_av_amg:
 	 case to_hv_amg:
diff --git a/lib/overload.t b/lib/overload.t
index d54068e..39333cf 100644
--- a/lib/overload.t
+++ b/lib/overload.t
@@ -47,7 +47,7 @@ sub numify { 0 + "${$_[0]}" }	# Not needed, additional overhead
 package main;
 
 $| = 1;
-use Test::More tests => 605;
+use Test::More tests => 607;
 
 
 $a = new Oscalar "087";
@@ -1420,7 +1420,7 @@ foreach my $op (qw(<=> == != < <= > >=)) {
 }
 
 {
-    # comparison operators with nomethod
+    # comparison operators with nomethod (bug 41546)
     my $warning = "";
     my $method;
 
@@ -1467,6 +1467,21 @@ foreach my $op (qw(<=> == != < <= > >=)) {
 }
 
 {
+    # nomethod called for '!' after attempted fallback
+    my $nomethod_called = 0;
+
+    package nomethod_not;
+    use overload nomethod => sub { $nomethod_called = 'yes'; };
+
+    package main;
+    my $o = bless [], 'nomethod_not';
+    my $res = ! $o;
+
+    is($nomethod_called, 'yes', "nomethod() is called for '!'");
+    is($res, 'yes', "nomethod(..., '!') return value propagates");
+}
+
+{
     # Subtle bug pre 5.10, as a side effect of the overloading flag being
     # stored on the reference rather than the referent. Despite the fact that
     # objects can only be accessed via references (even internally), the
-- 
1.5.6.5

p5pRT · 2009-12-11T18:33:22Z

From [Unknown Contact. See original ticket]

Updated patch attached, in git format.

This is just one of several problems in overload I found around
the same time. I would be willing to write reports and patches
for them, time permitting, after (if) this patch is applied (the
patches would be progressive; and if nobody has time for this
one, the simplest, ...)

p5pRT · 2009-12-14T14:39:13Z

@rgs - Status changed from 'open' to 'resolved'

p5pRT · 2009-12-14T14:39:46Z

From @rgarcia

2009/12/11 Michael Breen via RT <perlbug-comment@perl.org>:

Updated patch attached, in git format.

Thanks, applied as 2ab54ef to bleadperl.

This is just one of several problems in overload I found around
the same time. I would be willing to write reports and patches
for them, time permitting, after (if) this patch is applied (the
patches would be progressive; and if nobody has time for this
one, the simplest, ...)

No problem, and thanks for your will to contribute. As long as you
send self-contained patches with nice commit messages and regression
tests, they won't be too hard to review and apply/reject :)

p5pRT closed this as completed Dec 14, 2009

p5pRT added the Severity Low label Oct 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PATCH] overload: nomethod(..., '!') return value inverted #9784

[PATCH] overload: nomethod(..., '!') return value inverted #9784

p5pRT commented Jul 2, 2009

p5pRT commented Jul 2, 2009

p5pRT commented Jul 2, 2009

p5pRT commented Nov 11, 2009

p5pRT commented Dec 11, 2009

p5pRT commented Dec 11, 2009

p5pRT commented Dec 11, 2009

p5pRT commented Dec 14, 2009

p5pRT commented Dec 14, 2009

[PATCH] overload: nomethod(..., '!') return value inverted #9784

[PATCH] overload: nomethod(..., '!') return value inverted #9784

Comments

p5pRT commented Jul 2, 2009

p5pRT commented Jul 2, 2009

From perl@mbreen.com

p5pRT commented Jul 2, 2009

From perl@mbreen.com

p5pRT commented Nov 11, 2009

p5pRT commented Dec 11, 2009

From perl@mbreen.com

p5pRT commented Dec 11, 2009

From perl@mbreen.com

p5pRT commented Dec 11, 2009

From [Unknown Contact. See original ticket]

p5pRT commented Dec 14, 2009

p5pRT commented Dec 14, 2009

From @rgarcia