Skip to content

Conversation

@firewave
Copy link
Collaborator

No description provided.

@firewave firewave force-pushed the suppr-premium branch 3 times, most recently from b484351 to 9e30245 Compare October 29, 2025 10:17
@firewave
Copy link
Collaborator Author

I think we no longer need to differentiate the various suppressions (local, global, inline) (in some cases?) so some logic could be merged/moved. But I am not gonna touch that until the suppressions work completely. I recently spotted some more shortcomings with the builddir which I assume are currently not being tracked in any ticket and/or failing test.

@firewave

This comment was marked as resolved.

@firewave firewave force-pushed the suppr-premium branch 3 times, most recently from 587b7a7 to b6d366d Compare October 29, 2025 18:25
@firewave firewave changed the title fixed #13663 - reworked unmatchedSuppression exclusion logic / do not report misra-* or premium-* suppressions when they are not enabled fixed #13663/#14232 - reworked unmatchedSuppression exclusion logic / do not report misra-* or premium-* suppressions when they are not enabled Oct 29, 2025
@sonarqubecloud
Copy link

@firewave firewave marked this pull request as ready for review October 29, 2025 20:52
@firewave
Copy link
Collaborator Author

FYI This should be the penultimate step to finally being able to merge #3090.

@firewave firewave merged commit 71a4af2 into danmar:main Oct 30, 2025
55 checks passed
@firewave firewave deleted the suppr-premium branch October 30, 2025 14:43
@danmar
Copy link
Owner

danmar commented Oct 30, 2025

For information.. I am thinking about a possible update in the future where we probably want to write a separate unmatchSuppressionHash id if the suppression is based on a specific hash.
But that is not something I wanted that you would do in this PR.. :-)

@firewave
Copy link
Collaborator Author

For information.. I am thinking about a possible update in the future where we probably want to write a separate unmatchSuppressionHash id if the suppression is based on a specific hash. But that is not something I wanted that you would do in this PR.. :-)

You need to elaborate more that (at best in a ticket).

Please do not introduce anything new to the suppression handling until the remaining shortcomings have been addressed. Although this finally enables us to enable them in the CI there is still quite some work ahead because they still don't play well with the builddir (along with other builddir shortcomings).

I no longer have any local suppressions changes (finally) so it would not conflict but the part are not in the proper places and I fear that the necessary changes might cause some headaches when processes will be involved.

@danmar
Copy link
Owner

danmar commented Oct 30, 2025

You need to elaborate more that (at best in a ticket).

ok well this is not something we should look at before the release at all..

and it would sound good if the shortcomings are fixed first.

Background: Some premium checkers are by intention more noisy. As you might have noticed because we run cppcheck premium on open source Cppcheck PRs. The philosophy of Cert/Misra is more aggressive. And many customers wants to have more aggressive checking.

I intend to calculate a hash for the Cppcheck Premium warnings that will make it possible to suppress "old" warnings based on that.

New customers will be able to run cppcheck on their code and use the report as a "baseline". Ideally those warnings are not reported again then and Cppcheck will only write warnings for new code.

If a warning in the "baseline" goes away I am not sure that a "unmatchedSuppression" is wanted. But maybe a "unmatchedSuppressionHash" would be fine.. it's likely that the user will not care about those.

@firewave
Copy link
Collaborator Author

So basically a diff report based on existing results.

Based off the top my head you could generate special suppressions (i.e. do not report them as unmatched) from a builddir or an XML result. That could be easily be implemented and at first look even sounds like a neat feature.

The problematic part is when the location changes or code in question changes. That might require some messy logic and still would generate unwanted warnings.

@firewave
Copy link
Collaborator Author

I filed https://trac.cppcheck.net/ticket/14238 about it and it also gave me ideas for other features.

@danmar
Copy link
Owner

danmar commented Oct 31, 2025

looks good to me.

@danmar
Copy link
Owner

danmar commented Oct 31, 2025

The problematic part is when the location changes or code in question changes. That might require some messy logic and still would generate unwanted warnings.

yes. It's far from ideal but in cppcheck premium my first approach is that for warnings in global scope the hash is determined by the current statement. for warnings in executable scopes the hash is determined by the whole function code.

I would asssume we can make much better calculations than this later. a semantic warning should more or less only depend on current statement..

@danmar
Copy link
Owner

danmar commented Oct 31, 2025

I think you have a good idea in the ticket to just use the old result directly as a baseline and not mess with suppressions..

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.

2 participants