Ticket #2133 (new RFC)
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?