-
Notifications
You must be signed in to change notification settings - Fork 120
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
feat: new functions in stdlib from stdlib.fc
and math.fc
#986
feat: new functions in stdlib from stdlib.fc
and math.fc
#986
Conversation
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.
Sorry for reviewing the draft, I just wanted to keep those notes around :)
178e712
to
b9b6206
Compare
@novusnota @anton-trunov Implemented all functions. I'd like to hear some intermediate feedback on them before I proceed with writing tests. I made sure to include all missing functions from both stdlib.fc and mathlib.fc with some exceptions (see PR description). |
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.
afair, mathlib.fc
has lots of functions for fixed-arithmetic as well, I think we should add those too (perhaps as a separate library, like fixed_point_math.tact
).
Also, I left a couple comments in the source code
@anton-trunov @novusnota covered all new functions with tests! Found and fixed a few little mistakes in types (for |
@Gusarich Let's
|
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.
We should document the new functions in https://docs.tact-lang.org/ref/core-math and other relevant doc pages
Will do |
Co-authored-by: Novus Nota <68142933+novusnota@users.noreply.github.com>
macOS CIs exceed some time limits even after restarting those in isolation. Other than that, this PR seems to be done, I've even added the couple of "gas-expensive" badges :)
✔️ |
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.
Almost done, just one final push
Let's finally merge this :) |
Uno momento |
@i582 Resolved your suggestions (hopefully), please take a look |
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.
LGTM
Hooray 🎉 |
Issue
Closes #770
Towards #1029 (only
myCode()
)Checklist
docs/
and made the build locallyCurrent State
minmax
and noslice_bits_refs
because we don't have tensor types in Tact and it would be useless: Tensor types #1116set_data
andget_data
because Tact manages persistent storage by itselfbless
and other continuation functions because Tact manages control flow itself and for running some arbitrary code it's better to use RUNVM which doesn't require continuations parametersraw_reserve_extra
because it requires anuint32 -> varuint32
dictionary and we only supportvaruint16
for values currently:VarUInt32
,VarInt16
,VarInt32
serialization types #1117nan
andis_nan
because we don't support NaN in Tact. Do we really need them? @anton-trunovsub_rev
because it is useless in our case...MOD
instructions because they return two values as a tensor and it won't be very developer-friendly in Tact to make them return tuples in my opinion: Tensor types #1116