Skip to content

Conversation

@chavic
Copy link
Contributor

@chavic chavic commented Aug 25, 2025

Refactored int primitives with BigInt support and bounds checking

  • extended 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.
  • Added BigInt support for larger Int types

@chavic chavic requested review from DanGould and spacebear21 August 25, 2025 06:20
@DanGould
Copy link

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?

@chavic
Copy link
Contributor Author

chavic commented Aug 25, 2025

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:
The biggest reason is to get the type_limits fixture test to pass it fully. Earlier, I did a round on what was needed for stability... this fixture was key for a few reasons... The biggest one is maintaining precision and correctness across platforms. I also caught a few bugs along the way which I infered where caused by possible truncation, and sign reversing for unsigned ints

Copy link
Collaborator

@spacebear21 spacebear21 left a 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.

@chavic chavic force-pushed the chavic/bigint-support branch 2 times, most recently from 3e139b7 to de71bb8 Compare August 27, 2025 08:03
@chavic chavic force-pushed the chavic/bigint-support branch from de71bb8 to f278e0f Compare August 27, 2025 08:13
@chavic chavic force-pushed the chavic/bigint-support branch from a75df42 to 19aa4f2 Compare August 27, 2025 08:46
@davidcaseria
Copy link

I'm trying to generate Dart bindings for CDK and I'm running into RangeError issues. I'm not totally sure what the cause is, but it seems likely that it has something to do with using u64. Are these fixes still planned to be merged?

@chavic
Copy link
Contributor Author

chavic commented Oct 30, 2025

@davidcaseria, yeah, actively working on fixes...
One check, have to verify this PR contains the fix you need...

Have you done that already?

@chavic chavic self-assigned this Nov 1, 2025
@chavic
Copy link
Contributor Author

chavic commented Nov 3, 2025

I moved some of these changes/fixes to a newer PR #95

@davidcaseria
Copy link

I'm still testing this. I think it fixes one issue, but there are still other issues I'm trying to troubleshoot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants