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
Codechange: math functions - use cpp-style casts #7685
Conversation
67ee372
to
7659155
Compare
7659155
to
d838605
Compare
Of course commit checker fails because |
Ah i see, i just found an old conversation about this here: #7407 (comment) I'll make that change. |
d838605
to
c018e49
Compare
Please restore the indentation between |
c018e49
to
78dc3dc
Compare
78dc3dc
to
ed211aa
Compare
Not that I necessarily disagree with the changes, but I'd like to see some justification for them. What problem do they solve? How do they improve things? |
Using C++ style casts makes the intention of the code more explicit:
From: https://google.github.io/styleguide/cppguide.html#Casting That said, maybe this should be an all-or-nothing style decision. C++ casts could be used everywhere if that's decided to be an improvement, otherwise just keep everything as it is. |
@@ -383,7 +383,7 @@ static inline T ROR(const T x, const uint8 n) | |||
{ | |||
#if !defined(__ICC) && defined(__GNUC__) && ((__GNUC__ > 4) || ((__GNUC__ == 4) && __GNUC_MINOR__ >= 3)) | |||
/* GCC >= 4.3 provides a builtin, resulting in faster code */ | |||
return (uint32)__builtin_bswap32((int32)x); | |||
return static_cast<uint32>(__builtin_bswap32(static_cast<int32>(x))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html, the cast to int32 is unnecessary (or possibly wrong)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Close enough
No description provided.