Feat(#250): Implement bitwise operators (&,|,^,~,<<,>>)#257
Feat(#250): Implement bitwise operators (&,|,^,~,<<,>>)#257AryanBhirud wants to merge 2 commits intoarxlang:mainfrom
Conversation
| (astx.Int32, astx.LiteralInt32), | ||
| (astx.Int16, astx.LiteralInt16), | ||
| (astx.Int8, astx.LiteralInt8), | ||
| (astx.Int64, astx.LiteralInt64), |
There was a problem hiding this comment.
I'm afraid I don't think python supports bitwise operators for floats
There was a problem hiding this comment.
@AryanBhirud you are right that bitwise operators don't apply to floats but i guess @yuvimittal meant adding unsigned integer types here? e.g. (astx.UInt8, astx.LiteralUInt8),(astx.UInt16, astx.LiteralUInt16), etc. That would also help catch the >> signed vs unsigned behavior, since ashr and lshr produce different results
|
Hey @AryanBhirud, nice work on implementing all six bitwise operators! A few things I noticed: 1.first is that >> should use lshr for unsigned types — Right now it always uses ashr (arithmetic shift right), which preserves the sign bit. For unsigned integers it should use lshr (logical shift right). You can check _is_unsigned_node(node) the same way the / operator does (line ~1236 on main). 2.the second thing i noticed is missing float guard that is If someone accidentally passes float operands to &, |, etc., LLVM will crash. The other arithmetic ops (+, -, *) check is_fp_type() before dispatching,it might be good to add an early error like raise Exception("Bitwise operators are not supported for float types."). |
|
@AryanBhirud it seems I cannot push changes to your branch .. maybe because you are working on a branch called "main", which usually we don't do. I like the ideas @Jaskirat-s7 mentioned. please implement that. in a very near future we should move this kind of validation to the semantic layer. which we don't have yet. |
|
i was stuck up with some other work this past week so I couldn't get the changes done, I'll get started on them this week 👍🏻 |
Notes
you work. When you’re ready for a review, change the status to Ready for
review to trigger a new review round. If you make additional changes and
don’t want to trigger the bot, switch the PR back to Draft.
share your feedback; it helps us improve the tool.
as possible to increase the chances of a timely review. Large PRs may not be
reviewed and may be closed.
self-documenting
(guidance).
our Discord to discuss ideas, blockers, or issues
(https://discord.gg/Nu4MdGj9jB).
sensitive data/PII in code, configs, logs, screenshots, or commit history. If
something leaks, rotate the credentials immediately, invalidate the old key,
and note it in the PR so maintainers can assist.
needed for tests, prefer small fixtures or programmatic downloads declared in
makim.yaml (e.g., a task that fetches data at test time). If a large binary is
unavoidable, discuss first and consider Git LFS.
Pull Request description
Solves #250
How to test these changes
...Pull Request checklists
This PR is a:
About this PR:
Author's checklist:
complexity.
Additional information
Reviewer's checklist
Copy and paste this template for your review's note:
Reviewer's Checklist
mainbranch