Skip to content

Fix #14064 Files are reanalyzed with --cppcheck-build-dir and inline suppressions (regression) - #7741

Merged
chrchr-github merged 11 commits into
cppcheck-opensource:mainfrom
chrchr-github:chr_14064
Aug 19, 2025
Merged

Fix #14064 Files are reanalyzed with --cppcheck-build-dir and inline suppressions (regression)#7741
chrchr-github merged 11 commits into
cppcheck-opensource:mainfrom
chrchr-github:chr_14064

Conversation

@chrchr-github

@chrchr-githubchrchr-github commented Aug 14, 2025

Copy link
Copy Markdown
Collaborator

No description provided.

firewave
firewave reviewed Aug 14, 2025
View reviewed changes
Comment threadlib/suppressions.cpp Outdated

out << " <suppressions>" << std::endl;
for (const Suppression &suppression : mSuppressions) {
if (!filePath.empty() && !suppression.fileName.empty() && filePath != suppression.fileName)

@firewavefirewaveAug 14, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As the suppression could contain a pattern we need to use the matching logic of the suppressions.

@chrchr-githubchrchr-githubAug 15, 2025

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Not sure I follow. There is only one suppression at a time, and we only care about inline suppressions.

@firewave

firewave commented Aug 14, 2025

Copy link
Copy Markdown
Collaborator

Way to test the non-inline suppression case:

  • create two input files
  • analyze with builddir
  • provide --suppress CLI option which only matches one of the files
  • re-analyze
  • make sure only the matching file has a different hash

@firewave

firewave commented Aug 14, 2025

Copy link
Copy Markdown
Collaborator

It is possible that unmatchedSuppression warnings could somehow be leveraged to test this - but since those are still partially broken that is not really an option.

@chrchr-github
chrchr-github marked this pull request as ready for review August 15, 2025 06:57
@sonarqubecloud

sonarqubecloudBot commented Aug 15, 2025

Copy link
Copy Markdown

Quality Gate PassedQuality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@firewave

firewave commented Aug 18, 2025

Copy link
Copy Markdown
Collaborator

LGTM but tests requiring sleep always give me pause. But not writing at all and not modifying are actually different cases (not necessary to address here). I will try to write some tests at a later date.

Although I/we should rather work on #7079 as a priority and IMO no more builddir changes should be landed before that is in.

@chrchr-github

chrchr-github commented Aug 18, 2025

Copy link
Copy Markdown
CollaboratorAuthor

LGTM but tests requiring sleep always give me pause. But not writing at all and not modifying are actually different cases (not necessary to address here). I will try to write some tests at a later date.

sleep() is not strictly required. But depending on file time granularity and execution speed, reanalysis could happen in "zero time" and would not be caught without it.
On a related note, the loop with 10 repetitions caused the test to fail ~90% of the time before the fix.

@chrchr-github

chrchr-github commented Aug 19, 2025

Copy link
Copy Markdown
CollaboratorAuthor

We might want to backport this to 2.18 as well.

@firewave

firewave commented Aug 19, 2025

Copy link
Copy Markdown
Collaborator

We might want to backport this to 2.18 as well.

Definitely.

@chrchr-github
chrchr-github merged commit 4617bc2 into cppcheck-opensource:mainAug 19, 2025
63 checks passed
@chrchr-github
chrchr-github deleted the chr_14064 branch August 19, 2025 14:31
firewave pushed a commit to firewave/cppcheck that referenced this pull request Aug 20, 2025
@chrchr-github@firewave
Fix #14064 Files are reanalyzed with --cppcheck-build-dir and inline …
ac4a86d
…suppressions (regression) (cppcheck-opensource#7741)
(cherry picked from commit 4617bc2)
@firewavefirewave mentioned this pull request Aug 20, 2025
Merged
@firewave

firewave commented Aug 20, 2025

Copy link
Copy Markdown
Collaborator

We might want to backport this to 2.18 as well.

See #7756.

firewave added a commit that referenced this pull request Aug 22, 2025
@firewave@chrchr-github
Fix #14064 Files are reanalyzed with --cppcheck-build-dir and inline …
a2304d5
…suppressions (regression) (#7741) (#7756)
(cherry picked from commit 4617bc2)
Co-authored-by: chrchr-github <78114321+chrchr-github@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@firewavefirewavefirewave left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chrchr-github@firewave@web-flow