TT #389 fix introduced regression with globals.

[coke]

r45827 introduces a regression in partcl. Stripping it down to the smallest PIR I can:

.sub 'blah'
    $P1 = new 'Sub' # String prints "ok"...
    set_root_global 'arg', $P1

    $P0 = get_root_global 'arg'
    unless null $P0 goto good
      print "not "
    good:
      say "ok"
.end

This prints "not ok" in r45827 and HEAD, but I expect it to print "ok", even after the :nsentry fixes.

I suspect that the change to the namespace PMC was overbroad, and shouldn't have impacted this case where I am quite deliberately sticking something that ISA Sub into a namespace. (As opposed to declaring a .sub and letting the PIR compiler decide what to do with it.)

On the other hand, if I'm mistaken, and I have to do the same work that IMCC is doing to add the :nsentry flag, how do I do that? It doesn't appear that the C-level flag SUB_COMP_FLAG_NSENTRY (and sub_comp_flags_enum) is used anywhere other than IMCC or the Namespace PMC.

[coke]

Simplify the example even more.

[whiteknight]

I think it's becoming pretty obvious that the lowly NameSpace PMC is being tasked with more storage tasks than it has a reasonable interface to allow. We're using the NameSpace to separately store, by name, the following things:

• .sub Foo :nsentry
• Methods (stored in the NameSpace until moved into the class, and not supposed to be publicly-accessible)
• VTABLE overrides
• set_*_global data items
• nested sub-namespaces

NameSpace already uses the get/set_pmc_keyed* VTABLEs for storing most data items, and the get/set_pointer_keyed* VTABLEs for accessing sub-namespaces. The problem in this ticket stems from the fact that the :nsentry subs are stored and retrieved using the same VTABLE routines as the set_*_global opcodes use to store other arbitrary data items.

My suggestion is this: I suggest we create a some sort of cache object (probably a raw Hash PMC, but maybe something that derives from it if we need more features). The NameSpace gets an array of these cache objects, addressable by integer offset using VTABLE_get_pmc_keyed_int (VTABLE_get_pmc_keyed_str would work too, but string lookups are slower and we're going to have a fixed number of caches). Then, anything we want to store in the NameSpace goes through this kind of process:

• Lookup the appropriate cache in the namespace
• Set/get the desired item in the cache by name

Each individual get/set becomes a little bit more expensive, but we save a lot of effort trying to force the NameSpace to keep track of a growing number of lists of things.

[coke]

Replying to coke:

Simplify the example even more.

If anyone needs to workaround this, one possible solution is to stuff the Sub into an Array, and just get the container out on the other end. Suboptimal but functional.

[bacek]

Hello.

On current parrot it prints "ok" for test from description.

bacek@illusion:~/src/parrot (master) $ ./parrot t.pir 
ok

Can we close this ticket?

-- Bacek

[cotto]

Replying to bacek:

Hello. On current parrot it prints "ok" for test from description. {{{ bacek@illusion:~/src/parrot (master) $ ./parrot t.pir ok }}} Can we close this ticket? -- Bacek

Yup. Thanks for noticing.