Rename macro defintr
#60
Conversation
WalkthroughThe pull request introduces a systematic renaming of the Changes
Possibly related PRs
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/charms/pointer.ex (1)
25-25: Consider refactoring the size allocation logic for clarity.
While the rename is correct, the internal logic for handling differentsizetypes (integer, single-element, orMLIR.Value) is quite extensive. Consider extracting it into a helper function for improved readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
lib/charms/env.ex(1 hunks)lib/charms/intrinsic.ex(1 hunks)lib/charms/kernel.ex(2 hunks)lib/charms/pointer.ex(10 hunks)lib/charms/prelude.ex(4 hunks)lib/charms/simd.ex(2 hunks)lib/charms/term.ex(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- lib/charms/simd.ex
🔇 Additional comments (21)
lib/charms/env.ex (1)
8-8: Rename appears consistent.
Renaming from defintrinsic to defintr is clear and consistent. No issues observed with the new definition for t().
lib/charms/term.ex (1)
11-11: Rename aligns well with the new macro usage.
This is a straightforward, consistent rename to defintr. The intrinsic function logic remains the same, so no concerns here.
lib/charms/kernel.ex (2)
Line range hint 52-76: Binary ops definition updated smoothly.
Renaming defintrinsic to defintr for the binary operations is consistent and does not affect the logic. Implementation remains sound, including the handling of mismatched argument types.
Line range hint 80-92: Unary negation rename is coherent with the rest of the changes.
The rename for the !value intrinsic is straightforward. It continues to raise an exception for unsupported types. No issues found with the updated syntax.
lib/charms/prelude.ex (6)
Line range hint 21-30: Consistent rename for result_at/2.
No logic changes noted. The rename from defintrinsic to defintr follows the intended convention across the codebase.
35-37: Refactor retains behavior.
type_of/1 remains the same functionally. The rename is consistent with the rest of the codebase.
Line range hint 39-49: No functional changes for const/1.
The rename is straightforward, and the macro usage for constant creation remains intact.
51-51: dump/1 rename is consistent.
Retains the same debugging/logging logic. The rename has no functional impact.
101-101: Updated signature for ENIF bridging.
The loop-based rename for each ENIF function is correct, ensuring consistency.
Line range hint 1-1: Confirm no leftover references to defintrinsic remain.
All changes appear consistent, though consider verifying other references in the codebase, documentation, or comments that might mention the old macro name.
Use the following script to search for leftover references to defintrinsic:
✅ Verification successful
References to defintrinsic found in lib/charms/intrinsic.ex appear to be intentional
The occurrences of defintrinsic found in lib/charms/intrinsic.ex are part of the internal implementation:
- Used as a module attribute:
@defintrinsic - Used in metaprogramming contexts for generating unique identifiers:
__defintrinsic_#{name}__
These references are legitimate implementation details rather than leftover usage of a deprecated macro. The pattern suggests these are intentionally used as part of the intrinsic definition mechanism.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Searching for any leftover references to `defintrinsic` across the entire repository.
rg "defintrinsic"
Length of output: 322
lib/charms/intrinsic.ex (1)
70-70: Rename aligns with PR objective and is consistent across the codebase.
The renaming from defintrinsic to defintr is consistent with the broader refactor, and the updated macro logic remains unchanged in essence. Good job!
lib/charms/pointer.ex (10)
16-16: Rename to defintr is consistent and correct.
This change correctly aligns with the newly introduced macro name in Charms.Intrinsic. No issues found.
73-73: Rename to defintr looks good.
No functional concerns with the load(type, ptr). All logic remains unchanged.
88-88: Consistent rename for typed pointer load.
No further changes required here; function behavior is unaffected.
103-103: Store function rename is fine.
Implementation is consistent with the rest of the pointer logic.
150-150: defintr rename is correct; logic for offset calculation remains unchanged.
All good.
182-182: Element pointer overload rename is consistent.
No issues found.
194-194: Rename for element_type/1 remains aligned with the new intrinsic naming convention.
No concerns here.
207-207: Pointer type helper rename to defintr is consistent.
The function is straightforward and remains valid.
212-212: t(elem_t) rename is correct.
The pointer type logic is unchanged and still appropriate.
255-255: Memcopy function rename is consistent with new macro.
Functionality is unaffected, so this is good to go.
Summary by CodeRabbit
defintrinsicmacro todefintracross multiple modules includingCharms.Env,Charms.Intrinsic,Charms.Kernel,Charms.Pointer,Charms.Prelude,Charms.SIMD, andCharms.Termdefintrkeyword while maintaining existing functionality