refactor(zero-c): consolidate duplicated dynamic-array grow helpers (#390)#406
Open
Osamaali313 wants to merge 1 commit into
Open
refactor(zero-c): consolidate duplicated dynamic-array grow helpers (#390)#406Osamaali313 wants to merge 1 commit into
Osamaali313 wants to merge 1 commit into
Conversation
…ercel-labs#390) The same dynamic-array grow helper was duplicated across four files in native/zero-c/src: ir_grow_items (ir.c), lower_grow_items (program_graph_lower.c) and checker_grow_items (checker.c) shared an identical body, and ir_grow_tracked_items was defined verbatim in both ir.c and program_graph_mir.c. As noted in vercel-labs#390, the duplicated mir_bytes accounting can silently diverge if one copy is edited without the others. Hoist both into include/zero.h as shared `static inline` helpers (z_grow_items and ir_grow_tracked_items), placed after the IrProgram definition so the tracked variant can reference ir->mir_bytes, and drop the five local definitions, repointing call sites at the shared helpers. Pure refactor; no behavior change. Validation: native/zero-c builds clean under -Wall -Wextra -Wpedantic; host-target (linux-x64) build+run works; all conformance check graph fixtures pass `zero check`. The linux-musl-x64 build steps in agent:checks fail identically on a clean checkout in this environment (BLD003: no musl-capable C compiler installed), unrelated to this change.
|
@Osamaali313 is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Refactors dynamic array growth helpers by centralizing them into shared inline functions, reducing duplicated logic across several C source files.
Changes:
- Removed per-file
*_grow_itemshelpers inir.c,program_graph_lower.c,checker.c, andprogram_graph_mir.c. - Introduced shared
z_grow_items(andir_grow_tracked_items) helpers ininclude/zero.h. - Updated call sites to use
z_grow_items.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| native/zero-c/src/program_graph_mir.c | Removes local tracked grow helper in favor of shared header implementation. |
| native/zero-c/src/program_graph_lower.c | Replaces local grow helper usage with z_grow_items. |
| native/zero-c/src/ir.c | Removes local grow helpers and switches call sites to z_grow_items. |
| native/zero-c/src/checker.c | Removes local grow helper and switches call sites to z_grow_items. |
| native/zero-c/include/zero.h | Adds shared inline grow helpers (z_grow_items, ir_grow_tracked_items). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1077
to
+1079
| /* As z_grow_items, but also accounts the new allocation against an IrProgram's | ||
| * mir_bytes total. Replaces the verbatim copies in ir.c and program_graph_mir.c. */ | ||
| static inline void *ir_grow_tracked_items(IrProgram *ir, void *items, size_t len, size_t *cap, size_t initial, size_t item_size) { |
Comment on lines
+1069
to
+1075
| static inline void *z_grow_items(void *items, size_t len, size_t *cap, size_t initial, size_t item_size) { | ||
| if (len + 1 > *cap) { | ||
| *cap = z_grow_capacity(*cap, len + 1, initial); | ||
| return z_checked_reallocarray(items, *cap, item_size); | ||
| } | ||
| return items; | ||
| } |
Comment on lines
+1081
to
+1084
| size_t next_cap = z_grow_capacity(*cap, len + 1, initial); | ||
| void *next_items = z_checked_reallocarray(items, next_cap, item_size); | ||
| if (ir) ir->mir_bytes += next_cap * item_size; | ||
| *cap = next_cap; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #390.
The same dynamic-array grow helper was duplicated across four files in
native/zero-c/src:ir_grow_items(ir.c),lower_grow_items(program_graph_lower.c), andchecker_grow_items(checker.c) shared an identical body.ir_grow_tracked_items(the variant that also accountsir->mir_bytes) wasdefined verbatim in both
ir.candprogram_graph_mir.c.As the issue notes, the duplicated
mir_bytesaccounting can silently divergeif one copy is edited without the others.
Change
Hoist both helpers into
include/zero.has sharedstatic inlinefunctions(
z_grow_itemsandir_grow_tracked_items), placed right after theIrProgramdefinition so the tracked variant can referenceir->mir_bytes.The five local definitions are removed and their call sites repointed at the
shared helpers.
Pure refactor — the helper bodies are byte-for-byte the originals, so there is
no behavior change. Net
+61 / -81lines across 5 files.Validation
make -C native/zero-cbuilds clean under-Wall -Wextra -Wpedantic.zero build --emit exe --target linux-x64on
conformance/run/pass/hello.graphproduces an exe that runs and printsthe expected output.
conformance/check/pass/*.graphfixtures passzero check(this is thepath that exercises the modified
checker.candir.clowering).