-
Notifications
You must be signed in to change notification settings - Fork 8
Refactored int primitives with BigInt support and bounds checking #83
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
base: main
Are you sure you want to change the base?
Conversation
|
Hey Chavic, nice to wake up to pushes today. is there rationale for why these changes were made and for why you chose the approach you did? |
the tldr: |
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.
Similarly to my comment on #82, this PR doesn't need the first two commits which are unrelated to the purpose of this PR. Additionally I would modify the ordering of the changes as such:
- first commit adds BigInt support (5820b7d)
- second commit combines 407a700 and fc220ea to support type boundaries.
- third commit enables the type limit fixture. Adding this last ensures that CI stays green throughout every commit and tools like git bisect are effective.
3e139b7 to
de71bb8
Compare
de71bb8 to
f278e0f
Compare
a75df42 to
19aa4f2
Compare
|
I'm trying to generate Dart bindings for CDK and I'm running into |
|
@davidcaseria, yeah, actively working on fixes... Have you done that already? |
|
I moved some of these changes/fixes to a newer PR #95 |
|
I'm still testing this. I think it fixes one issue, but there are still other issues I'm trying to troubleshoot. |
Refactored int primitives with BigInt support and bounds checking
impl_renderable_for_primitive!with a bounds-checking variant (7 params); kept 4-param path for non-bounded types.type_limits(29 tests) all passed.Inttypes