feat[venom]: add IRContext prefix + merge#4939
Conversation
* reusable code+data snippets to avoid hand-invented unique label names * add `IRContext(prefix=...)` so labels are namespaced automatically * add `merge(*sources)` to combine prefixed sub-contexts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4018326459
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| function_labels = set(self.functions) | ||
| data_labels = {section.label for section in self.data_segment} | ||
|
|
||
| for src in sources: | ||
| for fn in src.functions.values(): |
There was a problem hiding this comment.
IRContext.merge() only checks fn.name and data-section labels for collisions, but it does not validate labels inside each function’s basic blocks. Two sources can therefore pass validation (e.g. unique function names) yet still contain identical block labels generated from get_next_label() under the same prefix/empty prefix, and VenomCompiler will emit duplicate assembly symbols that fail in resolve_symbols with duplicate label. This makes merge non-atomic for a real collision class and can break compilation as soon as merged code is lowered to assembly.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4939 +/- ##
==========================================
+ Coverage 91.94% 91.97% +0.02%
==========================================
Files 186 186
Lines 27571 27611 +40
Branches 4818 4830 +12
==========================================
+ Hits 25349 25394 +45
+ Misses 1508 1503 -5
Partials 714 714 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
What I did
Add
IRContext(prefix=...)+merge(*sources)so reusable code+data snippets can be authored with local names and spliced into a parent context without label collisions.How I did it
Prefix auto-applied to label-emitting methods + str overloads, with
prefixed_label(name)for refs that must survivemerge. Andmergevalidates first and raises on clash before mutating, clearing sources on success.How to verify it
pytest tests/unit/compiler/venom/test_context_prefix.pycovers prefix application and merge contract (moves+clears, raises on duplicate function / data-section labels, atomic-on-failure).Commit message
feat[venom]: add IRContext prefix + mergeDescription for the changelog
Added
IRContext(prefix=...),prefixed_label(), andmerge(*sources)for composing prefixed sub-contexts without label collisionsCloses #4938
Cute Animal Picture