Ticket #2133 (new RFC)

Opened 3 years ago

Last modified 3 years ago

src/extend.c Review

Reported by: whiteknight Owned by: whiteknight
Priority: normal Milestone:
Component: embed/extend Version: master
Severity: medium Keywords: extend
Cc: Language:
Patch status: Platform: all

Description

The functions in extend.c and extend_vtable.c use the PARROT_CALLIN_START and PARROT_CALLIN_END macros to ensure the stacktop value is set for proper GC operation. This is unnecessary for two reasons:

1) We have an embedding API, and a clear distinction between embedding and extending situations. In an extending situation, the stacktop should always be set.

2) The functions in extend.c and extend_vtable.c represent only a small subset of what we currently consider the "extending API". Many extension libraries use other subsystem API functions directly, and some extension libraries never use any of the functions defined in extend.c or extend_vtable.c, which means those libraries would never see this protection (and they aren't any worse off for it).

If those two macros go away (And I suggest they should), there are a few functions in extend.c which can go away because they are nothing but wrappers around other functions:

  • Parrot_PMC_new (Parrot_pmc_new)
  • Parrot_free_cstring (Parrot_str_free_cstring)
  • Parrot_register_pmc and Parrot_unregister_pmc
  • Parrot_register_string and Parrot_unregister_string

Also, there are a few other functions which need to be re-considered:

  • Parrot_get_*reg and Parrot_set_*reg (These functions should take a PMC * for the context, because the *_REG macros are going to read from the current context, which in an extending or NCI context is likely to be worthless. Also, these functions don't do any bounds-checking)
  • All the Parrot_printf-related functions. We have similar functions in src/utils.c and elsewhere which integrate properly with the IO and String subsystems
  • Parrot_new_string (Should probably be moved to src/string/api.c, because it's just a wrapper around Parrot_str_new_init with some logic to lookup encodings by name)

Also, it would be very good if we had an "official" definition for what our extending API is. Is it, as is the de facto definition, the sum of the individual subsystem APIs, or is it something distinct?

Change History

Changed 3 years ago by dukeleto

I agree with the sentiment of this TT. I think we need to explicitly define what our extending API is, and not leave it up to interpretation.

I will gladly help with that. Once I am done with my embed/extend TPF grant, I will have time to hack on PL/Parrot, which will shed more light on how embedders/extenders will use stuff and if what we are currently providing is good enough.

Note: See TracTickets for help on using tickets.