Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rakudo cannot take the minimum of undef #552

Closed
p6rt opened this issue Dec 30, 2008 · 13 comments
Closed

Rakudo cannot take the minimum of undef #552

p6rt opened this issue Dec 30, 2008 · 13 comments

Comments

@p6rt
Copy link

p6rt commented Dec 30, 2008

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

Searchable as RT61836$

@p6rt
Copy link
Author

p6rt commented Dec 30, 2008

From @masak

Rakudo r34628​:

$ perl6 -e 'undef min 2'
Multiple Dispatch​: No suitable candidate found for 'cmp', with signature 'PP->I'
[...]

@p6rt
Copy link
Author

p6rt commented Jan 5, 2009

From @moritz

Carl MXXsak (via RT) wrote​:

# New Ticket Created by "Carl Mäsak"
# Please include the string​: [perl #​61836]
# in the subject line of all future correspondence about this issue.
# <URL​: http://rt.perl.org/rt3/Ticket/Display.html?id=61836 >

Rakudo r34628​:

$ perl6 -e 'undef min 2'
Multiple Dispatch​: No suitable candidate found for 'cmp', with signature 'PP->I'
[...]

Added tests to t/spec/S03-operators/misc.t (but I don't know what the
result should be, I just tested with lives_ok).

Cheers,
Moritz

@p6rt
Copy link
Author

p6rt commented Jan 5, 2009

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

p6rt commented Feb 14, 2009

From @bacek

On Mon Jan 05 11​:53​:29 2009, moritz wrote​:

Carl MXXsak (via RT) wrote​:

# New Ticket Created by "Carl Mäsak"
# Please include the string​: [perl #​61836]
# in the subject line of all future correspondence about this issue.
# <URL​: http://rt.perl.org/rt3/Ticket/Display.html?id=61836 >

Rakudo r34628​:

$ perl6 -e 'undef min 2'
Multiple Dispatch​: No suitable candidate found for 'cmp', with
signature 'PP->I'
[...]

Added tests to t/spec/S03-operators/misc.t (but I don't know what the
result should be, I just tested with lives_ok).

Cheers,
Moritz

Part of this bug fixed in #​61840.
Part of this bug related to Parrot absence of VTABLE_cmp for "undef".

Attached patch adds VTABLE_cmp for Undef in Parrot. Other approach is to
add .sub 'cmp' :multi('Failure',_) to Rakudo. I can't judge with one is
better.

--
Bacek

@p6rt
Copy link
Author

p6rt commented Feb 14, 2009

From @bacek

undef.patch
commit 948c9412c390b86cf68eaf5cc76525e050a1e100
Author: Vasily Chekalkin <bacek@bacek.com>
Date:   Sat Feb 14 18:24:13 2009 +1100

    Implement Undef.cmp

diff --git a/src/pmc/undef.pmc b/src/pmc/undef.pmc
index ed949a0..0bde497 100644
--- a/src/pmc/undef.pmc
+++ b/src/pmc/undef.pmc
@@ -245,6 +245,25 @@ Returns 1 if the C<*value> is an Undef PMC, 0 otherwise.
     MULTI INTVAL is_equal(DEFAULT value) {
         return 0;
     }
+
+/*
+
+=item C<INTVAL cmp(PMC *value)>
+
+Returns 1 if the C<*value> is an Undef PMC, otherwise delegate comparition to value.
+
+=cut
+
+*/
+
+    MULTI INTVAL cmp(Undef value) {
+        return 1;
+    }
+
+    MULTI INTVAL cmp(DEFAULT value) {
+        return VTABLE_cmp(INTERP, value, SELF);
+    }
+
 }
 
 /*

@p6rt
Copy link
Author

p6rt commented Feb 16, 2009

From @ronaldxs

As demonstrated by the test script and output below, run with the
proposed patch applied, the patch proposed on 2/14 doesn't quite result
in the right answers. Note in the output that "2 min undef"
incorrectly yields 2 and fails to give an "uninitialized value"
warning. This can be partially fixed by changing the return values
from the multies to 'return 0;' and 'return -1 * VTABLE_cmp(INTERP,
value, SELF);' respectively as provided in undef_almost_fix.patch. The
revised fix still does not, however, always address "uninitialized
value" warnings for reasons described in parrot track #​285
(https://trac.parrot.org/parrot/ticket/285).  I have attached a patch,
Failure.patch, that seems to properly fix the problem by modifying
Failure.pir. Please apply either Failure.patch or
undef_almost_fix.patch and NOT both. The failure_misc.patch file
updates t/spec/S03-operators/misc.t to enable some enhanced versions of
the relevant tests. It also fixes a minor problem in the min operator
of List.pir that makes the operator do extra work that is not done by
the min function in any-list.pir and can cause extra
unneeded 'uninitialized value' warnings when min is used with undef.
The patches do not seem to break any spectest_regression tests.

Test script​: *******************
say 'next step is 2 min undef';
my $x = 2 min undef;
say 'done 2 min undef';
if (defined $x) {
  say "x wrongly defined with value​: $x"
}
else {
  say "x correctly undefined"
}

say 'next step is undef min 2';
$x = undef min 2;
say 'done undef min 2';
if (defined $x) {
  say "x wrongly defined with value​: $x"
}
else {
  say "x correctly undefined"
}

Test script output​: *******************
next step is 2 min undef
done 2 min undef
x wrongly defined with value​: 2
next step is undef min 2
Use of uninitialized value
done undef min 2
x correctly undefined

Ron

@p6rt
Copy link
Author

p6rt commented Feb 16, 2009

From @ronaldxs

undef_almost_fix.patch
Index: src/pmc/undef.pmc
===================================================================
--- src/pmc/undef.pmc	(revision 36800)
+++ src/pmc/undef.pmc	(working copy)
@@ -245,6 +245,28 @@
     MULTI INTVAL is_equal(DEFAULT value) {
         return 0;
     }
+    
+
+/*
+
+=item C<INTVAL cmp(PMC *value)>
+
+Returns 1 if the C<*value> is an Undef PMC, otherwise delegate comparition to value.
+
+=cut
+
+*/
+
+
+    MULTI INTVAL cmp(Undef value) {
+        return 0;
+    }
+
+    MULTI INTVAL cmp(DEFAULT value) {
+        return  -1 * VTABLE_cmp(INTERP, value, SELF);
+    }
+
+    
 }
 
 /*

@p6rt
Copy link
Author

p6rt commented Feb 16, 2009

From @ronaldxs

failure_misc.patch
diff --git a/src/classes/List.pir b/src/classes/List.pir
index 4375e6f..6c3ba7d 100644
--- a/src/classes/List.pir
+++ b/src/classes/List.pir
@@ -571,8 +571,8 @@ have_args:
 
     # Find minimum.
     .local pmc cur_min, it
-    cur_min = args[0]
     it = iter args
+    cur_min = shift it
 find_min_loop:
     unless it goto find_min_loop_end
     $P0 = shift it
Index: t/spec/S03-operators/misc.t
===================================================================
--- t/spec/S03-operators/misc.t	(revision 25359)
+++ t/spec/S03-operators/misc.t	(working copy)
@@ -124,8 +124,8 @@
 
 # L<S03/Tight or precedence/"any value of any type may be compared with +Inf
 # or -Inf values">
-#?rakudo todo 'RT #61836'
 {
-    lives_ok { (2 min undef) }, 'can do (2 min undef)'; 
-    lives_ok { (undef min 2) }, 'can do (undef min 2)'; 
+  # from 'RT #61836'
+  ok ( not defined (2 min undef), 'can do (2 min undef)');
+  ok ( not defined (undef min 2), 'can do (undef min 2)');
 }

@p6rt
Copy link
Author

p6rt commented Feb 16, 2009

From @ronaldxs

Failure.patch
diff --git a/src/classes/Failure.pir b/src/classes/Failure.pir
index 7a78d5a..7b4a8e7 100644
--- a/src/classes/Failure.pir
+++ b/src/classes/Failure.pir
@@ -124,6 +124,42 @@
     .return (self)
 .end
 
+=comment
+
+The cmp and cmp_num vtable methods below fix problems that really
+should be fixed in the parrot undef.pmc file when parrot trac #285
+is fixed because C<'abc' cmp undef> works through the parrot pmc
+level now.  These methods enable/fix C<undef cmp 'abc'>,
+C<undef E<lt>=E<gt> 2>, etc. more or less correctly for the time being.
+
+=cut
+
+.sub 'cmp' :vtable :method
+    .param pmc value
+    $I0 = isa value, 'Failure'
+    if $I0 goto ret_zero
+    $I0 = cmp value, self
+    $I0 = - $I0
+    .return ($I0)
+ret_zero:
+    $S0 = self      # exception (string here might change but OK for now)
+    $S0 = value     # exception (string here might change but OK for now)
+    .return (0)    
+.end
+
+.sub 'cmp_num' :vtable :method
+    .param pmc value
+    $I0 = isa value, 'Failure'
+    if $I0 goto ret_zero
+    $I0 = cmp_num value, self
+    $I0 = - $I0
+    .return ($I0)
+ret_zero:
+    $I0 = self      # exception
+    $I0 = value     # exception
+    .return (0)    
+.end
+
 
 # Local Variables:
 #   mode: pir

@p6rt
Copy link
Author

p6rt commented Feb 19, 2009

From @masak

<masak> rakudo​: undef min 100
<p6eval> rakudo 543e22​: OUTPUT«Multiple Dispatch​: No suitable
candidate found for 'cmp', with signature 'PP->I' [...]

Expected result​: undef, I think. Unless the 'min' numifies the result
and it becomes 0. Whatever.

@p6rt
Copy link
Author

p6rt commented Apr 4, 2009

From @ronaldxs

On Thu Feb 19 05​:27​:45 2009, masak wrote​:

<masak> rakudo​: undef min 100
<p6eval> rakudo 543e22​: OUTPUT«Multiple Dispatch​: No suitable
candidate found for 'cmp', with signature 'PP->I' [...]

Expected result​: undef, I think. Unless the 'min' numifies the result
and it becomes 0. Whatever.

With my proposed patches applied on my system​:

$ ./perl6 -e 'my $x = undef min 100; say $x.perl'
Use of uninitialized value
undef

@p6rt
Copy link
Author

p6rt commented Mar 9, 2010

From @moritz

$ ./perl6 -e 'say Any min 2'
2

you will be pleased...

@p6rt
Copy link
Author

p6rt commented Mar 9, 2010

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

@p6rt p6rt closed this as completed Mar 9, 2010
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant