feat(stdlibs): add package strconv
#1464
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1464 +/- ##
==========================================
+ Coverage 61.11% 63.47% +2.36%
==========================================
Files 565 552 -13
Lines 75396 86833 +11437
==========================================
+ Hits 46077 55117 +9040
- Misses 25950 28135 +2185
- Partials 3369 3581 +212
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Member
Author
|
So, tests are currently failing. A bunch of tests I've fixed after realising that the underlying problem is with how we do comparisons when we have a bitshift with an untyped integer on the lhs: #1462 (comment) I suspect there may be more bugs involved. In any case, I attempted to make a separate branch from this which merged in the changes from #1426. Turns out, that the tests are completely passing with that code applied to Gno. So, I'll be continuing work on this once #1426 is merged. For the meantime, I'll update the OP to mark what remaining work is to be done to complete this PR so I can easily pick it up later. |
Contributor
I did this as an experiment, it passes all the test after making a small adjustment on your code, |
8 tasks
Member
Author
thehowl
requested review from
jaekwon,
moul,
mvertes and
piux2
as code owners
thehowl
requested review from
a team,
gfanton,
leohhhn and
zivkovicmilos
as code owners
github-actions
bot
added
the
📦 ⛰️ gno.land
Member
Author
|
This PR is now ready for review. I updated the OP with a summary of the changes involved. |
albttx
approved these changes
Oct 16, 2024
Member
Sounds reasonable, let's do it. |
moul
reviewed
Oct 16, 2024
moul
reviewed
Oct 16, 2024
ltzmaxwell
approved these changes
Oct 17, 2024
ltzmaxwell
reviewed
Oct 17, 2024
ltzmaxwell
reviewed
Oct 17, 2024
Co-authored-by: Manfred Touron <94029+moul@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the full
strconvpackage, implemented as pure Gno code. It removes the native functionsItoa(and others), and adds support for new functions such asFormatFloat.Summary of changes
strconvfrom Go stdlib: https://gist.github.com/thehowl/904b42b1ea53fef9b8a0486155c02b73 -- tldr:unicodestrconvtests succeed.unicode/utf8fallback(as we now half-support it) and useAppendString.store.go,store_test.go,nodes.go,tests/imports.go.gonative.goandmachine.gochanges improve some error messages.preprocess.gochanges fix a bug which can be seen in thefor20.gnotest. If aforloop is labeled, then a barebreak(ie. without a label to break to) would panic, as it wouldn't find any for loop without a label (infindBranchLabel). I added a regression test as well as a couple test showing the error message for when we misplace continue/break statements.strconv.Itoanow uses more gas than its existing native implementation. This is to be expected; we can consider moving it back to a native implementation if we deem it useful for performance, but I think it's good for us to work on having as much code implemented directly in gno before moving it back to Go for performance.