Skip to content
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

rfl::Generic: add support for long (int64) #333

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

bjia56
Copy link

@bjia56 bjia56 commented Jan 13, 2025

While working on code that converts rfl::Generic to JSON and back, I found that reflect-cpp@v0.17.0 does not properly convert int64's through Generic. Namely, Generic is defined as a variant with int as one option, which causes the JSON parser to cast yyjson values to int, potentially truncating the number.

This change introduces long as a variant option under Generic. It is placed before int to prioritize converting numbers as the bigger variant in the JSON parser, as otherwise the parser will indiscriminately match all integers to int. However, this likely has the side effect where all ints are now treated as longs. As a corrective measure, to_int and to_long will fall back and static cast from the other type if the variant does not directly match. I was usure of the impact of upgrading the variant's int to long (instead of the proposed implementation of adding a new variant), but perhaps replacing int with long would be cleaner?

If an alternative implementation would be better, please let me know!

@liuzicheng1987
Copy link
Contributor

@bjia56 , some of the tests fail, could you take a look?

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.

2 participants