Ticket #968 (closed bug: fixed)

Opened 5 years ago

Last modified 5 years ago

Rename PObj_active_destroy_foo to PObj_custom_destroy_foo

Reported by: jrtayloriv Owned by: jkeenan
Priority: minor Milestone:
Component: coding_standards Version: 1.5.0
Severity: low Keywords:
Cc: Language:
Patch status: new Platform:

Description

Earlier, in #parrot I brought up the suggestion that we rename the macros/flags whose names contain "active_destroy" to "custom_destroy".

The function of setting the PObj_active_destroy_FLAG is to say that the the PMC has a custom destroy() function, and there is no reason not to have the name tell us this.

Also, the analogous set of flags/macros for the mark() VTABLE already ARE called PObj_custom_mark_foo, rather than PObj_active_mark_foo. These macros should be named similarly since they serve exactly the same purpose, for the two different functions (i.e. they say "I have a custom mark()/destroy() function").

I recommend globally replacing active_destroy with custom_destroy. Any reasons why we shouldn't?

Attachments

active_to_custom.patch Download (20.7 KB) - added by jrtayloriv 5 years ago.
replace "active_destroy" with "custom_destroy" -- updated to leave and deprecate original macros

Change History

follow-up: ↓ 2   Changed 5 years ago by jrtayloriv

I am attaching a patch to do the replacement.

in reply to: ↑ 1 ; follow-up: ↓ 3   Changed 5 years ago by darbelo

Replying to jrtayloriv:

I am attaching a patch to do the replacement.

The patch gets a +1 for me, with one caveat. Those flags are part of the interface exposed to PMC writers, and are subject to the deprecation policy, which means you have to keep the old definitions in include/parrot/pobj.h and document them as deprecated, to be removed later.

in reply to: ↑ 2   Changed 5 years ago by darbelo

Replying to darbelo:

Replying to jrtayloriv:

I am attaching a patch to do the replacement.

The patch gets a +1 for me, with one caveat. Those flags are part of the interface exposed to PMC writers, and are subject to the deprecation policy, which means you have to keep the old definitions in include/parrot/pobj.h and document them as deprecated, to be removed later.

Hit send too soon. It's not the flags that have to stay, but the accessor macros, which you can just redefine as:

#define PObj_active_destroy_SET(o) PObj_flag_SET(custom_destroy, o) 
#define PObj_active_destroy_TEST(o) PObj_flag_TEST(custom_destroy, o) 
#define PObj_active_destroy_CLEAR(o) PObj_flag_CLEAR(custom_destroy, o)

and leave them next to the new ones. Hope this clarifies my earlier comments.

  Changed 5 years ago by jrtayloriv

Done -- I added the deprecation notice and left the old macros/flags in, in addition to the new ones.

Changed 5 years ago by jrtayloriv

replace "active_destroy" with "custom_destroy" -- updated to leave and deprecate original macros

  Changed 5 years ago by chromatic

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

Thanks, applied as r40935.

  Changed 5 years ago by chromatic

I mean r40936.

Note: See TracTickets for help on using tickets.