-
Notifications
You must be signed in to change notification settings - Fork 96
enabled and fixed bugprone-narrowing-conversions clang-tidy warnings
#611
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
base: master
Are you sure you want to change the base?
Conversation
| currentToken += ch; | ||
| prev = ch; | ||
| currentToken += static_cast<char>(ch); | ||
| prev = static_cast<char>(ch); |
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.
I don't really agree here. ch is a unsigned char. all the bits will be saved in prev.
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.
This does not change any behavior. It just makes the implicit conversions explicit (prev is char and currentToken is basic_string<char>).
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.
I do know it doesn't change the behavior.
Casts are also bugprone and silence compilers. can bugprone-narrowing-conversions be solved without casts?
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.
Casts are also bugprone and silence compilers. can bugprone-narrowing-conversions be solved without casts?
No, not if the narrowing is intentional. The point is to make the code explicit.
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.
No, not if the narrowing is intentional. The point is to make the code explicit.
I feel that casts are dangerous because they hide all compiler warnings about all conversion errors. It's the sloppy approach to silencing warnings. Similar to writing --suppress=*. And there is no mechanism to detect redundant casts.
It does not say specifically that it tries to prevent warnings about the narrowing conversion. You could suggest a new cppcheck checker that will warn about this which provides a more explicit mechanism to hide the warnings. And I could approve that.
A similar code example where there is no warning:
void foo(unsigned int x) {
char y = static_cast<char>(x); // <- we have loss of precision here
}
Your cast hides warning about the sign conversion and it would also hide future warnings about loss of precision if we will have that bug. Real bugs can be hidden.
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.
clang-tidy does not tell me there is likely mistake here.
int y;
int foo(int x) {
y = static_cast<char>(x);
}
if I could indicate that my intention with the cast is only sign-conversion :
int y;
int foo(int x) {
y = static_cast_sign_conversion<char>(x);
}
then we could warn here, the cast does not do what was intended.
No description provided.