Skip to content

Fix expected output in TestCondition::alwaysTrue and TestCondition::alwaysTrueContainer #7709

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

Merged
merged 1 commit into from
Aug 4, 2025

Conversation

andreas-schwab
Copy link
Contributor

No description provided.

Copy link

@chrchr-github
Copy link
Collaborator

Can this be tested using e.g. -funsigned-char?

@andreas-schwab
Copy link
Contributor Author

-funsigned-char may change the ABI and thus invokes undefined behaviour.

@chrchr-github
Copy link
Collaborator

I was actually thinking of cppcheck's own -funsigned-char switch.

But I see now that we only run a single test (TestSymbolDatabase) in build_uchar (CI-unixish) with unsigned char, so the bad output was not detected. I wonder if we should run all tests?

@danmar
Copy link
Owner

danmar commented Aug 4, 2025

I wonder if we should run all tests?

it sounds reasonable to me.

however is that related to this PR? the std::numeric_limits<char>::is_signed would still be true no matter which cppcheck options is used?

@chrchr-github
Copy link
Collaborator

I wonder if we should run all tests?

it sounds reasonable to me.

however is that related to this PR? the std::numeric_limits<char>::is_signed would still be true no matter which cppcheck options is used?

Yes, -funsigned-charprobably doesn't help here. I hadn't realized that we have build_uchar in the CI, so we should run all tests there.

@danmar danmar merged commit e5efd12 into danmar:main Aug 4, 2025
65 checks passed
@firewave
Copy link
Collaborator

Different output depending on build flags feels like a bug to me. It should be platform-agnostic and this should rather be handled in the check than the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants