Skip to content

refactor: improvements to extension methods and generated API#231

Merged
embray merged 3 commits into
asdf-format:mainfrom
embray:copy-dealloc-refactor
Jul 16, 2026
Merged

refactor: improvements to extension methods and generated API#231
embray merged 3 commits into
asdf-format:mainfrom
embray:copy-dealloc-refactor

Conversation

@embray

@embray embray commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Description

A couple changes to the extension API informed by working on
libasdf-gwcs which uses it even more extensively so far than the libasdf
core:

  • The .dealloc method is now named .deinit and is not responsible for
    freeing the extension object itself--only its fields, in case there
    are deeply-nested fields (a shallow object does not even require a
    .deinit)

  • asdf__destroy is the same as before, and frees the extension
    object; adds asdf_<ext>_deinit which de-initializes the object and
    its nested fields without freeing the object (useful with objects
    that may be directly embedded in other objects, allocated on the
    stack, etc.)

  • Similarly the .copy method is no-longer reponsible for allocating
    memory for the copy; instead it is passed a destination pointer for
    the copy. The generated asdf_<ext>_copy method (replacing _clone,
    and getting rid of the confusing naming mismatch) handles the
    allocation boilerplate. asdf_<ext>_copy_into is added for copying
    into an existing memory destination for the copy (which again may not
    have been directly dynamically allocated with malloc; could be embedded
    in another object, stack-allocated, etc.

Also added some reserved space in asdf_extension_vtab_t for future
methods without ABI breakage. Debated how much space to reserve; 16
would be more than enough, but 8 seems enough too--at the moment I can
only think off to top of my head of two methods that might be added in
the future, but they are not high-priority.

AI Disclosure

No AI tools used.

embray added 3 commits July 16, 2026 11:59
A couple changes to the extension API informed by working on
libasdf-gwcs which uses it even more extensively so far than the libasdf
core:

- The .dealloc method is now named .deinit and is not responsible for
  freeing the extension object itself--only its fields, in case there
  are deeply-nested fields (a shallow object does not even require a
  .deinit)

  - asdf_<ext>_destroy is the same as before, and frees the extension
    object; adds `asdf_<ext>_deinit` which de-initializes the object and
    its nested fields without freeing the object (useful with objects
    that may be directly embedded in other objects, allocated on the
    stack, etc.)

- Similarly the `.copy` method is no-longer reponsible for allocating
  memory for the copy; instead it is passed a destination pointer for
  the copy.  The generated `asdf_<ext>_copy` method (replacing _clone,
  and getting rid of the confusing naming mismatch) handles the
  allocation boilerplate.  `asdf_<ext>_copy_into` is added for copying
  into an existing memory destination for the copy (which again may not
  have been directly dynamically allocated with malloc; could be embedded
  in another object, stack-allocated, etc.

Also added some reserved space in `asdf_extension_vtab_t` for future
methods without ABI breakage.  Debated how much space to reserve; 16
would be more than enough, but 8 seems enough too--at the moment I can
only think off to top of my head of two methods that might be added in
the future, but they are not high-priority.
A bit nicer to do now with the new API, and will be needed for ndarray
copy.
@embray embray added the refactoring Changes to code structure or APIs label Jul 16, 2026
@embray

embray commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

Wanted to get this in before committing to a 0.1 release -- it's not a complex change but a far-reaching API decision for implementing extensions. I'm going to test this now against libasdf-gwcs, but I already know that this is exactly the changes it was demanding.

I also went ahead and implemented the missing asdf_datatype_copy method. asdf_ndarray_copy is still unimplemented and broken, but I'll defer to a follow-up. I started on it but it's a considerably more complex case that might need a little more refactoring, and is not (yet) of highest priority.

@embray
embray merged commit ae63b46 into asdf-format:main Jul 16, 2026
12 checks passed
@embray
embray deleted the copy-dealloc-refactor branch July 16, 2026 11:53
@embray

embray commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

I'm also thinking that the _deserialize method might benefit from a similar refactoring--currently every deserialize is responsible for allocating the new object, but it might be helpful to have a version that can just take a destination pointer instead. In the case of serialize, on the other hand, I don't see much need for any related changes.

Actually as I was writing this I already forgot that the _deserialize API already supports that in principle so no further changes are needed, though it's not fully taken advantage of. Maybe I'll revisit that question later. At least nothing else needs to be changed right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring Changes to code structure or APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant