Ticket #185 (closed bug: fixed)

Opened 10 years ago

Last modified 10 years ago

thawed subs with :vtable don't register properly for existing classes

Reported by: jhorwitz Owned by: jhorwitz
Priority: normal Milestone:
Component: core Version:
Severity: medium Keywords:
Cc: Language:
Patch status: Platform:

Description (last modified by jhorwitz) (diff)

If a class is created before load_bytecode thaws a :vtable sub for that class, parrot tries to override a vtable method with the same name as the thawed sub. This blows up when the sub name is not a valid vtable function, which is common in Rakudo. Everything works in fine in pure PIR.

At first glance, the culprit appears to be add_vtable_override() in src/pmc/class.pmc, which only uses the sub's name to check if it's a valid override. Shouldn't it also check the thawed sub's vtable_index?

This breaks mod_perl6, as mod_parrot needs to load P6object before loading Rakudo's perl6.pbc.

Test case follows.

bar.pir:

.sub '__onload' :load
    $P0 = newclass ['Bar']
.end

foo.pir:

.sub '__onload' :load
    load_bytecode 'bar.pbc'
.end

.namespace ['Bar']

.sub 'invalidname' :vtable('get_string') :method
    $S0 = 'hello world'
    .return($S0)
.end

baz.pir:

.sub main :main
    load_bytecode 'bar.pbc'
    load_bytecode 'foo.pbc'
.end

Output:

[jeff@groovy parrot]$ ./parrot -o foo.pbc foo.pir
[jeff@groovy parrot]$ ./parrot -o bar.pbc bar.pir
[jeff@groovy parrot]$ ./parrot baz1.pir
'invalidname' is not a valid vtable function name.
current instr.: 'main' pc 2 (baz1.pir:3)

Attachments

parrot-vtable-method.patch Download (3.8 KB) - added by jhorwitz 10 years ago.

Change History

Changed 10 years ago by jhorwitz

  • description modified (diff)

Changed 10 years ago by jhorwitz

Changed 10 years ago by jhorwitz

  • owner set to jhorwitz
  • status changed from new to assigned

The attached patch fixes the problem and all tests pass, but I'd like a review just in case there are unintended consequences.

The patch checks for a sub's vtable_index and, where appropriate, uses the vtable method's name in lieu of the sub's name. I had to add one additional function, Parrot_get_vtable_name, so I could reliably map the vtable index to the method name (various preprocessor directives prevented me from accessing it from PMC code, and this seemed cleaner anyway).

Changed 10 years ago by allison

Looks like you've got your nesting wrong on line 100 of src/pmc/namespace.pmc. Should be:

if {

if { } else { }

}

Otherwise, thumbs up.

Changed 10 years ago by jhorwitz

  • status changed from assigned to closed
  • resolution set to fixed

The nesting is actually correct, though it's not immediately obvious. We set method_name to the supplied key and leave it alone if everything else is kosher. Perhaps there's a better way to express this, but it works as is.

Patch tweaked for new string API and applied in r37803.

Note: See TracTickets for help on using tickets.