cmp/bitwise/other mmd vtable functions should not go through proxy pmc #352
Comments
|
2317 byte attachment from ronaldxs
t/oo/vtableoverride.t (revision 36383) + t/oo/vtableoverride.t (working copy) - plan(12) - mmd_override_not_proxied() @ -86,6 +87,43 @
ok( $I0, ‘:vtable should imply the self parameter’ )
.end
+.sub ‘mmd_override_not_proxied’ @ -169,6 +207,27 @
.return( 1 )
.end
+.namespace [‘Three’]
``` |
|
Patch to test script relevant to feature. |
|
2429 byte attachment from jkeenan index de2625d..a6b3724 100644
--- a/t/oo/vtableoverride.t
+++ b/t/oo/vtableoverride.t
@@ -17,11 +17,12 @@ Tests the behavior of VTABLE interfaces that have been overriden from PIR.
.sub main :main
.include 'test_more.pir'
- plan(15)
- plan(18)
newclass_tests()
subclass_tests()
vtable_implies_self_tests()
- mmd_override_not_proxied()
anon_vtable_tests()
invalid_vtable()
get_pmc_keyed_int_Null()
@@ -122,6 +123,43 @@ CODE
ok($I0, "Override get_pmc_keyed_int without .return - TT #1593")
.end
+.sub 'mmd_override_not_proxied'
- .local pmc integer_cl, three_cl, three_obj
+
- get_class integer_cl, 'Integer'
- subclass three_cl, integer_cl, 'Three'
- three_cl.'add_attribute'('marker')
- three_cl.'add_attribute'('zero_marker')
- three_cl.'add_attribute'('one_marker')
- three_obj = new 'Three'
+
- # zero_marker/one_marker initialization should go in init vtable method
- # -- todo because seems to seg fault now
- $P0 = new ['Integer']
- $P0 = 0
- setattribute three_obj, 'zero_marker', $P0
- $P0 = new ['Integer']
- $P0 = 1
- setattribute three_obj, 'one_marker', $P0
-
- # not mmd but basic test
- three_obj.'clear_marker'()
- $I0 = three_obj
- three_obj.'verify_marker'('assign to int register calls vtable get_integer override')
+
- .local pmc i
- i = new 'Integer'
- i = 2
+
- three_obj.'clear_marker'()
- $P0 = sub three_obj, i
- three_obj.'verify_marker'('subtract calls vtable get_integer override')
-
- three_obj.'clear_marker'()
- $I0 = cmp_num three_obj, i
- three_obj.'verify_marker'('cmp_num with calls vtable get_integer override')
+.end
+
.namespace [ 'MyObject' ]
.sub '__onload' :anon :init
@@ -220,6 +258,27 @@ yes:
# No .return
.end
+.namespace ['Three']
+
+.sub '' :vtable('get_integer') :method
- $P0 = getattribute self, 'one_marker'
- setattribute self, 'marker', $P0
- .return (3)
+.end
+
+.sub clear_marker :method
- $P0 = getattribute self, 'zero_marker'
- setattribute self, 'marker', $P0
+.end
+
+.sub verify_marker :method
- .param string test_description
- .local pmc get_integer_marker
+
- get_integer_marker = getattribute self, 'marker'
- is(get_integer_marker, 1, test_description)
+.end
+
# Local Variables:
# mode: pir
# fill-column: 100
|
|
Attempt at updating patch submitted by ronaldws |
|
Since vtableoverride.patch no longer applies cleanly, I tried to rework the patch and apply it to t/oo/vtableoverride.t. However, I got a test failure: $ prove -v t/oo/vtableoverride.t t/oo/vtableoverride.t .. 1..18 ok 1 - get_string VTABLE override ok 2 - attribute sideeffect of get_string ok 3 - check first does, ok ok 4 - check second does, ok ok 5 - no it doesn't ok 6 - Morph VTABLE override 1 ok 7 - Morph VTABLE override 1 ok 8 - check first does, ok ok 9 - check second does, ok ok 10 - no it doesn't ok 11 - inherited does ok 12 - :vtable should imply the self parameter ok 13 - assign to int register calls vtable get_integer override ok 14 - subtract calls vtable get_integer override not ok 15 - cmp_num with calls vtable get_integer override # Have: 0 # Want: 1 ok 16 - can have :vtable :anon ok 17 - invalid :vtable throws an exception ok 18 - Override get_pmc_keyed_int without .return - TT #1593 Failed 1/18 subtests ## Test Summary Report t/oo/vtableoverride.t (Wstat: 0 Tests: 18 Failed: 1) Failed test: 15 Files=1, Tests=18, 0 wallclock secs ( 0.02 usr 0.01 sys + 0.01 cusr 0.00 csys = 0.04 CPU) Result: FAIL
Suggestions? Thank you very much. kid51 |
|
It is now two years later and it would take me a week or two to get back up to speed on the status of this ticket. In the meantime undef has essentially become obsolete in Rakudo. The currently active equivalents like Int.new() don't have the same problem. There is probably no solid requirement for Perl 6 filled at this point by these patches. More theoretically/abstractly the idea of the patch is to have cmp behave more like other math functions with respect to its use of proxy pmcs. There may be some performance or simplicity advantage to doing it that way but off the top of my head I am not at all in a position to try and make or even guide that sort of judgement call. Looking back it was chromatic who thought the issue worth further thought on IRC. It looks like there was an attempt to apply the patch to the tests without the change to PMC processing which I would not have expected to be workable. Over the next week or two I can probably attend to the nuts and bolts of making the patches applicable but it would help for someone more familiar with the path of parrot development to assess whether basic idea is still of interest. Thanks, Ron |
In rakudo 1 <=> undef was doing the right thing while undef <=> 1 was failing. The first case works because integer.pmc has a multi cmp_num for undef and the VTABLE_get_integer(INTERP, value) call on the Failure value was correctly calling the Failure get_integer vtable method. When I tried to patch undef.pmcI noticed that I was having trouble getting a cmp_num vtable function in that file to call the Failure get_integer vtable method because the value for SELF was an undef pmc rather than a failure pmc. After further digging I concluded that the cmp_num function was being called through a proxy pmc and cmp_num was behaving differently in that regard from add and subtract which did an mmd call through the default pmc. The module lib/Parrot/Pmc2c/PMC/Object.pm generates the vtable forwarding functions and relies on the vtable_method_does_multi subroutine in lib/Parrot/Pmc2c/PMC.pm to decide which vtable methods go through a proxy and which don’t.
After dropping by #parrot on Feb 5 I was advised that having bitwise and cmp functions go through a proxy pmc might be an oversight and I was asked to come up with an appropriate test if possible.
The patch included (not attached) below relies on the largely baseless assumption that none of the default pmc vtable mmd functions should go through a proxy from an object when I only seem to have been told that fewer of them should. I thought at least parts of it might be useful if taken fwiw.
The attached patch to t/oo/vtableoverride.t should provide some testing for the desired behavior.
Cheers,
Ron
The fwiw parrot patch:
lib/Parrot/Pmc2c/PMC.pm (revision 36383)
+ lib/Parrot/Pmc2c/PMC.pm (working copy)
@ -328,14 +328,27 @return $self→vtable→attrs($methodname)→{write};
}
+my $multi_cmp = qr/cmp(?:num|string|pmc)?/;
return 1 if ($methodname =~ m/^+my $multi_logical = qr/logical_(?:or|and|xor)/;
+my $multi_bitwise = qr/bitwise_(?:or|and|xor|shl|shr|lsr)/;
+my $multi_bitwise_str = qr/bitwise_(?:or|and|xor)/;
+my $multi_arithmetic = qr/add|subtract|multiply|divide|floor_divide|modulus|pow/;
+
+my $multi_bitwise_style = qr/(?:$multi_bitwise|repeat)(?:int)?/o;
+my $multi_bitwise_str_style = qr/(?:$multi_bitwise|concat)(?:str)?/o;
+my $multi_arithmetic_style = qr/$multi_arithmetic(?:int|float)?/o;
+
+my $multi_bit_or_arith_style = qr/
- (?:i_)?
- (?:$multi_bitwise_style|$multi_bitwise_strstyle|$multiarithmeticstyle)
+/xo;
+
sub vtablemethoddoesmulti {
my ( $self, $methodname ) = @;
- (?:i_)?
sub super_method {- (?:add|subtract|multiply|divide|floor_divide|modulus)
- (?:int|float)?
- $/x);
- (?:$multi_cmp|$multi_logical|$multi_bit_orarithstyle)
- $/xo);
}
Originally http://trac.parrot.org/parrot/ticket/285
The text was updated successfully, but these errors were encountered: