Skip to content

C: Deprecate hb_array_size#885

Merged
marcoroth merged 1 commit into
marcoroth:mainfrom
timkaechele:deprecate-hb-array-size
Dec 5, 2025
Merged

C: Deprecate hb_array_size#885
marcoroth merged 1 commit into
marcoroth:mainfrom
timkaechele:deprecate-hb-array-size

Conversation

@timkaechele

Copy link
Copy Markdown
Contributor

This PR deprecates the hb_array_size function in favor of directly accessing hb_array_T.size

As part of the deprecation, ast nodes now always have fully initialized array members.

@marcoroth marcoroth left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why we don't remove hb_array_size with this right away?

@timkaechele timkaechele force-pushed the deprecate-hb-array-size branch from 040628d to de27870 Compare November 17, 2025 06:24
@timkaechele

Copy link
Copy Markdown
Contributor Author

It’s going to be dropped as part of the migration to the new array implementation.

@timkaechele timkaechele force-pushed the deprecate-hb-array-size branch from de27870 to 719d516 Compare November 17, 2025 19:47
@timkaechele timkaechele force-pushed the deprecate-hb-array-size branch from 719d516 to 8fd61ff Compare December 4, 2025 23:07

@marcoroth marcoroth left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @timkaechele! 🙏🏼

@marcoroth marcoroth merged commit a43c7e6 into marcoroth:main Dec 5, 2025
24 checks passed
@timkaechele timkaechele deleted the deprecate-hb-array-size branch December 5, 2025 06:52
marcoroth added a commit that referenced this pull request Dec 29, 2025
…ge (#1025)

This pull request reverts #885 which caused memory issues in WebAssembly
by always allocating arrays, even when `NULL` was passed.

The `hb_array_size()` function safely returns `0` for `NULL` arrays,
eliminating the need for allocating arrays for all AST Nodes. This was
causing the `"Cannot enlarge memory"` errors in WASM builds, as seen in
#1016.

This pull request brings back `hb_array_size()` in places where we
previously accessed `array->size` directly. With that, we don't need to
allocate `array_init(8)` just so we can assume we always have a `size`.

The problem is, even if we would use `array_init(0)` it would allocate
memory that we are not going to need. So for now, I think it's better to
be avoid allocating arrays and use `hb_array_size()` until we have fully
integrated the new array implementation from #782. (/cc @timkaechele).

I also added a `hb_narray_size()` for API consistency that should help
with the future array migration.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants