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

Consider switching to use _t types. #1787

Closed
externl opened this issue Feb 12, 2024 · 6 comments
Closed

Consider switching to use _t types. #1787

externl opened this issue Feb 12, 2024 · 6 comments
Assignees
Labels
Milestone

Comments

@externl
Copy link
Member

externl commented Feb 12, 2024

I think we should consider switch our numeric types to use _t types (Is there a better name for these?).

Old Type New Type
typedef unsigned char Byte uint8_t
typedef short Short; int16_t
typedef int Int; int32_t
typedef long long int Long; int64_t
typedef float Float; float_t
typedef double Double; double_t
@externl externl added this to the 3.8.0 milestone Feb 12, 2024
@bernardnormier
Copy link
Member

We should check how compatible this proposed change here with the CI platforms.

@externl
Copy link
Member Author

externl commented Feb 16, 2024

Did a preliminary test on macOS. Updated the compiler to generate these types and changed nothing else. Going to test MSVC and GCC next.

@externl
Copy link
Member Author

externl commented Feb 16, 2024

So, macOS and Windows work, however Ubuntu's GCC typedefs int64_t to unsigned long, whereas we use unsigned long long.

So while this is not a backward compatible change I think it's still a change worth making. Yes, users that are upgrading will have to make some changes. I think that's fine as it will bring us more inline with what modern C++ code should look like, as I think these type are the expected norm now.

https://google.github.io/styleguide/cppguide.html#Integer_Types

@pepone
Copy link
Member

pepone commented Feb 16, 2024

I agree on updating this to use _t types.

however Ubuntu's GCC typedefs int64_t to unsigned long

I assume they would use long int in 64bits platforms and long long int in 32bit platforms see https://stackoverflow.com/a/60391237/491807

@externl externl self-assigned this Feb 16, 2024
@externl
Copy link
Member Author

externl commented Feb 19, 2024

This was done in #1806

@externl
Copy link
Member Author

externl commented Feb 19, 2024

In the end we only did with fixed with types, so we did not use float_t or double_t.

@externl externl closed this as completed Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants