Skip to content

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.

Sign up for GitHub

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

Open
firewave wants to merge 1 commit into
base: master
Choose a base branch
from
Open

enabled and fixed bugprone-narrowing-conversions clang-tidy warnings #611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .clang-tidy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ Checks: >
-boost-use-ranges,
-bugprone-branch-clone,
-bugprone-easily-swappable-parameters,
-bugprone-narrowing-conversions,
-bugprone-switch-missing-default-case,
-concurrency-mt-unsafe,
-misc-no-recursion,
Expand Down
36 changes: 18 additions & 18 deletions simplecpp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,8 +765,8 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename,
if (ppTok->next && (ppTok->next->str() == "error" || ppTok->next->str() == "warning")) {
char prev = ' ';
while (stream.good() && (prev == '\\' || (ch != '\r' && ch != '\n'))) {
currentToken += ch;
prev = ch;
currentToken += static_cast<char>(ch);
prev = static_cast<char>(ch);
Copy link
Owner

@danmar danmar Jan 6, 2026

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.

Copy link
Collaborator Author

@firewave firewave Jan 8, 2026

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>).

Copy link
Owner

@danmar danmar Jan 8, 2026

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?

Copy link
Collaborator Author

@firewave firewave Jan 8, 2026

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.

Copy link
Owner

@danmar danmar Jan 11, 2026 •
edited
Loading

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.

Copy link
Owner

@danmar danmar Jan 11, 2026 •
edited
Loading

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.

ch = stream.readChar();
}
stream.ungetChar();
Expand All @@ -780,7 +780,7 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename,
if (isNameChar(ch)) {
const bool num = !!std::isdigit(ch);
while (stream.good() && isNameChar(ch)) {
currentToken += ch;
currentToken += static_cast<char>(ch);
ch = stream.readChar();
if (num && ch=='\'' && isNameChar(stream.peekChar()))
ch = stream.readChar();
Expand All @@ -792,14 +792,14 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename,
// comment
else if (ch == '/' && stream.peekChar() == '/') {
while (stream.good() && ch != '\n') {
currentToken += ch;
currentToken += static_cast<char>(ch);
ch = stream.readChar();
if (ch == '\\') {
TokenString tmp;
char tmp_ch = ch;
char tmp_ch = static_cast<char>(ch);
while ((stream.good()) && (tmp_ch == '\\' || tmp_ch == ' ' || tmp_ch == '\t')) {
tmp += tmp_ch;
tmp_ch = stream.readChar();
tmp_ch = static_cast<char>(stream.readChar());
}
if (!stream.good()) {
break;
Expand All @@ -813,7 +813,7 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename,
if (pos < check_portability.size() - 1U && check_portability[pos] == '\\')
portabilityBackslash(outputList, location);
++multiline;
tmp_ch = stream.readChar();
tmp_ch = static_cast<char>(stream.readChar());
currentToken += '\n';
}
ch = tmp_ch;
Expand All @@ -830,7 +830,7 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename,
(void)stream.readChar();
ch = stream.readChar();
while (stream.good()) {
currentToken += ch;
currentToken += static_cast<char>(ch);
if (currentToken.size() >= 4U && endsWith(currentToken, COMMENT_END))
break;
ch = stream.readChar();
Expand Down Expand Up @@ -862,11 +862,11 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename,
// C++11 raw string literal
if (ch == '\"' && !prefix.empty() && *cback()->str().rbegin() == 'R') {
std::string delim;
currentToken = ch;
currentToken = static_cast<char>(ch);
prefix.resize(prefix.size() - 1);
ch = stream.readChar();
while (stream.good() && ch != '(' && ch != '\n') {
delim += ch;
delim += static_cast<char>(ch);
ch = stream.readChar();
}
if (!stream.good() || ch == '\n') {
Expand All @@ -882,7 +882,7 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename,
}
const std::string endOfRawString(')' + delim + currentToken);
while (stream.good() && (!endsWith(currentToken, endOfRawString) || currentToken.size() <= 1))
currentToken += stream.readChar();
currentToken += static_cast<char>(stream.readChar());
if (!endsWith(currentToken, endOfRawString)) {
if (outputList) {
Output err = {
Expand All @@ -907,7 +907,7 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename,
continue;
}

currentToken = readUntil(stream,location,ch,ch,outputList);
currentToken = readUntil(stream,location,static_cast<char>(ch),static_cast<char>(ch),outputList);
if (currentToken.size() < 2U)
// Error is reported by readUntil()
return;
Expand Down Expand Up @@ -939,7 +939,7 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename,
}

else {
currentToken += ch;
currentToken += static_cast<char>(ch);
}

if (*currentToken.begin() == '<') {
Expand Down Expand Up @@ -1001,7 +1001,7 @@ static bool isFloatSuffix(const simplecpp::Token *tok)
{
if (!tok || tok->str().size() != 1U)
return false;
const char c = std::tolower(tok->str()[0]);
const char c = static_cast<char>(std::tolower(tok->str()[0]));
return c == 'f' || c == 'l';
}

Expand Down Expand Up @@ -1400,7 +1400,7 @@ std::string simplecpp::TokenList::readUntil(Stream &stream, const Location &loca
bool backslash = false;
char ch = 0;
while (ch != end && ch != '\r' && ch != '\n' && stream.good()) {
ch = stream.readChar();
ch = static_cast<char>(stream.readChar());
if (backslash && ch == '\n') {
ch = 0;
backslash = false;
Expand All @@ -1412,7 +1412,7 @@ std::string simplecpp::TokenList::readUntil(Stream &stream, const Location &loca
bool update_ch = false;
char next = 0;
do {
next = stream.readChar();
next = static_cast<char>(stream.readChar());
if (next == '\r' || next == '\n') {
ret.erase(ret.size()-1U);
backslash = (next == '\r');
Expand Down Expand Up @@ -2842,7 +2842,7 @@ long long simplecpp::characterLiteralToLL(const std::string& str)
case 'U': {
// universal character names have exactly 4 or 8 digits
const std::size_t ndigits = (escape == 'u' ? 4 : 8);
value = stringToULLbounded(str, pos, 16, ndigits, ndigits);
value = stringToULLbounded(str, pos, 16, static_cast<std::ptrdiff_t>(ndigits), ndigits);

// UTF-8 encodes code points above 0x7f in multiple code units
// code points above 0x10ffff are not allowed
Expand Down Expand Up @@ -2926,7 +2926,7 @@ long long simplecpp::characterLiteralToLL(const std::string& str)

// All other cases are unsigned. Since long long is at least 64bit wide,
// while the literals at most 32bit wide, the conversion preserves all values.
return multivalue;
return static_cast<long long>(multivalue);
}

/**
Expand Down