Apply warnings/suggestions reported by clang-tidy v9 #67
Conversation
Clang-tidy version 9 reported a few warnings, and since we treat errors as warnings we better address them. I've disabled the trailing-return-type suggestion because I believe it's ridiculous. Signed-off-by: Marco Magdy <[email protected]>
Signed-off-by: Marco Magdy <[email protected]>
| constexpr int space = 0x20; // space (0x20, ' ') | ||
| constexpr int form_feed = 0x0c; // form feed (0x0c, '\f') | ||
| constexpr int line_feed = 0x0a; // line feed (0x0a, '\n') | ||
| constexpr int carriage_return = 0x0d; // carriage return (0x0d, '\r') | ||
| constexpr int horizontal_tab = 0x09; // horizontal tab (0x09, '\t') | ||
| constexpr int vertical_tab = 0x0b; // vertical tab (0x0b, '\v') | ||
| switch (ch) { | ||
| case space: | ||
| case form_feed: | ||
| case line_feed: | ||
| case carriage_return: | ||
| case horizontal_tab: | ||
| case vertical_tab: | ||
| return true; | ||
| default: | ||
| return false; |
There was a problem hiding this comment.
isspace has a cloud of UB around it.
The behavior is undefined if the value of ch is not representable as unsigned char and is not equal to EOF.
I'm not sure what "not representable as unsigned char" really means here. So I went ahead and did the simple thing I need to avoid the shenanigans.
Signed-off-by: Marco Magdy <[email protected]>
src/runtime.cpp
Outdated
| @@ -276,7 +290,8 @@ runtime::next_outcome runtime::get_next() | |||
|
|
|||
| if (resp.has_header(DEADLINE_MS_HEADER)) { | |||
| auto const& deadline_string = resp.get_header(DEADLINE_MS_HEADER); | |||
| unsigned long ms = strtoul(deadline_string.c_str(), nullptr, 10); | |||
| constexpr int decimal_radix = 10; | |||
There was a problem hiding this comment.
This one feels stupid. I don't love the magic numbers check (disabled it in all of the CRT libs), but this one just feels extra unnecessary.
There was a problem hiding this comment.
Sigh. Yes. You're absolutely right. I hate it.
P.S I rewrote this comment 3 times starting with: "I'm feeling worried about future people working on the project" to "I'm close to nuking it if it complains one more time" to "You're right, screw this"
There was a problem hiding this comment.
Decided to keep the magic number warning
I was about to remove the constexpr variable and replace the argument
with the hardcoded value with an inline comment and I realized that this
is exactly what the warning/error is about. Documentation/readability.
I was about to remove the constexpr variable and replace the argument with the hardcoded value with an inline comment and I realized that this is exactly what the warning/error is about. Documentation/readability.
Clang-tidy version 9 reported a few warnings, and since we treat errors
as warnings we better address them.
I've disabled the trailing-return-type suggestion because I believe it's
ridiculous.
Signed-off-by: Marco Magdy [email protected]
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.