Ticket #1295 (closed RFC: wontfix)

Opened 5 years ago

Last modified 4 years ago

Should FixedPMCArray autovivify nested arrays on set_*keyed()?

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

Description

(From RT #46675)

Here is an example of code:

VTABLE void set_pmc_keyed(PMC *key, PMC *value) {
        const INTVAL k = VTABLE_get_integer(INTERP, key);
        PMC   *nextkey = key_next(INTERP, key);

        if (!nextkey) {
            SELF.set_pmc_keyed_int(k, value);
        }
        else {
            PMC *box = SELF.get_pmc_keyed_int(k);

            /* RT #46675: autovivify an Array and insert it in SELF */
            if (!box)
                box = pmc_new(INTERP, SELF.type());

            VTABLE_set_pmc_keyed(INTERP, box, nextkey, value);
        }
    }

It's clear from the code above that if the element at SELF[k] doesn't exist, that a new array is created, but that new array is never inserted into SELF. So the question is this: What is the expected autovivification behavior of FixedPMCArray (and, by extension, the other Fixed*Array types)?

Attachments

tt_1295.patch Download (1.0 KB) - added by whiteknight 5 years ago.
patch to implement nested array autovivification on fixedpmcarray

Change History

Changed 5 years ago by whiteknight

patch to implement nested array autovivification on fixedpmcarray

Changed 5 years ago by whiteknight

I've attached a patch above that fixes the autovivification behavior to do what I think was intended originally. If people agree, we can apply that patch and add test for the autovivification behavior.

Changed 5 years ago by coke

  • type changed from cage to RFC

Changed 5 years ago by coke

  • milestone changed from 1.9 to 2.0

Changed 5 years ago by chromatic

  • milestone changed from 2.0 to 2.1

Changed 4 years ago by coke

  • milestone 2.2 deleted

Changed 4 years ago by jkeenan

  • cc jkeenan added

whiteknight,

Reviewing old tickets today, I came across this one and tried to apply your patch to trunk at r49555. It did not apply cleanly.

Are the issues you raised in this ticket still relevant? If so, could you please pull a new patch and re-submit?

Thank you very much.

kid51

Changed 4 years ago by bacek

  • status changed from new to closed
  • resolution set to wontfix

Auto-vivification of nested aggregates is deprecated in #1561. Rejecting ticket.

-- Bacek

Note: See TracTickets for help on using tickets.