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

Prefer bitsize variant over standard integral types #128

Merged

Conversation

winwinashwin
Copy link
Contributor

Fixes #69

Have adapted some tests, replaced some types in const, pointer tests to keep code clean.

Note : Although the CI checks are all passing, the standard does not mandate fixed width integer types to be available. Though very rare, there might be cases when these are undefined (embedded applications is what I can think of). I am not sure if if this project in intended to support such applications. If that is the case we could wrap each association with a feature test macro. Something like

#if defined(INT16_MAX)
DBG_MACRO_REGISTER_TYPE_ASSOC(short, int16_t)
#else 
DBG_MACRO_REGISTER_TYPE_ASSOC(short, short)
#endif 

Or if you have any other elegant suggestions please let me know!

tests/example.cpp Outdated Show resolved Hide resolved
Copy link
Owner

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much!

@sharkdp
Copy link
Owner

sharkdp commented Jan 15, 2023

Note : Although the CI checks are all passing, the standard does not mandate fixed width integer types to be available. Though very rare, there might be cases when these are undefined (embedded applications is what I can think of). I am not sure if if this project in intended to support such applications.

Thank you for thinking about this. I think it's fine to rely on the availability. For now, we do support the platforms and compilers that are tested in CI (GCC, Clang, MSVC). If someone wants to add support for additional platforms or compilers, we can re-evaluate this.

@winwinashwin
Copy link
Contributor Author

Another behaviour to note, even though the associations are registered, type names deduced from __PRETTY_FUNCTION__ and friends will still show int instead of int32_t. See below

int var{};
dbg(var);  // var = 0 (int32_t)

int arr1[3]{};
dbg(arr1);  // arr1 = {0, 0, 0} (int [3])

std::array<int, 3> arr2{};
dbg(arr2);  // arr2 = {0, 0, 0} (std::array<int, 3>)

std::array<std::pair<int, int>, 3> arr3{};
dbg(arr3);  // arr3 = {{0, 0}, {0, 0}, {0, 0}} (std::array<std::pair<int, int>, 3>)
dbg(arr3[0]);  // arr3[0] = {0, 0} (std::pair<int32_t, int32_t>&)

I am not sure how to fix this apart from specialising get_type_name for such common cases. What are your thoughts on this?

README.md Show resolved Hide resolved
@sharkdp
Copy link
Owner

sharkdp commented Jan 15, 2023

Thank you. This looks (almost) good to go. Can you please remove the tests/example binary?

I am not sure how to fix this apart from specialising get_type_name for such common cases. What are your thoughts on this?

Hm. Adding a specialization for C-style arrays and std::array would probably be a good idea, to avoid inconsistencies/confusion. But feel free to skip this for now.

@winwinashwin winwinashwin force-pushed the feature/bitsize-for-integral-types branch from 1605f94 to e7594a0 Compare January 15, 2023 14:18
- Specializations for `std::array` and C-style array declarations
@winwinashwin winwinashwin force-pushed the feature/bitsize-for-integral-types branch from e7594a0 to cf4ad54 Compare January 15, 2023 14:22
Copy link
Owner

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the update!

@winwinashwin
Copy link
Contributor Author

Love this project! Taught me a lot 😄

@sharkdp sharkdp merged commit 3c755ee into sharkdp:master Jan 15, 2023
@winwinashwin winwinashwin deleted the feature/bitsize-for-integral-types branch January 15, 2023 14:34
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.

Print integral types as bit sizes instead of standard types
2 participants