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

some array autovivification fixes/improvements #669

Closed
p6rt opened this issue Jan 31, 2009 · 4 comments
Closed

some array autovivification fixes/improvements #669

p6rt opened this issue Jan 31, 2009 · 4 comments
Labels

Comments

@p6rt
Copy link

p6rt commented Jan 31, 2009

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

Searchable as RT62948$

@p6rt
Copy link
Author

p6rt commented Jan 31, 2009

From @ronaldxs

This is a follow up to rt #​61324 that observed, among other things, that
'my @​a; @​a[2] = "b"; say @​a' crashed with a null PMC error.

rt #​61324 was closed and that problem was fixed but a closer look
revealed a number of seemingly related problems that were not. If the
program started with 'my @​a; @​a[1] = "b"' several other cases like 'my
@​b = @​a;', 'my @​b = ("c", @​a);', 'my $x = + @​a.grep({ $_ eq "b"})', and
'my $x = @​a.min()' all crashed with null PMC errors even after #​61324
patches.

It seems that when you take some slurpy params like say/print and grep,
min, join etc. with a declaration like that below

.param pmc args :slurpy

your args are allocated as a ResizablePMCArray. List.pir registers
itself as a parent class of ResizablePMCArray and effectively provides
the '!flatten' method for the args in such a case. When the List.pir
'!flatten' method sees a perl array (Perl6Array) it calls splice to
flatten and append it to the result set. The ResizablePMCArray
get_pmc_keyed_int vtable function, called by splice, was appending
PMCNULL values to the results and causing the crashes. ResizablePMCArray
is an ancestor of Perl6Array. The array assignments like 'my @​a; @​a[1] =
"b"; my @​b = @​a' where also calling the same '!flatten method.

The patch I include here provides a reasonably simple, pir based,
get_pmc_keyed_int vtable function in Array.pir that seems to fix all of
the above problems without breaking anything. There are other possible
approaches to the problem. Since it also fixes the problem with
print/say I also include a patch to remove the changes to print that are
no longer needed.

Tests for related conditions were already in array_extending.t so I
include a patch that puts more tests there including the problem cases
noted above. It seems to me that the tests in array_extending.t are
really autovivification tests that might belong in the
S09-autovivification directory.

I also include a patch that tentatively removes some autovivification
code from Positional.pir since my patch to Array.pir covers all test
cases, the autovivification of rvalue Positional references was a
problem in the existing code and it is not clear to me whether the
Positional role is intended to do autovivification going forward. The
patch to Array.pir includes a comment noting the problems with rvalue
autovivification similar to those described for hashes in rt #​61882.

I expect to be boarding a flight to Mexico tomorrow but expect to be
able to get some access to email by WIFI next week.

Hope some of this helps,
Ron

@p6rt
Copy link
Author

p6rt commented Jan 31, 2009

From @ronaldxs

array_autoviv_bitbetter.patch
Index: t/spec/S02-builtin_data_types/array_extending.t
===================================================================
--- t/spec/S02-builtin_data_types/array_extending.t	(revision 25127)
+++ t/spec/S02-builtin_data_types/array_extending.t	(working copy)
@@ -2,7 +2,7 @@
 
 use Test;
 
-plan 16;
+plan 21;
 
 {
     # Compare with Perl 5:
@@ -86,3 +86,19 @@
     ok @a[1] ~~ undef, '... and the second is undef';
     is @a[2], 6,       '... and  the third is 6';
 }
+
+{
+    my @a;
+    @a[2] = 'b';
+    my @b = @a;
+    is +@b, 3, 'really a degenerative case of assigning list to array';
+    @b = (6, @a);
+    is +@b, 4, 'assigning list with extended array to an array';
+    my $s = @a.join(':');
+    is $s, '::b', 'join on extended array';
+    my $n = + @a.grep({ $_ eq 'b'});
+    is $n, 1, 'grep on extended array';
+    @a[1] = 'c';  # cmp doesn't handle undef cmp undef yet
+    my $m = @a.min();
+    ok not defined $m, 'min on list with undefined el returns undef';
+}
Index: src/classes/Array.pir
===================================================================
--- src/classes/Array.pir	(revision 36120)
+++ src/classes/Array.pir	(working copy)
@@ -30,7 +30,9 @@
 .namespace ['Perl6Array']
 .sub 'delete' :method :multi(Perl6Array)
     .param pmc indices :slurpy
-    .local pmc result
+    .local pmc super_class_accessor, result
+    
+    super_class_accessor = getattribute self, ['ResizablePMCArray'], 'proxy'
     result = new 'List'
     null $P99
 
@@ -47,7 +49,7 @@
     dec $I0
   shorten_loop:
     if $I0 < 0 goto shorten_end
-    $P0 = self[$I0]
+    $P0 = super_class_accessor[$I0]
     unless null $P0 goto shorten_end
     delete self[$I0]
     dec $I0
@@ -267,6 +269,29 @@
 
 =cut
 
+=item get_pmc_keyed_int()   (vtable method)
+
+Try to return undef/Failure rather than null for null index.  Also
+autovivifies for now.
+
+=cut
+
+# autovivification will happen here for rvalues which will need fixing later ...
+
+.sub get_pmc_keyed_int :vtable :method
+    .param int key
+    .local pmc super_class_accessor, retval
+    
+    super_class_accessor = getattribute self, ['ResizablePMCArray'], 'proxy'
+    retval = super_class_accessor[ key ]
+    unless null retval goto done
+    retval = 'undef'()
+    super_class_accessor[ key ] = retval
+done:
+    .return (retval)
+.end
+
+
 # Local Variables:
 #   mode: pir
 #   fill-column: 100
Index: src/classes/Positional.pir
===================================================================
--- src/classes/Positional.pir	(revision 36120)
+++ src/classes/Positional.pir	(working copy)
@@ -46,9 +46,10 @@
     goto end
   result_fetch:
     result = self[$I0]
-    unless null result goto end
-    result = 'undef'()
-    self[$I0] = result
+# -- not clear on whether autovivication is part of positional role (maybe?) --    
+#    unless null result goto end
+#    result = 'undef'()
+#    self[$I0] = result
     goto end
   result_whatever:
     result = 'list'(self)
Index: src/builtins/io.pir
===================================================================
--- src/builtins/io.pir	(revision 36120)
+++ src/builtins/io.pir	(working copy)
@@ -20,9 +20,6 @@
   iter_loop:
     unless it goto iter_end
     $P0 = shift it
-    unless null $P0 goto iter_nonull
-    $P0 = new 'Failure'
-  iter_nonull:
     print $P0
     goto iter_loop
   iter_end:

@p6rt
Copy link
Author

p6rt commented Aug 9, 2009

From @ronaldxs

On Fri Jan 30 17​:19​:56 2009, ronaldxs wrote​:

This is a follow up to rt #​61324 that observed, among other things,
that
'my @​a; @​a[2] = "b"; say @​a' crashed with a null PMC error.

rt #​61324 was closed ... (but) ...
If the program started with 'my @​a; @​a[1] = "b"' several other cases
like 'my
@​b = @​a;', 'my @​b = ("c", @​a);', 'my $x = + @​a.grep({ $_ eq "b"})',
and
'my $x = @​a.min()' all crashed with null PMC errors even after #​61324
patches.

It is now many months later and all of test cases above, except the
last one which uses min, run ok. The failure with the last example
involving min is now covered by RT #​61836. The examples were coded as
test cases in the patch included in the original posting. The part of
the patch providing the tests has now been applied.

Ron

@p6rt
Copy link
Author

p6rt commented Aug 9, 2009

@ronaldxs - Status changed from 'new' to 'resolved'

@p6rt p6rt closed this as completed Aug 9, 2009
@p6rt p6rt added the patch label Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant