Skip to content

Conversation

@autoantwort
Copy link
Contributor

Fixes https://trac.cppcheck.net/ticket/13696 + #6257 (comment)
One could argue that the size of a member of an unknown type should be 1 instead of 0 because the member will be at least 1 byte large (if not empty and annotated with [[no_unique_address]])

@chrchr-github
Copy link
Collaborator

Thanks for your contribution. There are situations where the exact size of a type is required, and others (e.g. the passedByValue check) where a lower bound is sufficient. We should probably differentiate this by a flag passed to getSizeOf().

@firewave
Copy link
Collaborator

firewave commented Jun 2, 2025

We also need to add a test for the referenced ticket.

@autoantwort
Copy link
Contributor Author

autoantwort commented Jun 2, 2025

We also need to add a test for the referenced ticket.

Can you point me to the place where I should insert it (in which of the test cases)?

@chrchr-github
Copy link
Collaborator

We also need to add a test for the referenced ticket.

Can you point me to the place where I should insert it (in which of the test cases)?

There is a iterateByValue() test in testother.cpp.

@autoantwort autoantwort force-pushed the fix-costs-unknown-members branch from fcda7db to 769cbfe Compare June 3, 2025 09:07
@autoantwort
Copy link
Contributor Author

We should probably differentiate this by a flag passed to getSizeOf().

Done now. I was not sure what the right flag should be in every context.

We also need to add a test for the referenced ticket.

Done :)

@autoantwort autoantwort force-pushed the fix-costs-unknown-members branch 2 times, most recently from bcf58df to ed219c1 Compare June 3, 2025 13:01
@autoantwort
Copy link
Contributor Author

There are situations where the exact size of a type is required, and others (e.g. the passedByValue check) where a lower bound is sufficient.

Btw the current implementation already returned the lower bound if inheritance is used.

@autoantwort autoantwort force-pushed the fix-costs-unknown-members branch from ed219c1 to 6e90e74 Compare June 7, 2025 11:32
@autoantwort
Copy link
Contributor Author

@firewave Can you approve the workflow runs?

@chrchr-github chrchr-github merged commit 3181f13 into danmar:main Jun 16, 2025
53 checks passed
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