Skip to content

fixed #13363 - apply default signedness to char only #7155

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

Merged
firewave merged 1 commit into from
Jan 12, 2025
Merged

fixed #13363 - apply default signedness to char only #7155

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
4 changes: 2 additions & 2 deletions lib/symboldatabase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7185,7 +7185,7 @@ static const Token* parsedecl(const Token* type,
else if (enum_type->isUnsigned())
valuetype->sign = ValueType::Sign::UNSIGNED;
else
valuetype->sign = defaultSignedness;
valuetype->sign = defaultSignedness; // TODO: this is implementation-dependent might be separate from char
Copy link
Copy Markdown
Collaborator

@danmar danmar Jan 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? I feel there is something I don't understand.

enum {A,B,C} abc;
if (abc == A) {}  // <- A is an 'int' right?

Copy link
Copy Markdown
Collaborator Author

@firewave firewave Jan 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an integer but not necessarily int.

From https://en.cppreference.com/w/cpp/language/enum:
the underlying type is an implementation-defined integral type that can represent all enumerator values; this type is not larger than int unless the value of an enumerator cannot fit in an int or unsigned int

Also https://stackoverflow.com/a/1122128/532627 for an actual reference to the standard.

It should definitely not depend on the flag we use for char which is configurable via compiler flags.

Copy link
Copy Markdown
Collaborator

@danmar danmar Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an integer but not necessarily int.

ok I see.

It should definitely not depend on the flag we use for char which is configurable via compiler flags.

I agree.

Copy link
Copy Markdown
Collaborator Author

@firewave firewave Jan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed https://trac.cppcheck.net/ticket/13602 about this.

const ValueType::Type t = ValueType::typeFromString(enum_type->str(), enum_type->isLong());
if (t != ValueType::Type::UNKNOWN_TYPE)
valuetype->type = t;
Expand Down Expand Up @@ -7586,7 +7586,7 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to
else if (tok->previous()->isSigned())
valuetype.sign = ValueType::Sign::SIGNED;
else if (valuetype.isIntegral() && valuetype.type != ValueType::UNKNOWN_INT)
valuetype.sign = mDefaultSignedness;
valuetype.sign = (valuetype.type == ValueType::Type::CHAR) ? mDefaultSignedness : ValueType::Sign::SIGNED;
setValueType(tok, valuetype);
}

Expand Down
20 changes: 20 additions & 0 deletions test/testio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ class TestIO : public TestFixture {
TEST_CASE(testPrintfParenthesis); // #8489
TEST_CASE(testStdDistance); // #10304
TEST_CASE(testParameterPack); // #11289

TEST_CASE(testDefaultSignInt); // #13363
}

struct CheckOptions
Expand All @@ -85,6 +87,7 @@ class TestIO : public TestFixture {
bool inconclusive = false;
bool portability = false;
Platform::Type platform = Platform::Type::Unspecified;
char defaultSign = '\0';
bool onlyFormatStr = false;
bool cpp = true;
};
Expand All @@ -96,6 +99,7 @@ class TestIO : public TestFixture {
settings1.severity.setEnabled(Severity::portability, options.portability);
settings1.certainty.setEnabled(Certainty::inconclusive, options.inconclusive);
PLATFORM(settings1.platform, options.platform);
settings1.platform.defaultSign = options.defaultSign;

// Tokenize..
SimpleTokenizer tokenizer(settings1, *this);
Expand Down Expand Up @@ -4933,6 +4937,22 @@ class TestIO : public TestFixture {
"}\n");
ASSERT_EQUALS("", errout_str());
}

// TODO: we need to run big tests with a platform that has unsigned chars
void testDefaultSignInt() { // #13363
// Platform::defaultSign should only affect char
const char code[] =
"void f() {\n"
" double d = 1\n;"
" printf(\"%i\", int(d));\n"
"}\n";
check(code);
ASSERT_EQUALS("", errout_str());
check(code, dinit(CheckOptions, $.defaultSign = 's'));
ASSERT_EQUALS("", errout_str());
check(code, dinit(CheckOptions, $.defaultSign = 'u'));
ASSERT_EQUALS("", errout_str());
}
};

REGISTER_TEST(TestIO)