-
Notifications
You must be signed in to change notification settings - Fork 131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multivalue-opcodes #543
base: master
Are you sure you want to change the base?
Multivalue-opcodes #543
Conversation
With some vision from #541 , where I'm going to create a |
For |
Would you mind reviewing this PR as well? @michaeldiamant |
return super().__teal__(options) | ||
|
||
|
||
WideExpr.__module__ = "pyteal" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this after reading PEP3130. I think it's rejected? I never used this personally, but I still added it as it's part of your convention. I hope I did it correctly.
Co-authored-by: Zeph Grunschlag <[email protected]>
@PabloLION Thanks for putting in some cycles here. Due to other priorities, it's been some time since I last looked into the PR's topic. From my perspective, I see the ergonomics and API design as intertwined. Since pyteal makes API backwards compatibility guarantees, I'm hesitant to bring the PR in and consider ergonomics later. I'm not looking to discourage you from continuing ahead. But, I want to be upfront that given the current priority set, we're unlikely to merge the PR in a near future. |
@michaeldiamant Thanks for the reply. |
Before finishing the ergonomic part of Wide64 arithmetic in #541, the basic opcodes should be implement first.
Goal
Here I'm trying to add
addw
,mulw
,divw
(exsited),divmodw
. And maybe some other stack ops likedig n
,swap
,cover n
,uncover n
.TBD
dig n
,swap
,cover n
,uncover n
manipulate stack hence are not very useful in pyteal, where we don't usually interact the stack.WideExpr(MultiValue)
in new file 'wideexpr.py', and named the methods asAddW
,MulW
, etc. instead ofAddw
, etc, since they are all multivariable functions (likeaddw
is binary-multivalue), hence current system which categorizes functions based on their input var num (like 'binaryexpr', 'ternaryexpr', 'naryexpr') doesn't work well on them. The common property is that they all focus onWideInt
, but this might breaks the current categorization to be not mutually exclusive and might leads to confusion.Tasks
addw
addw
for valid caseaddw
with invalid case,mulw
,expw
,divmodw
with their testsadd function overload foraddw
(adding many 64bit uint)test for addw overload for valid case and invalid caseadd function overload formulw
(multiplying many 64bit uint)It seems there's no arithmetic test in tests so I skip them too.
These parts I didn't finish are moved to PR#541 where I implement the ergo-features.