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

Maybe fix bug in remove_trailing_zeros(u64) #66

Merged
merged 1 commit into from
Aug 10, 2024

Conversation

tobybell
Copy link
Contributor

Not sure if this was a typo or not, but seems like might be given that the integer wouldn't fit into a u32. If this is not a typo and was intentional, I'd be interested to understand the reasoning behind it.

Even if this is a typo, it does not appear to have any visible impact on the behavior. At least on my compiler (Clang, macOS), the UINT32_C macro applied to integers that are too large to fit in 32 bits still seems to produce values of type unsigned long, which is 64 bits on my compiler. I don't know if there are other compilers where this indeed results in a bug.

Not sure if this was a typo or not, but seems like might be given that
the integer wouldn't fit into a `u32`. If this is not a typo and was
intentional, I'd be interested to understand the reasoning behind it.

Even if this is a typo, it does not appear to have any visible impact on
the behavior. At least on my compiler (Clang, macOS), the UINT32_C macro
applied to integers that are too large to fit in 32 bits still seems to
produce values of type `unsigned long`, which is 64 bits on my compiler.
I don't know if there are other compilers where this indeed results in a
bug.
@jk-jeon jk-jeon merged commit fe0e5fe into jk-jeon:master Aug 10, 2024
11 checks passed
@jk-jeon
Copy link
Owner

jk-jeon commented Aug 10, 2024

This is indeed a typo. Thanks for catching that!

jk-jeon added a commit to jk-jeon/rtz_benchmark that referenced this pull request Aug 24, 2024
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