[codex] Support POD struct array fields#343
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds multi-dimensional fixed-size array parsing and mapping to nested Numba UniTuple types, recursive nested POD struct binding with duplicate-name deduplication, tests validating type/LLVM layouts, and documentation describing POD struct field modeling and multidimensional ordering. ChangesMulti-dimensional Fixed-size Array Support for POD Struct Fields
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
5d8c459 to
af7e7ea
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@numbast/src/numbast/struct.py`:
- Around line 462-463: The code currently deduplicates nested struct bindings
using short names stored in bound_struct_names/_bound_struct_names which causes
incorrect skips for same short names in different scopes; update all checks and
mutations that refer to nested_names, bound_struct_names, and
_bound_struct_names to operate on fully qualified names only (compute or use the
qualified name set instead of short names when testing with nested_names and
when adding entries), and replace the places that currently store short names
(the block that populates _bound_struct_names / bound_struct_names around the
later storage) so they insert qualified names; apply the same change to the
other similar checks mentioned (the blocks around the nested binding checks and
the storage of bound names).
In `@numbast/tests/test_struct.py`:
- Around line 89-99: The test currently picks the bound struct by positional
index from _bind_pod_structs_from_source (e.g., PodArrayFields =
_bind_pod_structs_from_source(...)[0]), which relies on parse ordering; change
this to lookup by struct name instead: call _bind_pod_structs_from_source(...)
and find the struct whose name equals "PodArrayFieldsForNumbast" (or the
intended binding name) and assign that to PodArrayFields; apply the same
name-based selection for the other occurrences around lines 110–124 to make the
tests deterministic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 871c04ef-e06c-4010-b99d-5aae3af8b3be
📒 Files selected for processing (5)
docs/source/supported_declarations.rstnumbast/src/numbast/static/types.pynumbast/src/numbast/struct.pynumbast/src/numbast/types.pynumbast/tests/test_struct.py
af7e7ea to
f301a90
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@numbast/tests/test_struct.py`:
- Around line 107-111: The current assertion comparing LLVM IR types by string
in the test (assert [str(ty) for ty in llvm_ty.elements] == [...]) is fragile;
replace it with direct structural comparisons using llvmlite.ir type
constructors: build expected types with ir.ArrayType(ir.IntType(32), 20),
ir.ArrayType(ir.ArrayType(ir.FloatType(), 12), 2), and
ir.ArrayType(ir.FloatType(), 12) and assert equality against llvm_ty.elements
(e.g., assert llvm_ty.elements[0] == ir.ArrayType(ir.IntType(32), 20), etc.),
doing the same for the other occurrences at the noted assertions so tests
compare types structurally instead of by string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 7bc4acf4-c3db-428b-8ea3-49ab4a3c7918
📒 Files selected for processing (1)
numbast/tests/test_struct.py
## Summary - update root `VERSION` to `0.10.0` - add `0.10.0` to `docs/versions.json` and `docs/nv-versions.json` for docs version picker support ## Changelog - Remove direct numba imports from MLIR backend (#365) - [codex] add numba-cuda-mlir support guide (#364) - Split CI tests for Numba-CUDA and MLIR (#363) - Migrate experimental MLIR backend (#346) - Bump test-summary/action from 2.4 to 2.6 in the actions-monthly group (#345) - [codex] Support POD struct array fields (#343) - fix(types,callconv): register CUDA vector ABI alignment (#321) ## Validation - `python -m json.tool docs/versions.json` - `python -m json.tool docs/nv-versions.json` - `git diff --check` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Version 0.10.0 released * Updated version references in documentation configuration files <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/numbast/pull/367?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Michael Wang <isVoid@users.noreply.github.com>
Summary
Adds support for POD struct data-model fields with fixed-size arrays, multidimensional arrays, nested POD structs, and arrays of nested POD structs. Fixes
float[2][12]so it models two rows of twelve floats.Also keeps nested struct binding deduplication scoped to fully qualified record names, so same short names in different nested scopes do not collide.
Validation
pre-commit run --all-filespassed.5 passed.python -m compileall -q numbast/src/numbast/struct.py numbast/tests/test_struct.pypassed.git diff --checkpassed.Full
numbast/tests/test_struct.pycannot complete here because CUDA kernel launch tests fail withCUDA_ERROR_NO_DEVICE.Summary by CodeRabbit
New Features
Documentation
Tests